Skip to content

Make zwave_js config panel inclusion state aware#11556

Merged
bramkragten merged 9 commits intohome-assistant:devfrom
raman325:inclusion_state
Feb 17, 2022
Merged

Make zwave_js config panel inclusion state aware#11556
bramkragten merged 9 commits intohome-assistant:devfrom
raman325:inclusion_state

Conversation

@raman325
Copy link
Copy Markdown
Contributor

@raman325 raman325 commented Feb 5, 2022

Proposed change

Related to home-assistant/core#65398

This code probably needs some work as it feels a bit hacked together, but the end result is that when a user loads the zwave_js config panel while an inclusion/exclusion is in progress, the buttons to add and remove devices disappear and an alert is shown with an option to cancel (see screenshot below). A couple of things to consider/improve:

  1. If you cancel the inclusion/exclusion from the alert, you have to press it twice before the frontend re-renders and the buttons come back. I hope there's a simple way to fix this.
  2. There are more inclusion state's beyond Idle, Including, and Excluding. But you can only cancel on Including or Excluding. I can probably fix this up so the alert shows but the button doesn't or is disabled, I just thought it would add more complexity and wanted to hear back on another approach if needed before fixing it.

image

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@MartinHjelmare
Copy link
Copy Markdown
Member

Core PR is approved and waiting for frontend team feedback.

raman325 and others added 3 commits February 14, 2022 17:32
…ve_js-config-dashboard.ts

Co-authored-by: Bram Kragten <mail@bramkragten.nl>
…ve_js-config-dashboard.ts

Co-authored-by: Bram Kragten <mail@bramkragten.nl>
@matthiasdebaat
Copy link
Copy Markdown
Member

On the Z-Wave page we use add device and remove device. Personally I think this is more user than inclusion and exclusion (but don't know if this is a must to make us certified?).

To keep it consistent the alert could be changed to [integration] is searching for devices and the button stop searching.

@raman325
Copy link
Copy Markdown
Contributor Author

raman325 commented Feb 16, 2022

On the Z-Wave page we use add device and remove device. Personally I think this is more user than inclusion and exclusion (but don't know if this is a must to make us certified?).

To keep it consistent the alert could be changed to [integration] is searching for devices and the button stop searching.

It's not a must to use the words exclusion/inclusion. At the same time, it's not 100% accurate that the integration is searching for devices. The Z-Wave JS driver is searching for devices and it may have been initiated from HA but it may not have been. Maybe then we say Z-Wave JS is searching for devices? The stop searching text makes sense

@matthiasdebaat
Copy link
Copy Markdown
Member

It's not a must to use the words exclusion/inclusion. At the same time, it's not 100% accurate that the integration is searching for devices. The Z-Wave JS driver is searching for devices and it may have been initiated from HA but it may not have been. Maybe then we say Z-Wave JS is searching for devices? The stop searching text makes sense

Yeah let's do that!

@bramkragten bramkragten merged commit b55c7ed into home-assistant:dev Feb 17, 2022
@raman325 raman325 deleted the inclusion_state branch February 17, 2022 17:54
@github-actions github-actions bot locked and limited conversation to collaborators Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants