@chris would know for sure.
Iām not sure - itās not a feature that has been requested very often and Iāve not spent a lot of time looking at it so I donāt really remember where itās at.
That sounds this part is OK. I am guessing if you trace the messages it will send it but get back a message saying nothing changed. I think the fix might not have made it into the binding but canāt be sure without pulling the latest code but I would have to mess a bit to do that.
It has worked. This device is a bit odd as the inputs can be configured in various ways and this impacts how they work with the outputs. Also not all possible protection values are supported by the device.
Unfortunately I do not use this specific version, because I thaught the change has already been included to the official version.
Iām sorry for that.
Just a question to @chris: Is it planned to merge the change with the master branch?
regards,
Oliver
I know Chris is working through a 3 month backlog of changes due to the new build system. I just wanted to warn that even if this is planned it may take a while to actually happen.
I donāt see any open or closed PR for that:
https://github.com/openhab/org.openhab.binding.zwave/pulls
Without a PR, no fix
Apparently, there was an issue opened, but it then needs programming to generate a PR.
I thought this was already done:
If it was just a dirty hack, Chris would not accept that change anyway. But it does not help anyone if changes to the code are not published to the official zwave repo.
I think if you read the thread @rmichalak tried but was not sure how it should all be submitted.
As to hack if you mean in the sense that @rmichalak used his skills to make it work then fine if you donāt like his style then I am not sure how to respond.
In my book something that works is always better than something pretty that serves no purpose. His code works. The current code may be more beautiful but does not support this version of the command class. I was very impressed he spotted the issue.
Please be careful not to wrongly attribute quotes. That is not my words or claim. I totally failed to find the missing bit in the code and fix it. All credit to @rmichalak but possibly if it is not in the release or submitted someone could help by explaining what needs doing or confirm it is just that Chris is snowed under and it will arrive in time.
Yes, that is what I meant.
I have no programming skills so maybe I used the wrong words
It is just the word hack has gained various connotations over the years. It sways between a respectful term indicating great skill and a derogatory term implying not nice depending on context.
As i say the issue was not easy to spot and @rmichalak admitted he had no deep knowledge of zwave. To find it and fix was very cool in the time he managed to do it.
I am not saying there may not be an even better fix but that would require someone that knows more about the binding and the fix does work so might be of interest to people in an interim release even if it never makes it to a full.
Iām not sure what change you are asking to merge? Can you reference this please?
Hi Chris,
It is the one made by @rmichalak to add support for version 2 of the protection command class. He may have raised the request or possibly it is sat to one side needing pushing over to you for review
.
Itās not about style necessarily, but there does have to be some code control, and we canāt allow ānasty hacksā into the binding or it will become unreadable and unusable.
Maybe, but it needs to be coded properly - it canāt be a case of adding āanythingā even if it works.
As nobody has reviewed it it could be hack in the good sense
Can you provide a reference please? I donāt see any open OR about the protection class?