Clean 700 Controller PR

[apella12]

This replaces #1848 that had an orphan file.

For those willing to test, I have a jar based on 4.1 snapshots on my github page (under latest release). I have had these changes working with zooz 700 controller for close to nine months and a few others have tested it (no complaints so far)

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

[lsiepel]

Would it make sense to also look at #1765 and #1370 and see if those changes can be incorperated into this PR? Or does this PR make those obsolete?

[apella12]

I started with #1765 and improved it to follow the same logic as the current binding and #1370 is outdated based on the recent SDK 7.18+. 700 controllers can be setup as static, not gateway.

[apella12]

Thanks appreciate your comments! Only regret is that they weren't closer to when I originally proposed this is February. Two months ago, I moved my OH test and production installations over to zwave-js-ui. I do have a zooz 800 controller on order, so can spin up an OH ZWave binding test environment on WSL next week with a device or two.

My main concern on a quick read is the two controller's comment. As history, I started with #1765. That PR just sets the Transaction complete in the ZWaveTransactionManager, if "Bridge" (0xA8)

                                                if (incomingMessage.getMessageClass() == SerialMessageClass.AppCmdHandlerBridge) {
                                                    logger.debug("NODE {}: zwave700 dbg => cmd_id {} cmd_class {} replay_class {}.", 
                                                            nodeId, command.getCommandClassId(), command.getCommandClassCommand(), 
                                                            transaction.getExpectedReplyClass());
                                                    transaction.setTransactionComplete();
                                                }

That is probably the easiest way to address your comments as I don't need the controller information. WDYT?

The "normal" way to complete WAIT_DATA is in the ZwaveTransaction class

            case WAIT_DATA:
                if (incomingMessage.getMessageClass() != payload.getExpectedResponseSerialMessageClass()
                        || incomingMessage.getMessageType() != SerialMessageType.Request) {
                    break;
                }
                // Check if the nodeId is correct
                if (incomingMessage.getMessageType() == SerialMessageType.Request && payload.getDestinationNode() != 255
                        && payload.getDestinationNode() != incomingMessage.getMessageNode()) {
                    break;
                }
                // We've received the data we wanted - we're done
                transactionStateTracker = TransactionState.DONE;
                break;

To use that code additions/changes were needed to switch the ExpectedResponseClass for 700 controllers. I don't see off the top how to dynamically change the ExpectedResponseClass when both 500 and 700 controllers are sending SendData messages. I have never had a setup on one binding with two SendData controllers for testing. Is that really that common? Could we just note that it is not a supported case? Wall controllers (or key fobs) will work fine because the usually just send a basic set or scene number.

[cdjackson]

Only regret is that they weren't closer to when I originally proposed this is February.

Sorry - this year has been hectic and I've only just got some time off.

Is that really that common?

I don't know, but I would say it's not likely to be unheard of that someone has an old controller, and then buys a new one to extend a large network (or just play with a separate network for some reason). I think excluding this as a use case is going to cause problems pretty quickly.

How about a slightly different suggestion. Instead of handling the difference between the two application classes in the receive transaction processing, do a conversion in the receive thread and convert the bridge class to the application command handler class? I've not looked at how easy this is - I guess it shouldn't be too difficult, and you already have some code there to handle the differences. This would make the receive side at least agnostic to the controller types.

I'm not sure if there is any code to send different class types - I don't see any, but I only did a quick look.

[apella12]

After my write-up I also decided it was going to have to be on the receive side (someway). I'll see what I can do to change it where I correct the message node for the Bridge and take out all the controller stuff.

[apella12]

Was able to test with my spare 500 controller (sending (0x04) and the BridgeApplicationCommandHandler as the default Expected Command Class. Reversed (actually left it the way it is now) the default Expected class in this PR to ApplicationCommandHandler since the OH community is starting with those devices but allowing for BridgeApplicationCommandHandler messages to advance from WAIT_DATA to DONE. Still waiting for my spare controller to test the PR, but should be okay as it works in reverse. Also the original PR had 7 months of testing by me and a few brave souls without an issue, so I'm optimistic.

[cdjackson]

Thanks - this looks fine to me. Can I just ask you to run the formatter over it please? If not I'll do it post merge...

[apella12]

Thanks ! I did not know about the formatter, but found a reference in the forum. The only complication is that it returned changes in 40 files, so I discarded the 36 not related to this PR so as not to confuse the situation. I hope that is ok.

[cdjackson]

Thanks @apella12 for your work on this and sorry again it took sooo long...

[cdjackson]

Thanks ! I did not know about the formatter, but found a reference in the forum. The only complication is that it returned changes in 40 files, so I discarded the 36 not related to this PR so as not to confuse the situation. I hope that is ok.

Yep - thanks. I think the formatter has changed over the years, but this is probably. the best approach.

[apella12]

As to the time, you have to pay the rent!, so no worries. (Although, I had wondered if there was some side deal with the German central committee to put the binding on life support (DB updates only)).

I don't see a major improvement with 700 chips vs 500 (a few more one hop devices), but as the year went on it was hard for new users to get a 500 controller, so it was likely keeping them from considering OH. This will help that.

Again thanks for your time.

Bob

[cdjackson]

No - no side deals :)

This year has just been a bit hectic and I've spent quite a bit of time abroad (mostly US) which makes it difficult to do a lot due to both lack of time and equipment. Who knows what next year will bring....

[apella12]

If you are ever in Hartford, CT (Pratt Whitney) send me a PM and we can meet for a beer. :-)