We migrated Z-Wave to be based on pip. However, it looks like the network key used to be saved in the Python Z-Wave directory which is now part of the package. That means we'll lose the key on every upgrade causing people to have to pair their secure devices again.
To fix this, we need to add a config parameter to the Z-Wave component that we pass as the Open Z-Wave option NetworkKey to Python Open Z-Wave.
We pass options to Z-Wave network here.
@robbiet480 you were looking into this. Do you have a branch? If not, please note it here so someone else can pick this up.
This needs to be fixed before 0.45.
CC @armills @andrey-git @turbokongen
Currently python-openzwave will read an options.xml file in the hass config directory, presumably from the user_path we set in the ZWaveOption object. Does this not work with the pip version?
We might just need to update the docs telling people to copy the file to their config directory before editing. This would be preferable since it allows anything else in options.xml to remain configurable.
On a related note, someone submitted a PR to fix the PyOZW docs to note the network key can be passed in like I am doing here.
There is code to copy options.xml from config path to user path if the file is missing from user path. I assume it is read from user path?
@armills is up to something. Maybe we should automatically copy options.xml to the Home Assistant config dir if it doesn't exist?
options.xml needs to be inside the config dir
Okay, now I am confused. I was under the impression that somehow Python Open Z-Wave stored the key in the config files that are stored inside the Python package. Is this not the case ?
PyOZW just clones the OZW config directory into its package directory. options.xml exists inside of that OZW config directory. Here it is.
I guess then we should do the same ? And use user_path to point PyOZW at that. We default now to the config dir btw but I think we should move that to a new z-wave folder.
From reading ozw code I understand the following:
When locking options, ozwpy copies options.xml from config dir to user dir if it was missing in the user dir: https://github.com/OpenZWave/python-openzwave/blob/master/src-lib/libopenzwave/libopenzwave.pyx#L745 this was added April 26th
Then ozw cpp code first loads options from config dir, then user dir, then command line: https://github.com/OpenZWave/open-zwave/blob/master/cpp/src/Options.cpp#L371
Thus the only problem is for user without options.xml in user dir switching from version 0.3.x to 0.4.x
If possible wouldn't it be better if we could keep the key in the main HA configuration file?
@Landrash It has advantages, but also has a downside of being unable to use secure devices with other programs, like ozwcp.
Agree with @andrey-git. Another downside to using configuration.yaml is that network key is only one of the options in options.xml, and it would be better to keep all of them accessible. options.xml was always accessible from the user path. That's how I've always used it since I run with docker.
Is it possible to add some transition logic that looks for an options.xml to copy in the old directory, if there is none in the user_path?
So if I get it correctly then, are we all good for this release and we don't need any changes?
(I'll be on PyCon from now till release so I need someone else to take the lead on this issue)
We need to write in huge bold red letters that people should back up options.xml (once when switching to 0.45)
I don't like #7637 myself, as we're now giving users two different places to configure the network key. If we are going to require user action for the transition either way, I'd much more prefer that we just ask them to move options.xml into their config directory, where it really belonged in the first place.
It also sounds like going forward, python-openzwave is going to automatically copy a template there if one doesn't exist, so this will be the long-term expected practice.
Hopefully OZWCP wont be needed for the wast majority of users so the option in #7637 makes sense in a HA standalone way.
Having the user having to copy a file that may or may not be in a specific place is a bad work-around and one should always strive for the minimum amount of needed user interaction.
It's not a workaround though, it's the normal way zwave will work in the future. python-openzwave is already going to copy the file there automatically whether we like it or not, including a placeholder to set the network key. For a new install after 0.45, users will see options.xml right in their config directory next to zwcfg.xml, so it will be obvious where they should be editing it. No one should have been editing the file outside of the config directory to begin with.
Also, if we start pushing options.xml configuration parameters into the hass config, soon we'll end up with everything from options.xml in configuration.yaml. The network key is just the most currently visible option. I'm just of the opinion that duplicating upstream logic is just setting ourselves up for maintenance issues and breakages. We should be taking advantage of upstream code wherever possible.
And on a final note, users already are going to have to find that file that may or may not be in a specific place, because they're the ones who were editing it in that place. They'll need to copy the network key out either way, so they may as well just copy the file to where it belonged in the first place. Copying the file to the config directory is a much simpler request than asking them to extract the info from an xml file and add it to their configuration.yaml.
Ahh don't misunderstand me. I'm personally not a fan of of having external config files if they are not needed for the basic users since it only increases the support amount others will need to do.
If it possible to do both I don't see a problem with either.
@armills you're right, I like that more. Have them copy that file an we're all good. Let me wip up a quick PR.
Okay, don't have time to do a PR right now . Will leave the config option in for now.
We should also update the user path to be <config>/zwave. Should we copy the options.xml over when the user has none ? That would be the easiest. PR to fix this welcome 馃憤
I did some digging, since from the lines @andrey-git posted we would expect to see an options.xml file in the user_path we provide. It turns out there is an upstream bug that was preventing this from occurring. I've submitted a fix upstream. https://github.com/OpenZWave/python-openzwave/pull/82
I think that after this is merged, and we can get the zwave user path in it's own subdirectory, it should be much clearer.
@armills why does your fix help? In HA we set the user_path explicitly, so shouldn't the value in both ZWaveOption and libopenzwave.PyOptions be the same?
The user path was being reported correctly, but the calculated config_path was lost after this line executed. Once it was set as a python attribute, the cython variable became inaccessible. That means that this line was never able to execute successfully, even though it was properly loaded from the correct path. (This is at least how I interpreted the behavior I saw. I am by no means a Cython expert.)
HA was always able to load options.xml from the user_path, but python-openzwave didn't copy the template there, which caused a lot of confusion.
Most helpful comment
We need to write in huge bold red letters that people should back up options.xml (once when switching to 0.45)