openhab / org.openhab.binding.zwave

openHAB binding for Z-Wave
Eclipse Public License 2.0
170 stars 202 forks source link

Zwave 700 controller changes #1848

Closed apella12 closed 7 months ago

apella12 commented 1 year ago

Had some struggles with the maven compiler with a “spotbug” missing resource that I originally thought was something I did, so I deleted my fork (in frustration) which closed my previous PR. I also seem to be unable to get rid of the zerodim2pol_0_0.md. Even after deleting my fork and starting over, it pops up as a change. As to the main content:

I have this PR binding working on a 24 node Rpi3 and a 34 node Rpi4 with 700 controllers on SDK 7.18. I have also tested it successfully on a Windows Linux partition with my old 500 controller and one battery node. As noted before, I started with the other 700 controller PR, but have pretty much replaced all the code. The other PR worked, but short-circuited transaction advance process. A recap;

  1. SerialMessage - Two changes: Added the BridgeApplicationCommandClass (A8) and changed the Node destination byte from 5 to 6, if the message is "Bridge" A8.
  2. ZWaveController - One change: Created a public static integer from the controller initialization process to identify a 700 controller (Library =7). This is used in the ZwaveCommandClassTransactionPayload class as noted below (number three).
  3. ZwaveCommandClassTransactionPayload class - One change: Using the Library integer, I switch the Expected Response SerialMessageClass between the BridgeApplicationCommandHandler and the ApplicationCommandHandler so this will work with both 500 and 700 chip controllers.
  4. ZWaveTransactionManager class - Two changes: As in the other 700 PR both the “Bridge” and “non-Bridge” are now allowed options. However, I reworked the byte removal to remove the specific extra byte. With one less byte the “Bridge” message can be processed like the “non-Bridge” message is currently.
  5. Zwave Transaction class - No changes: With the destination node (serial message class) and reply class (TransactionPayload) matching the commands proceed through the stages to DONE.
  6. Zwave Controller Handler - One change: As reported earlier (in my closed PR) the 700 controller can't heal itself, so I excluded it from the network heal process. I could import the Library method so as to keep that for a 500 controller, but from the zniffer logs provided earlier, the 500 controller doesn’t heal very well either.

Signed-off-by: Bob Eckhoff katmandodo@yahoo.com

lsiepel commented 1 year ago

@NilsBohr @spalish did you also happen to use/test this change? Both of you have 700 series related PR pending, was wondering if this can be combined (if needed). So maybe we can get this going, would also be nice if @cdjackson can comment on the 3 PR's if not allready done.

apella12 commented 1 year ago

I have speculated for several months on the lack of comment. The SDK 7.x has evolved greatly since its original inception with Z/IP and the versions 7.17+ can be made to work like the 500 chip z-sticks (I’m using this PR on an RPi3 & Rpi4 with Zooz 700 Zsticks (SDK 7.18). Possibly the issue is the handling of the extra byte in the 0xA8 message versus the 0x04 message. From documentation; Bridge controller library-This library is intended for controller nodes that are able to allocate more than 1 NodeID to themselves and use them for transmitting/receiving frames.

So the extra byte in 0xA8 is the “destination in a multiple node ID environment” but this is redundant in the way OH currently works, so both the recent 700 controller PRs deal with this extra byte by deleting it/bypassing it. That is probably not how he would do it. So both PRs work (I started with #1765, but changed things to better use the existing 500 series flow).

The only other item that may be an issue is that I exclude the controller from the network heal. The network heal does not work with the 700 controller. Anyway those are my speculations.

I would welcome any testing. Except for the controller network heal (and I could probably put that back for 500 controllers) there is no effect on 500 controllers using 0x04 (I also have the PR running in a WSL environment with a 500 aeotec Zstick). The 700 zstick would have to be configured as a static controller and the SDK needs to be 7.17+ Zooz-700-PC.Controller.pdf

Phalen commented 1 year ago

Is there currently support for the 700 series in mainline or is that pending these changes? also if a tester is needed please let me know.

apella12 commented 1 year ago

Is there currently support for the 700 series in mainline or is that pending these changes? also if a tester is needed please let me know.

The "main" or any of the branches do not have 700 controller support, but 700 chip devices work fine with the 500 controllers and any of the current bindings.

The key review is the developer, but personally I would like to see if these changes work for more than just me (and try to fix any issues), so please test. My main uncertainty at this point is with securely included devices. I hope it is okay, but do not have any to test.

Phalen commented 1 year ago

The "main" or any of the branches do not have 700 controller support, but 700 chip devices work fine with the 500 controllers and any of the current bindings.

Unfortunately until I can get the controller working I cant do any testing. sadly the 500's new are 60+ and a 700 is 25$

apella12 commented 1 year ago

Unfortunately until I can get the controller working I cant do any testing

I was thinking that was the first test ;-). Do not know your setup, etc. but since you are on github, fork the binding to your private repository and make the changes. There are only about 50 lines over 5 files. (Ignore the .md file that was a github burp.) I use the github desktop and VSC as an editor on a windows machine that I have installed maven. Once changes in place use mvn clean install in a terminal session and the new jar will be in the target directory. Ironically the 700 controller is starting okay, you just can't add devices without the bridgeapplicationcommandhandler CC

erikklavon commented 1 year ago

I applied these changes and they resolved problems I was having adding new z-wave devices.

I am new to Z-wave and OpenHAB, having started to use both a couple days ago. I am using a Zooz 800 Series Z-Wave Long Range S2 USB Stick ZST39 LR with a Zooz 700 Series Z-Wave Plus Smart Plug ZEN04 and OpenHAB 4.0.0SNAPSHOT. The Z-wave binding had no trouble finding the ZEN04, however OH did not recognize the type of device, marking it as unknown. Modifying the code with additional debugging statements, I determined that the node's Manufacturer ID, Device Type and Device ID fields were empty - this is why the discovery process could not identify the device. I tried hard coding these values for my specific node, and that worked around the issue. Looking at the logs to further understand why these values were not populated, I noticed OH sending Command Class ManufacturerSpecific requests to obtain this information. It appeared that the device was responding, but the reponses couldn't be processed because the type (168 or Hex 0xA8) wasn't supported by OpenHAB. As a result the Command Class ManufacturerSpecific requests timed out in WAIT_DATA state. After working around this issue by hard coding, one of the other problems I noticed was occasional latency between when I issued REST commands and when I got a response; it seemed these coincided with when OH was waiting for responses which were type 168 / 0xA8. Using a clean copy of the repo without my prior changes, I applied apella12's commits, complied the binding, then copied it into the addons directory. I then wiped my install's cache and user data, so that I started with no prior state. Walking through the setup process, I saw in the logs that the Command Class ManufacturerSpecific queries were getting respoonses so that OH now was populating the Manufacturer ID, Device Type and Device ID fields. After applying these changes I was able to add my ZEN04 device successfully without needing to hard code data for these fields. Additionally the increased latency events are no longer occurring. Thanks very much apella12 for making these changes; I hope they become part of the distribution soon.

apella12 commented 1 year ago

Thanks very much apella12 for making these changes; I hope they become part of the distribution soon.

You are welcome. Thanks for testing. As you note, SDK 7.xx uses 0xA8 in place of 0x04 (500 chip) for "unsolicited" messages. There are three "switches" in the PR' (the .md file was a github burp and is unnecessary) so the binding will work with both 500 and 700 controllers. The 0xA8 CC has one extra byte that throws off the message meaning.

As my testing was on a Zooz ST10 700, I would like to see the debug level log of the ZWave binding startup (if you have one) with the 800 controller, just to see if there are any differences of concern.

A note of caution; Based on what I have read 800 devices with the 800 controller use a two byte node ID and will not work using the OH ZWave binding. That would require a major refactoring.

erikklavon commented 1 year ago

Here's full trace log output after restarting OH. Let me know when you've got a local copy so I can remove access. https://app.box.com/s/g3gltqkvie7bdug0cr14ar0eu8y862a6

Thanks for the note of caution; much appreciated!

apella12 commented 1 year ago

Let me know when you've got a local copy

Got it thanks!

edit: everything looks fine

openconcerto commented 1 year ago

Hi OpenHAB devs, what are you waiting to merge this pull request?

Phalen commented 1 year ago

I just upgraded to oh 4 and this patch immediately showed up. I was able to successfully add my 700 stick and it detected the ge enbrighten switch. The enbrighten switch is in the list and is currently showing as unknown in oh.