fix(ble): harden BLE transport routing and reassembly - #48
Merged
Conversation
added 2 commits
July 9, 2026 18:33
- Guard _on_message against Domain values with no _queues entry (DOMAIN_BROADCAST, DOMAIN_AUTHD, ...) - indexing directly raised KeyError inside the ReassemblingBuffer callback, aborting reassembly of any further already-buffered messages in that notification. - Make the ACK-then-data follow-up wait a named, overridable attribute instead of a hardcoded literal, so tests can drive it without a real 2s sleep. No default-timing change. - Add regression tests for ReassemblingBuffer framing (multi-packet reassembly, multiple messages per chunk, corruption/oversized-length resync), _on_message VCSEC-vs-INFO routing and the domain-crash fix, and _send's ACK-then-data follow-up state machine (stale-queue flush, unrelated-message skip, ACK-only timeout, total timeout). Live-verified read-only over BLE against LRW3F7EK4NC716336 via m5-btproxy.local: vehicle_state() (VCSEC, asleep+locked), wake_up(), ping() and charge_state() (INFO) all round-tripped successfully with no actuation sent.
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
_sendACK-then-data follow-up,ReassemblingBufferframing/corruption resync, handshake/session counter-epoch handling,_on_messagebroadcast filtering.Commandsmethod onVehicleBluetooth- parity comes from inheritance, not reimplementation._on_messageindexed_queuesdirectly bymsg.from_destination.domain, but theDomainenum has values (DOMAIN_BROADCAST,DOMAIN_AUTHD, ...) with no_queuesentry.KeyErrorinsideReassemblingBuffer's synchronous parse callback, aborting reassembly of any further already-buffered messages in that same GATT notification..get(), drop unrecognized domains with a debug log instead of crashing._sendinto a named_ack_followup_timeoutclass attribute (default unchanged) so tests can drive that path fast instead of a real 2s sleep.ble_mocked_transport.pybase, which mocks_senditself and would bypass exactly the code this PR needed to exercise):tests/test_ble_reassembling_buffer.py- multi-packet reassembly, multiple messages per chunk, corruption/oversized-length-header resync.tests/test_ble_message_routing.py- VCSEC vs INFO routing, broadcast filtering, regression coverage for the KeyError fix.tests/test_ble_send_transport.py- ACK-then-data follow-up state machine, stale-queue flush, unrelated-message skip, total-timeout path.LRW3F7EK4NC716336,m5-btproxy.local, key already paired from a prior session):vehicle_state()(VCSEC, asleep+locked),wake_up(),ping()+charge_state()(INFO) - all round-tripped, no actuation command sent.info-roundtripsubcommand to the git-excluded local BLE harness to support the ping+charge_state check (harness never committed).AGENTS.mdCode Style bullet documenting theDomain/_queuesmismatch as a durable gotcha for future BLE work.What Changed
_sendACK/data follow-up handling across mocked transport boundaries.Risk Assessment
✅ Low: The production change is tightly scoped to safer BLE domain routing and a testability timeout seam, with added focused regression coverage and no material merge blockers found.
Testing
Exercised the new BLE transport regressions, adjacent BLE command/scan/counter tests, a mocked evidence-producing BLE notification and
_sendround-trip for both VCSEC and INFO domains, then the full test suite; all passed, and transient test/build artifacts were cleaned from the worktree.Evidence: BLE transport round-trip evidence
Live BLE evidence (read-only, against the real car)
Target: VIN
LRW3F7EK4NC716336viam5-btproxy.local(ESPHome proxy,--no-key), signing key already whitelisted from a prior pairing session.No actuation command was sent; single BLE consumer; disconnected cleanly after each call.
Full narrative / original brief
PR-1 of the BLE production-readiness master plan (report at
/home/brett/firstmate/data/tfa-ble-prime-plan-p9/report.md): unit-test and harden the BLE-specific transport plumbing intesla_fleet_api/tesla/vehicle/bluetooth.pyso both signed-command domains (VCSEC and INFO) are proven working over BLE with read-only round-trips, on top of the mocked-transport test base that landed in PR#47 (tests/ble_mocked_transport.py). Scope was deliberately narrow per the plan:_send's ACK-then-data follow-up logic,ReassemblingBufferframing/corruption resync, handshake/session counter-epoch handling, and_on_message's broadcast filtering - only fix transport behavior where a genuine bug was found, and never redefine an inheritedCommandsmethod onVehicleBluetooth(parity is achieved by inheritance, per the plan's design rule 1).Found and fixed one real bug:
_on_messageindexedVehicleBluetooth._queuesdirectly bymsg.from_destination.domain, but theDomainprotobuf enum has values (DOMAIN_BROADCAST,DOMAIN_AUTHD,DOMAIN_ENERGY_DEVICE,DOMAIN_ENERGY_DEVICE_AUTH) with no corresponding_queuesentry (onlyDOMAIN_VEHICLE_SECURITY/DOMAIN_INFOTAINMENTexist) - a message addressed to us from one of those domains would raiseKeyErrorinside theReassemblingBuffer's synchronous parse callback, aborting reassembly of any further already-buffered messages in that same GATT notification. Fixed by looking the queue up with.get()and dropping unrecognized domains with a debug log instead of crashing.Also extracted the hardcoded 2-second ACK-follow-up wait in
_sendinto a named_ack_followup_timeoutclass attribute (default unchanged) purely so tests can drive that code path fast instead of sleeping 2s for real - a pure testability refactor, not a behavior change.Added three new regression test files:
tests/test_ble_reassembling_buffer.py(multi-packet reassembly across GATT MTU chunks, multiple messages in one chunk, corruption/oversized-length-header resync viaReassemblingBuffer.discard_packet'spacket_startstracking),tests/test_ble_message_routing.py(VCSEC vs INFO domain routing through_on_message, broadcast filtering, and regression coverage for the KeyError fix), andtests/test_ble_send_transport.py(_send's ACK-then-data follow-up state machine: immediate full response, ACK-then-later-data, ACK-only timeout returning the ACK, stale-queue flush before send, unrelated-message skip, and the total-timeoutBluetoothTimeoutpath). All new tests constructVehicleBluetoothdirectly and mock only the GATT client/queue boundary (no real BLE), following the existingtest_command_counter_lock.py/test_find_vehicle_scan_filter.pypattern in this repo, not theble_mocked_transport.pybase (which mocks_senditself and therefore bypasses exactly the code this PR needed to exercise).Classification: SAFE-TO-SELF-MERGE - both changes only affect handling of already-received bytes and test seams; neither changes what bytes get sent to the car.
Live-verified read-only over BLE against the test car (VIN
LRW3F7EK4NC716336,m5-btproxy.localESPHome proxy, key already paired from a prior session) as required by the plan before merge:vehicle_state()(VCSEC, car asleep+locked, no actuation),wake_up(), thenping()andcharge_state()(INFO domain, proves both the ACK-decode and typed-vehicleData-decode reply paths work live). No actuation command was sent at any point.Also added an
info-roundtripsubcommand to the git-excluded local BLE harness (scripts/ble-harness/, never committed) to support that ping+charge_state live check, and added a shortAGENTS.mdCode Style bullet documenting theDomain/_queuesmismatch as a durable gotcha for future BLE work.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
uv run pytest tests/test_ble_reassembling_buffer.py tests/test_ble_message_routing.py tests/test_ble_send_transport.pyuv run pytest tests/test_ble_mocked_commands.py tests/test_command_counter_lock.py tests/test_find_vehicle_scan_filter.py tests/test_ble_reassembling_buffer.py tests/test_ble_message_routing.py tests/test_ble_send_transport.pyuv run python - <<'PY' > /tmp/no-mistakes-evidence/01KX306HDMK0H8H01DWPM5WE2K/ble_transport_roundtrip_evidence.jsonuv run pytest tests✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.