zwave-js / zwave-js-ui

Full featured Z-Wave Control Panel UI and MQTT gateway. Built using Nodejs, and Vue/Vuetify
https://zwave-js.github.io/zwave-js-ui
MIT License
989 stars 206 forks source link

Inclusion for LR devices through SmartStart should be easier, especially for non-LR -> LR migration case #3754

Open madbrain76 opened 6 months ago

madbrain76 commented 6 months ago

Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

I had 16 devices that were already included in non-LR mode, that actually supported LR. They are all ZEN76 800 series. I wanted to switch them to LR mode. 1) I excluded them all in Z-Wave JS UI 2) Then, I enabled them in SmartStart, through the pre-existing SmartStart entries. 3) My assumption was that the devices would automatically be re-included in LR mode, now that Z-Wave JS had added LR support. 4) That assumption turned out to be wrong. The devices kept being re-included in non-LR mode. 5) I could not figure how to do SmartStart inclusion in LR mode, until I saw a thread in the HA community explaining it. The solution is that one needs to toggle the "Z-wave" graphical button to change it to "Z-wave LR" in the "protocol" column before enabling SmartStart for the device. There was nothing obvious about that, and I quite simply would never have figured it out on my own without a search engine and the community's help.

Describe the solution you'd like A clear and concise description of what you want to happen.

I'm not a UI/UX designer, so feel free to ignore my suggestions and come up with something better. There needs to be a clear and obvious opportunity to choose the LR protocol before re-inclusion. I don't have any particular preference or great idea on how help with this. Here are some ideas in no particular order :

1) In the Smart Start screen, instead of showing a "protocol" column with 4 possible states Z-wave or Z-wave LR clickable, and Z-wave or Z-wave LR greyed out after inclusion, make it more obvious that this is a property that be changed, and also more consistent with the other Z-wave options (S2, etc) which also get greyed out after inclusion. A very simple way to change that would be to delete the "protocol" column and replace it with a "Long range" column. And then have a checkbox in that column, which gets greyed out after inclusion, like other properties already do.

2) Not sure if it's in the spec, but perhaps Z-wave JS could query the device for LR support, before it is excluded, and use that information later on for SmartStart LR re-inclusion, by automatically checking the LR box for that entry in step 1. Even if it's in spec, it probably requires a re-interview, so it isn't of great help unless one is willing to re-interview the nodes manually. It wouldn't happen through just exclusion. There might not be much value in this particular suggestion :(

3) When clicking the "active" selector to "on", for any entries that were added prior to Z-wave JS adding LR support, ask the user if they want to switch to LR inclusion mode. The downside is that many people will leave the entry state as "on" even after inclusion, and thus won't get promped to switch - the device will just be re-included in non-LR mode.

4) This is the most work to implement, but also the best for the user : Add an option to migrate the non-LR device to LR mode, sort of a wizard. This is somewhat contrary to SmartStart, which is supposed to work by itself. Yet, perhaps adding a "Migrate to LR" button under "actions" could trigger this wizard. Though right now the trash button is tiny. Perhaps this migration process really belongs in the main control panel screen, but it also has an impact on SmartStart entries.

Anyway, the migration wizard would need to do 3 things : a) de-activate the SmartStart entry for the device b) exclude the device c) update the SmartStart entry for the device with LR inclusion d) turn on the SmartStart entry for the device e) wait for inclusion f) huge bonus points if all the entities names/IDs remain the same after migration as they were before. I had a lot of automations to fix after migrating 16 switches to LR mode.

5) maybe lookup existing devices in a database, see if they support LR mode, and prompt the user to migrate them at some point, using the wizard from step 4. The big question is when to prompt ... And if not prompt, make it really obvious somehow.

6) The same sort of wizard could probably be used to migrate devices from non-secure to secure mode. I have several that initially were included without encryption, or with S0 instead of S2. There is a case to be made for "upgrade security" also, if Z-wave JS has the info about the device supporting a higher level, again from a database. Obviously that is no in-scope for this particular task.

Describe alternatives you've considered I used the existing UI, after help from the community.

robertsLando commented 6 months ago

I would like to know @AlCalzone opinions about this. I think that the only thing that's really useful is to add a button like Migrate to LR in Provisioning entries table that shows up a wizard kind of like the inclusion dialog that guides you throught the process of the migration:

  1. Exclude the node (needs a manual action in order to put the device on exclusion mode)
  2. Edit the provisioning entry (maybe this can be done before the exclusion)
  3. Just wait for the new node to be included back

About the ids them cannot be the same unfortunately

AlCalzone commented 6 months ago

I understand where this is coming from, but LR has a few downsides aswell:

For this reason, not automagically including with LR is a conscious decision, and the user must decide whether they want this or not.

prompt the user to migrate them at some point

For the same reason we don't want to indicate to the users that they should migrate. They can if they want.

A very simple way to change that would be to delete the "protocol" column and replace it with a "Long range" column. And then have a checkbox in that column, which gets greyed out after inclusion, like other properties already do.

I think I tried to convince @robertsLando to do this. The current badge used for this isn't obvious, I agree.

query the device for LR support lookup existing devices in a database, see if they support LR mode

We don't keep track of that. The only way to know for existing devices in the mesh is if they were included via SmartStart, and that SmartStart entry contains LR as a supported protocol. This does not work if they were included with S2 Unauthenticated unfortunately, because there we cannot map the node to its provisioning entry. Likewise we don't keep a database of supported security classes, this would be a maintenance nightmare.

all the entities names/IDs

Not sure about entitiy IDs, but the node IDs at least are guaranteed to change. LR nodes can only be included with SmartStart, and their node IDs are >= 256 by design, whereas mesh devices use 1..232.


All this said, I think a "wizard" makes the most sense. It wouldn't do a lot other than what you can already do manually, except that it would tell you what to do when. For a device that...

...it could:

The steps in bold would be what you do by hand, FYI.

madbrain76 commented 6 months ago

Hi,

I understand where this is coming from, but LR has a few downsides aswell:

* LR devices do not take part in the mesh, so you cannot use them to bridge to other nodes in remote locations. You still need the mesh for that

* LR devices do not support associations, nor do they support talking to each other, so this might break any direct communication you might have

Thanks. I knew about the first point, but didn't realize the second point.

Perhaps the wizard should be bi-directional, ie. migrate to LR or away from LR (not sure what the right term would be. Maybe "mesh") ?

For this reason, not automagically including with LR is a conscious decision, and the user must decide whether they want this or not.

prompt the user to migrate them at some point

For the same reason we don't want to indicate to the users that they should migrate. They can if they want.

I see. The main issue I faced was that I couldn't do so easily even though I wanted, due to the UI being a bit obscure.

A very simple way to change that would be to delete the "protocol" column and replace it with a "Long range" column. And then have a checkbox in that column, which gets greyed out after inclusion, like other properties already do.

I think I tried to convince @robertsLando to do this. The current badge used for this isn't obvious, I agree.

Thanks, maybe we just needed more votes :)

query the device for LR support lookup existing devices in a database, see if they support LR mode

We don't keep track of that. The only way to know for existing devices in the mesh is if they were included via SmartStart, and that SmartStart entry contains LR as a supported protocol. This does not work if they were included with S2 Unauthenticated unfortunately, because there we cannot map the node to its provisioning entry. Likewise we don't keep a database of supported security classes, this would be a maintenance nightmare.

FWIW, I have seen my Smart start entries completely disappear. I'm not sure at which point they did, but it happened. This is part of the reason that motivated me to write the program I did to recover them all.

I would think for S2 unauthenticated devices, it would be less important to store SmartStart entries, even though it could also be done, of course.

As far as "maintenance nightmare", it seems you already rely on some kind of Z-wave device database to translate IDs to fetch strings for product names and brand names. I bought some ZAC36 repeaters before they were added to Z-Wave JS UI, and they just showed with numbers. I thought perhaps this same database might contain information about security properties or LR properties of each device. Perhaps that isn't the case, but if it was, you wouldn't have to maintain it locally, or at worst just fetch it if it disappears.

all the entities names/IDs

Not sure about entitiy IDs, but the node IDs at least are guaranteed to change. LR nodes can only be included with SmartStart, and their node IDs are >= 256 by design, whereas mesh devices use 1..232.

Yes, indeed, I have seen that.

The Node ID is not exposed to Home Assistant through controls, sensors, events, configuration or diagnostics. It is shown under "Z-Wave info". Can a Home assistant automation actually access this ? There is no corresponding entity. It looks like it is unfortunately exposed as part of the default entity names. For example, I have a smartplug with a control of button.node_135_ping and and sensor.node_135_node_status . That is somewhat unfortunate :-(. One could of course keep those names even though the ID is changing. It would be somewhat confusing if you ever look at those entity names.

Perhaps part of the solution should come from Home Assistant - ie. allow an entity to be renamed without breaking all automations that use it. Many people have asked for this feature, and it hasn't happened.

All this said, I think a "wizard" makes the most sense. It wouldn't do a lot other than what you can already do manually, except that it would tell you what to do when. For a device that...

* was included via Z-Wave Classic

* using SmartStart

* using S2 Authenticated or Access Control

* and has Z-Wave LR as a supported protocol in its provisioning entry

...it could:

* **enable exclusion mode on the controller** and tell the user to **enable exclusion mode on the end device** (maybe even show the steps for this from the device config)

* once the device is excluded (which disables the provisioning entry), **switch the protocol on the provisioning entry to LR** and **enable it again**

* inform the user that the device will now join using ZWLR again, maybe show an info when it happened

The steps in bold would be what you do by hand, FYI.

I think the entity renaming should be part of this process, too, to the extend it can be automated (node IDs embedded in entity names really suck).

AlCalzone commented 6 months ago

I think the entity renaming should be part of this process, too

This would need some kind of event for applications that carries the information that a node's ID changed

madbrain76 commented 6 months ago

@AlCalzone , Can any application actually store the node ID ? At least in HA, the value doesn't seem to be exposed through entities. So, how could any HA automation depend on it ?

The node ID is however embedded in some device entity names, unfortunately. Those entities should be renamed, if possible. It appears the HA developers don't want to add this feature, though.

AlCalzone commented 6 months ago

So, how could any HA automation depend on it ?

I thought it does indirectly through the entity names?

Those entities should be renamed, if possible.

This requires that HA knows which node has changed IDs, which is what I'm referring to in my previous comment.

madbrain76 commented 6 months ago

@AlCalzone Yes, indirectly indeed. What I meant is that you can't create an automation that queries the Node ID, and take some action based on its value. Or print the node ID value. Or any other number of things you could think of in HA. The only thing HA has access to is the entity name, AFAIK . I don't know if it's possible for an automation to get a string for an entity, process it, and extract the node ID from that. Maybe it is - I am not an HA developer. If so, it complicates things.

Your concern seems to be notifying HA of the entity name change, but the more problematic one is that HA doesn't currently support renaming entities at all. The only supported way is "use VSCode and do a global search/replace". This is what I was told yesterday in this thread :

https://community.home-assistant.io/t/wth-is-there-no-refactor-functionality-when-changing-an-entity-name/467196

Unfortunately, it doesn't look like it's going to be solved any time soon :-(

The only choice might be to fully preserve all the entity names during migration. The entity names would no longer match the Node ID. Ugly, but it is unlikely to breaking anything ...

rohankapoorcom commented 4 months ago

Sorry, I'm going slightly off topic, but one of the points from @madbrain76 caught my attention:

FWIW, I have seen my Smart start entries completely disappear. I'm not sure at which point they did, but it happened. This is part of the reason that motivated me to write the program I did to recover them all.

Looks like you ran into https://github.com/zwave-js/node-zwave-js/issues/6755. Would you be able to share your script for parsing and recovering that data? We can move the conversation to that issue.

madbrain76 commented 4 months ago

Sorry, I'm going slightly off topic, but one of the points from @madbrain76 caught my attention:

FWIW, I have seen my Smart start entries completely disappear. I'm not sure at which point they did, but it happened. This is part of the reason that motivated me to write the program I did to recover them all.

Looks like you ran into zwave-js/node-zwave-js#6755. Would you be able to share your script for parsing and recovering that data? We can move the conversation to that issue.

See https://github.com/zwave-js/node-zwave-js/discussions/3910#discussioncomment-9545852