OpenZWave / open-zwave

a C++ library to control Z-Wave Networks via a USB Z-Wave Controller.
http://www.openzwave.net/
GNU Lesser General Public License v3.0
1.05k stars 918 forks source link

Add basic command class to Aeotec switches and dimmers #2517

Closed matejdro closed 3 years ago

matejdro commented 3 years ago

As discussed on https://github.com/OpenZWave/qt-openzwave/issues/60, only way to use get Basic events going forward is to add couple of flags to the devices that support this.

These three devices all support basic command independently from controlling the load (e.g. pressing the physical switch can trigger basic set independently of controlling the actual load connected to those switches) and thus I think it is useful for OZW to also report basic status independently from actual load status.

matejdro commented 3 years ago

I'm not sure why PR check failed. From what I see it is caused by another file that is not changed in this PR. Can anyone with more knowledge of this system help here?

abmantis commented 3 years ago

After getting this in Z2M, state reporting is completely broken for aeotec switches. When pressing the switch, on the log I get "No Valid Mapping for Basic Command Class and No ValueID Exported. Error?", and the state does not change.

kpine commented 3 years ago

All of the recent changes to XMLs like this basically break any software that isn't ozwdaemon (qt-openzwave).

abmantis commented 3 years ago

@kpine how would this work on qt-openzwave?

kpine commented 3 years ago

It wouldn't work there either to be honest. Can you change your switch to send multilevel reports instead of basic?

matejdro commented 3 years ago

Huh, sorry, I had no idea this would break everything that is not qt-openzwave.

@nechry should this be reverted or should other non-qt ozw instances be updated to support this?

kpine commented 3 years ago

@matejdro It's not really your fault.

The thing with these Aeotecs in particular is that, as you originally said, the Basic Report signals the stage change of the actual switch. The Nano Switch for example, can use Basic CC to report off/on, which is mapped to the Binary Class. With these changes the switch value no longer changes state, instead only the basic values do. While you want to use basic set for the non-load switches, the override affects all the switches.

Another example, the other similar changes to the GE switches affect other non-ozwd software (z2m, Domoticz, is there anything else?) that would expect to be using Node Events instead of values. Only ozwd is written to utilize this behavior.

matejdro commented 3 years ago

Okay, I see the issue now. It even happens on ozw-qt on my switches, so it's not limited to to the others.

Maybe the real solution here is to revert this change and make this into a per-device config option? That way anyone can pick which behavior they want from the switch.

kpine commented 3 years ago

As of now there is no way to personally customize devices in this fashion outside of modifying the XML file. My preference has just been to maintain my own local copies. You can do this permanently by setting the Revision number in the XML to something like 999, so OZW won't download a version to replace it.

The same can be done to quickly revert to the previous behavior if you have access the the config files. Simply download the previous version (such as this for the nano switch) and bump the Revision to 999, and replace the existing one.

matejdro commented 3 years ago

Then I vote to revert my PR. Adding support to a niche use case is not worth requiring to switch all notifications from Basic to Hail.