Repository navigation
vNext: Change bind procedure - #546
Merged
RobHofmann merged 10 commits intoOct 8, 2026
Merged
Conversation
Because the protocol does not guarantee an alignement of requested properties and returned values, make the first time fetching properties to query a single property at a time to ensure a 1:1 mapping of their values. This is important because the first fetch defines device capabilities
This allows for a single controller to be bound once. In a VRF setup, consecutive subunits reuse the same binding info as the first unit
Before, MAX_UNANSWERED_IN_A_ROW with error_as_missing set to True would mean that after MAX_UNANSWERED_IN_A_ROW the remaining props being queried would be marked as missing. However MAX_UNANSWERED_IN_A_ROW should be a error check for when the device stops responding alltogether, meaning a device is not supposed to not respond to a prop for more that MAX_UNANSWERED_IN_A_ROW. If it does, it should be raised as an error and break whatever thing was using query_props.
…ng after MAX_UNANSWERED_IN_A_ROW Instead of assuming that after MAX_UNANSWERED_IN_A_ROW the device is unresponsive, do a last check with STATUS_CANARY_PROP and continue if device responds to that.
This allows multiple devices sharing the same controlelr to reuse the same binding information and have all devices updated when a single device in the controller updates the binding.
…ommunication instead of relying on MAX_UNANSWERED_IN_A_ROW to signal a failure
Allow batching since this does not seem like a problem. No real/world regists of it failing alignement in fetch status, only in set status.
- DeviceApiClient called add_device and remove_device with the device and controller MACs swapped. For a VRF sub-unit, connections was keyed by the sub-unit MAC, the listener sat on the gateway MAC, and bound was always False, so every poll sent a new bind request (2 sub-units: 5 binds at setup, then 1 per poll). The calls now use keyword arguments. - set_controller_binding_info used setdefault, so a forced rebind kept the old key while another sub-unit of the controller was connected. A new binding now replaces the old one for every device of the controller. - The cipher property never returns None, so the two None checks on it were dead code. They now check binding_info. - Small fixes: loop without enumerate, two typos, query_props docstring. Tests: the MQTT transport tests and FakePushTransport use the new add_device/remove_device API. The sweep breaker test is replaced by two canary tests. Four new tests cover the shared bind per controller; all four fail on the previous commit. FakeGreeDevice gets an ignored_props keyword. Docs: protocol.md describes the canary and the bind per controller, architecture.md and actions.md drop MAX_UNANSWERED_IN_A_ROW.
Without error_as_missing the error is raised either way, so the canary only added up to 2 x PROBE_TIMEOUT and an "Assessing if device is responding" warning to every poll of a unit that is switched off. A normal poll now raises at once and marks the device unavailable. The canary stays for error_as_missing sweeps, where its answer decides between "ignored prop" and "device gone". On my test unit, get_prop_values with ElcDatDte and ElcDatHor: both time out (10 s), the canary is answered in about 15 ms, and the sweep returns Pow, SetTem and Lig with the two ElcDat props as missing.
Fix VRF binding on the transport, update tests and docs
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 changes 2 things: