-
-
Notifications
You must be signed in to change notification settings - Fork 33.8k
Allow setting Z-Wave network key via options #7635
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
Comments
Currently python-openzwave will read an 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 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 |
From reading ozw code I understand the following: When locking options, ozwpy copies 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 8000 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 Is it possible to add some transition logic that looks for an |
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 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. |
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 Also, if we start pushing 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 |
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. |
@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 |
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. OpenZWave/python-openzwave#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. |
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
The text was updated successfully, but these errors were encountered: