Auto update zwave_js device config files once a day #71850
Conversation
|
Hey there @home-assistant/z-wave, mind taking a look at this pull request as it has been labeled with an integration ( |
raman325
changed the title
| assert client.driver.controller | ||
| entry.async_on_unload( | ||
| async_track_time_interval( | ||
| hass, async_check_for_config_updates, timedelta(days=1) |
There was a problem hiding this comment.
Should we check once when setting up the integration too?
|
This should be configurable so users can opt-out. |
|
Please give an example why that's needed. |
|
I don't think it's acceptable for HA to modify an external application, that I've installed myself, without my consent. Therefore I should have the option to disable it. My preference is to manage software updates manually, especially since I run a standalone installation of zwavejs2mqtt, which already provides this capability (with a method more compatible with my maintenance procedures). To re-iterate, I'd rather perform the updates on my own terms, rather than being forced to do so. Given HA's user base, I doubt I'm alone with this preference. I also don't trust the process to not break my installation, as it has in my personal experience (that's all supposedly fixed, but prove it first!). I'll ask the contrasting question, what reason is there for not allowing this functionality to be disabled? I'm also curious how this has been tested, given there haven't been any available updates in almost 4 months. Have you considered all the risks? As I mentioned, in my experience it's a fragile and invasive process that needs more road time to prove its reliability. Not allowing the process to be disabled means it's impossible to recover and avoid any unexpected failure scenarios. Adding a configuration option would be relatively trivial and satisfies both of our concerns. Enable it by default if you want. I can provide much more if the above isn't enough. |
|
I don't think we should add an option. Either we think it's safe and do it or we don't think it's safe and don't do it. |
|
third option is we go back to using an update entity... Users that want it to be automated can make it so, users that don't have the option to update. |
|
It looks like the current method to update within docker just downloads a gzip file and extracts it. So it shouldn't be as bad as I remember then. However, this still requires the zwave-js installation to be configured properly with the external config directory. |
Are you asking what testing we've done? The answer is none, I expect that if the driver offers this as a feature, that it's stable and doesn't need to be tested. |
|
Just offering an alternative viewpoint, as usual. Hopefully my pessimism is proven wrong. I hope this change means that the config update story is going to get some attention from Z-Wave JS and HA. The experience now is not good. |
|
I'll mirror what I said on Discord - there's not much we can do to improve the experience, we just have to decide whether it's manual (update entity) or automatic (no entity), and if it's automatic, whether it's configurable or not. If we do make it configurable I would suggest we still use an update entity then because we can use its auto update feature |
|
I've probably convinced myself that an auto-update is not the end of the world. I do worry about edge cases though. Expanding on my comments about the experience not being good, I am speaking to both about Z-Wave JS and HA, not just HA. The driver docs give us minimal guidance about what to do after an update.
As a normal user, if I see a log message about a config update, am I supposed to do something? According to those docs (which HA users should not need to read):
Since I (as the user) don't known what happened, I'll either ignore the updates, or have to re-interview all of my devices. That's not likely. If these required actions aren't taken, then no benefits are gained with the updates. Also, the message being in the HA logs probably means it'll be missed anyways. I guess if the main goal is to support new devices, it helps. To make the best use of config file updates, there needs to more focused notifications about which devices are affected (limited by the driver here), or the driver needs to automatically take action. Nothing HA can do about in the meantime. My point in highlighting this isn't to push back on this change, but rather to see it be improved. I would hate to see either this (or the update entity) get added, and then nothing else happen later. |
|
I agree that it's not a good user experience if the user is required to interact with the device or interview it again after an automatic config update and we don't inform the user explicitly about what to do. By keeping config updates synced with a specific zwave-js release it's at least possible currently for the user to read the release notes and search for device names that they own. |
|
Ok so should we close this for now and revisit it if/when we can provide more information to the user? |
|
I'm leaning to that end, yes. |
Proposed change
Sets a 24 hour interval check for config file updates, and if updates are found, they will be installed and a message will be logged
I also removed the WS API commands I added for this functionality since they will no longer be needed
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: