-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SmartStart provisioning support to zwave_js WS API #59037
Conversation
Hey there @home-assistant/z-wave, mind taking a look at this pull request as it has been labeled with an integration ( |
We can rebase here now since we've bumped the library. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI I will be moving most of the validation logic into the lib since we have to bump the schema anyway. That should shorten this PR by quite a bit |
Co-authored-by: Paulus Schoutsen <[email protected]>
This comment has been minimized.
This comment has been minimized.
@raman325 I changed the provision info schema to allow None values for the optional values. It was the most simple fix I saw to make it working. I'm testing this with Bram's frontend branch now. I managed to provision a test device. We'd like to add the device to the device registry when provisioning a device to have it show in the device list in the frontend with provisioned status. What do you think? Can you add that? |
Are you talking specifically about Smart Start nodes? So register the device when |
Yeah, if the provision succeeds, we add the device to the device registry. Is there a way to tell if the provision succeeds or fails? We're also discussing how to guard against duplicate provisions since the controller doesn't seem to guard against that. We could add |
Bram can handle the frontend work, if any, that's needed here. But I'd like your opinion about the architecture design if this makes sense. |
it definitely adds complexity. The device identifier is currently |
Ok, let's just make a new view then in the zwave js panel with a list of provisioned devices. |
Ok, yeah there needs to be some identifier that is the same in both the provisioned device and the added device to be able to map them. Let's go with a separate list of provisioned devices for now then. |
Let's leave any complexity for future us. |
Adding of devices with QR is done now in home-assistant/frontend#10726 Will make another PR for listing provisioned devices |
Proposed change
In this PR we update
add_node
andreplace_failed_node
commands to accept the new inputs. We also add the following commands:provision_smart_start_node
- Pre-provision smart start nodeunprovision_smart_start_node
- Remove pre-provisioned smart start node from provisioning listget_provisioning_entries
- gets all registered provisioning entriesparse_qr_code_string
- Parses string from a QR code into provisioning informationsupports_feature
- Returns whether controller supports a feature (Smart Start is the only feature to check at the moment)And update
remove_node
to accept the optionalunprovision
parameter (also new)The lib also has
get_provisioning_entry
but I am not sure that it would serve a purpose here unless we want to do something with it on the device info page for a node that was already added but the entry remains.Dependent on home-assistant-libs/zwave-js-server-python#315 and zwave-js/zwave-js-server#432
Will be in draft for a bit while things get reviewed and released upstream, but I wanted to share this so that we could discuss any required changes to the model and plan the frontend work.
CC @MartinHjelmare
Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: