Handle Housekeeping Messages Sent to Modem During Scene Command #183
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
Currently we just ignore these messages and assume the resulting state of devices based on the ultimate AllLinkStatus message. This is wrong for 2 reasons: 1. If there are a lot of devices in a scene, the length of time that elapses from initiating the command until when the AllLinkStatus message is received may cause a timeout. By handling and returning Msg.Continue, we tell the handler to stay alive and that the command hasn't timed out. 2. If the AllLinkStatus is NAK, it just means that one or more devices failed to send a CleanupAck. So if we don't handle the cleanupAcks, we erroneously assume that all devices in the scene failed and we update none of them. By looking at the Acks we can update the correct ones appropriately.
This is now the second time I have fixed this error caused by a typo in the horrible Insteon Documentation. See this commit from nearly 7 years ago: krkeegan/misterhouse@f314d16 I short, the insteon documentation for 0256 messages is wrong. There is no byte 3 that is always set to 0x01.
Catch as a valid response to an ALL-Link Broadcast, but report as an error.
Cmd_2 possibly refers to the failure reason.
Appears to not be needed, the modem seems to retry even though it reports a NAK. But for reference in the future, we are able to send Cleanup messages without sending the scene command.
More stability improvements. We know after a broadcast that the device will send a number of messages based on the number of linked responders. We can calculate the proper wait time and delay sending anything during this time to avoid collisions.
|
I pushed a few more commits that improve ALLLink commands even more. Notably I did a lot more research on the Modem Scene process and most importantly, is that an InpAllLinkStatus NAK is not the final attempt by the Modem. Additionally, we should increase the wait_time even when we receive garbled messages. Second, I also added a delay after a device sends a Broadcast message. We can calculate how many messages the device will send based off how many responders are present. We can then cause a delay to avoid colliding with those messages. All of this has resulted in a much more stable and accurate network when sending long and complex ALLLink Commands. |
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.
Solves issues with:
Timeouts because of a lot of AllLink Cleanup messages coming in.
Better handle failed scene commands by changing the successful devices.
Properly warn users about failures with error messages.
Ironically, this includes fixing a bug that is caused by an error in the Insteon documentation. The same bug I fixed almost exactly seven years ago in MisterHouse. Seven years working on Insteon, ugh, I would never have expected that.