fix(funnel): treat any unlocked vehicle lock state as unlocked - #134
Merged
Conversation
… unlocked Map VEHICLELOCKSTATE_INTERNAL_LOCKED as locked and VEHICLELOCKSTATE_SELECTIVE_UNLOCKED as unlocked in _LOCK_STATES, and update the comment to state the current rule instead of the prior unvalidated-reasoning note. UNKNOWN and FAILED_UNLATCH remain unmapped.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
Intent
Widen funnel.py's _LOCK_STATES mapping to treat any unlocked lock state as unlocked, per a settled product ruling: 'should a driver-door-only unlock report your car as fully unlocked, yes. Any unlocked is unlocked.' Add VEHICLELOCKSTATE_INTERNAL_LOCKED -> True and VEHICLELOCKSTATE_SELECTIVE_UNLOCKED -> False to _LOCK_STATES. Rewrite the comment above _LOCK_STATES to state the current rule (any unlocked state reads unlocked) rather than the old 'unvalidated against live frames, decline to decide' reasoning. Deliberately leave VEHICLELOCKSTATE UNKNOWN and FAILED_UNLATCH-equivalent closure states untouched -- those don't exist on the lock enum, and the ruling explicitly does not extend to _CLOSURE_STATES's already-unmapped CLOSURESTATE_UNKNOWN/CLOSURESTATE_FAILED_UNLATCH by analogy, so _CLOSURE_STATES was left as-is except fixing a comment that referenced the old lock-state reasoning by 'same reason.' No restructuring of _LOCK_STATES, the funnel, or its tests beyond the two new entries, the comment rewrite, and updated test coverage. This unblocks the Home Assistant lock entity from having no reading at cold start when the vehicle is in one of these two states and only BLE frames are available; the HA integration itself is out of scope for this change. Extended tests/test_funnel_bluetooth.py's existing test (previously asserting these two states emit no observation) to assert they now map to True/False respectively.
What Changed
tesla_fleet_api/funnel.py: addedVEHICLELOCKSTATE_INTERNAL_LOCKED -> TrueandVEHICLELOCKSTATE_SELECTIVE_UNLOCKED -> Falseto_LOCK_STATES, so a driver-door-only unlock now reports the vehicle as fully unlocked instead of emitting no observation._LOCK_STATESto state the current rule (any unlocked state reads unlocked), and updated the_CLOSURE_STATEScomment so it no longer refers back to the old lock-state reasoning by "same reason" —_CLOSURE_STATESitself is unchanged.tests/test_funnel_bluetooth.py: renamed and rewrote the test previously assertingINTERNAL_LOCKED/SELECTIVE_UNLOCKEDemit no observation, now asserting they map toTrue/Falserespectively.AGENTS.md: updated theObservationFunneldescription to match the widened lock-state mapping.Risk Assessment
✅ Low: The change is a minimal, well-scoped dict/comment addition matching the stated product ruling exactly, with correct enum semantics (INTERNAL_LOCKED→True, SELECTIVE_UNLOCKED→False) and a behavior-driven test through the real message-routing path; the prior round's stale-doc finding was correctly fixed in the actual file (AGENTS.md, target of the CLAUDE.md symlink).
Testing
Ran the targeted funnel test suite (43 tests) which passed, including the intent-specific regression test that drives the real BleBroadcastPublisher → ObservationFunnel pipeline with INTERNAL_LOCKED/SELECTIVE_UNLOCKED VCSEC broadcasts and asserts the sink now observes True/False rather than nothing — this is an execution of the actual public behavior, not a source-text check. Diff review confirms the change is scoped exactly as the user intent describes: two new _LOCK_STATES entries, rewritten comments on both _LOCK_STATES and _CLOSURE_STATES, _CLOSURE_STATES itself untouched, and the AGENTS.md (CLAUDE.md symlink target) sentence updated to reflect the new rule. No issues found.
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
CLAUDE.md:72- CLAUDE.md's ObservationFunnel entry still states 'VCSEC INTERNAL_LOCKED/SELECTIVE_UNLOCKED and closure UNKNOWN/FAILED_UNLATCH are unmapped pending live-frame validation.' This change maps INTERNAL_LOCKED->True and SELECTIVE_UNLOCKED->False in _LOCK_STATES, so the sentence is now false for those two lock states (it remains true only for the closure UNKNOWN/FAILED_UNLATCH states). A future session relying on this doc would wrongly believe these two lock states still emit no observation.🔧 Fix: docs: fix stale CLAUDE.md sentence on lock-state mapping
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
uv run pytest tests/test_funnel_bluetooth.py tests/test_funnel.py -v(43 passed)tests/test_funnel_bluetooth.py::TestBroadcastTranslation::test_partial_lock_states_map_to_booleans — extended regression test driving BleBroadcastPublisher with INTERNAL_LOCKED then SELECTIVE_UNLOCKED VCSEC broadcasts through the real ObservationFunnel, asserting the sink receives [(LOCKED, True), (LOCKED, False)] instead of the old no-observation behaviorgit diff of funnel.py/test_funnel_bluetooth.py/AGENTS.md between base 8bccb5a and target dc9ace6 to confirm scope matched the stated intent (two new enum entries, comment rewrites, test extension, no other restructuring)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.