Add Bridge Application Command Handler Class #1838
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was extracted from #1765, updated with additional comments, and the removal unneeded file changes. With his PR 700 controllers will work with Openhab. It is not perfect or technically complete, but will buy time and suppress the crazy talk on the forum. I'll add supporting information in the comment section. Signed-off-by: Bob Eckhoff <[email protected]> Bridge Application message Class file? After several rounds of testing it does not appear the class file is needed. All of the bridge responses are handled in the transaction manager. The same appears to be true for the Application Command Message Class, but I just left that. In all the Debug level logs I have looked at (granted not all inclusive), I did not see any instance where the debug messages in these classes appeared. The may be other reasons they need to be included of which I am unaware, but I'll put this out there for comment. I'll attach a debug startup with the changes in this commit. Signed-off-by: Bob Eckhoff <[email protected]> Remove Controller from Network Heal I will probably need “re-education” as to why the controller is included in the first step of the network heal. On the surface it only seems to provide the controller neighbors for its XML and feed the network diagram. It looks to me like the controller is kicked out by the stage advancer after the first step because “Controllers aren’t designed to allow communication with their node” (comment line 393). Even with 500 controllers healing the controller creates a lot of network routed errors and unanswered messages. (See attached-extracted node 1 to node 1 messages only) However, eventually enough messages are ACK’ed so a new controller XML is created. However, with my testing of the 700 controller this no longer works at all. I started a network heal with the 700 controller and all the nodes, except the controller were “healed” in a few minutes. I could see that none of the controller to controller messages were ACK’ed. Twenty two hours later, about 30 minutes apart the controller is still trying (see this and this). Anyway excluding the controller resolves the 700 issue and also shortens the 500 controller network heal by 4 minutes. Also (IMO) without the controller involved, the 500 controller post-heal routings appear less scrambled from where they started (more direct communication between the device and the controller). My speculation is that since mostly multiple hops are ACk’ed (1 -->device --->device--->1) that there is a bias towards the “healed” path being at least one hop. Over time (without network heals) and with individual node heals, direct paths are restored. I did notice that the frames a 500 controller and 700 controller send to “Find Nodes” are different. (see pictures). Without neighbors on the controller it will not appear on the network map. It could be restored manually by adding neighbors using the API explorer. Signed-off-by: Bob Eckhoff <[email protected]> Change class Name for better match Changed the class name to match the API documentation also following the pattern for Application Command handler 0x04 Signed-off-by: Bob Eckhoff <[email protected]> Changes so Transaction advance will work with 700 Although the previous versions all worked there were some loose ends. This 1) does a more accurate strip of the extra byte in the transaction manager 2) modifies the serial message to pick up the dest node from a Bridge message, and 3) set the Bridge (A8) as the expected reply class, so the transaction advance will work like before (no need to set complete if Bridge message). Only problem right now is this will only work for 700, so is not mergeable unless a separate branch. Will work on that.. Need to find a way to get a 500/700 switch in the transaction payload. Signed-off-by: Bob Eckhoff <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was extracted from #1765, updated to OH4.0, additional comments added, and unneeded files removed. With this PR, 700 controllers should work with Openhab. It is not perfect or technically complete (explained later) but will buy time to improve the code to your standards and suppress the crazy talk on the forum. Based on information on the Home Assistant site and google, Version 7.17 or greater should be required (earlier versions seemed to have problems). I started with 7.17 but upgraded to 7.18 for testing
Below is a picture that the Zooz 700 Controller set as a Static PC Controller. (This can only be set with a blank stick, but mine came this way out of the box), and three logs of initial tests of startup, inclusion, exclusion, and Heal from a working jar created with the changes in this file. I also have the jar with these changes working on a RPi3b with an older Aeotec ZW090 controller to validate no interference with the current installed base.
Zooz-700-PC Controller.pdf
zwave-node2-addand delete.log
zwave-node3-addand heal.log
zwave-node4-add.log
The "not perfect" comment is because one of the extra bytes (Destination) in 0xA8 versus 0x04 is simply dropped and the others (one minimum) regarding Multicast Destination Node Mask ignored. You would know better but my speculation it for a multiple controller environment where the bridge controller could create a new message to forward to another controller. However, I believe these bytes are remnants from the Z/IP idea and not currently needed.
My last observation is the ZWave viewer will need to address 0xA8 with the same parameters as 0x04. Edit 1: I'm assuming this is not open source? If it is I could look to see if I could figure out what needs to be changed for you. Edit 2: Found source and adjusted for 0xA8
Edit 2: Did a compare of the Application Command class with the Bridge Application Command class. Edit: Not relevant in latest commit !
Compare Bridge to Application Command.pdf
Edit 3 for Third commit: Files referenced in the commit commentary are zniffer . Converted and shortened to excel files
Here is the 500 controller Network Heal (today's binding)
Controller 500 Network Heal.xlsx
Here are the 700 controller network heal 22 hours after other nodes have healed
Controller 700 Network Heal issues.xlsx
Here are the two pictures I referenced in Edit 3 related to the Heal. First is 700 and second is 500, both Zooz controllers.
EDIT 4: Here is a debug level start with the last commit without a separate BridgeApplicationCommandMessage class. This works on my Rpi3 and Rpi4. The only hiccup (and I believe is unrelated to the code) that I see it that node 24 "sent data" while the binding was still "Waiting Request" mode, but it was sorted out in a few seconds.
Could be ready to others to try as beta test, but I sense you want to write your own.
startupwithPR version-rpi3.txt
Signed-off-by: Bob Eckhoff [email protected]