Gate motion binary_sensor publish behind a successful Security+2.0 sync (#118)#119
Closed
heythisisnate wants to merge 1 commit into
Closed
Gate motion binary_sensor publish behind a successful Security+2.0 sync (#118)#119heythisisnate wants to merge 1 commit into
heythisisnate wants to merge 1 commit into
Conversation
Motion is a Security+2.0-only feature; the Security+1.0 protocol has no motion command. gdolib's protocol probe can transiently run the Sec+2.0 decoder against a Sec+1.0 bus, where a corrupt frame can decode as GDO_CMD_MOTION, so a Sec+1.0 opener with no motion sensor can publish motion (#118, konnected-io/gdolib#30). Downstream this is unrecoverable: the cloud infers a motion sensor exists the moment the entity leaves its "unknown" state, so one spurious publish permanently marks a sensor that isn't there. Track sync and protocol state on GDOComponent and drop motion updates unless we are synced on Security+2.0. Defense in depth -- the parser-side fix belongs in gdolib, but the ESPHome layer should not forward a state it has no basis to believe. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Better fix is upstream in konnected-io/gdolib#31 |
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
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.
Addresses action item 3 from #118: gate the ESPHome motion publish behind a successful Security+2.0 sync, rather than binding the gdolib callback straight to
publish_state.Why
Motion is a Security+2.0-only feature — the Security+1.0 protocol has no motion command, and
decode_v1_packet()has no motion path. But gdolib's protocol probe can transiently run the Security+2.0 decoder against a Security+1.0 bus (gdo_sync_task()falls back to V2 emulation atgdo.c:911-929when the V1 handshake doesn't resolve in ~7.5 s), and a corrupt frame read in that window can decode asGDO_CMD_MOTION. That is how a Security+1.0 opener with no motion sensor ended up publishing motion. Full trace in #118.Downstream, a single publish is unrecoverable: the cloud infers a motion sensor exists the moment the entity leaves its
unknownstate, so one spurious blip permanently marks a sensor that isn't there — and it self-clears after 3 s, so it looks perfectly innocuous in the web UI.Before this change there was no gate anywhere between a decoded
0x285and an MQTT publish:binary_sensor/__init__.pybinds the parent callback directly toGDOBinarySensor::publish_state, andGDOBinarySensoris an empty class.What
synced_andprotocol_onGDOComponent, populated from the existingGDO_CB_EVENT_SYNCEDhandler.set_motion_state()moves out of the header and drops the update (with a warning log) unless we are synced and on Security+2.0.Behavior by case:
synced_/protocol_false/ unsettrue/ V1true/ V2Ordering is safe: in
gdo_event_handler,set_protocol_state()is called beforeset_sync_state()and only when the sync actually succeeded, soprotocol_is never populated from a failed probe.Scope
This is defense in depth, not the root-cause fix. The parser-side fix belongs in gdolib — filed as konnected-io/gdolib#30 (
update_motion_state()has no protocol guard). This PR only ensures the ESPHome layer won't forward a motion state it has no basis to believe. Note it does not address spurious motion on a genuine Security+2.0 opener with no motion sensor — on such a device the gate is open by construction, and that case depends on the gdolib parser hardening.Only motion is gated. Obstruction, button, motor, light, lock and door all exist in the Security+1.0 protocol and are left alone.
Testing
Compiles clean for
garage-door-GDOv2-Q.yaml(ESP32-S3) against the local component tree.Not yet exercised on hardware — the interesting path needs a Security+1.0 opener whose V1 handshake fails into the V2 probe window, which I can't stage here. The confirming signal on a real device is the new
Ignoring motion state ...; not synced on Security+2.0warning appearing where aMotion: detectedwould previously have published, correlated with gdolib'ssecplus V1 panel emulation failed, trying secplus V2 panel emulation.🤖 Generated with Claude Code