Skip to content

fix(ble): harden BLE transport routing and reassembly - #48

Merged
Bre77 merged 2 commits into
mainfrom
fm/tfa-ble-pr1-transport-t4
Jul 9, 2026
Merged

fix(ble): harden BLE transport routing and reassembly#48
Bre77 merged 2 commits into
mainfrom
fm/tfa-ble-pr1-transport-t4

Conversation

@Bre77

@Bre77 Bre77 commented Jul 9, 2026

Copy link
Copy Markdown
Member

Intent

  • PR-1 of the BLE production-readiness plan: prove VCSEC and INFO domains both work over BLE with read-only round-trips, and harden the transport plumbing under them (builds on PR#47's mocked-transport test base).
    • Scope was deliberately narrow per the plan: _send ACK-then-data follow-up, ReassemblingBuffer framing/corruption resync, handshake/session counter-epoch handling, _on_message broadcast filtering.
    • Design rule followed: never redefine an inherited Commands method on VehicleBluetooth - parity comes from inheritance, not reimplementation.
  • Bug found and fixed: _on_message indexed _queues directly by msg.from_destination.domain, but the Domain enum has values (DOMAIN_BROADCAST, DOMAIN_AUTHD, ...) with no _queues entry.
    • A message addressed to us from one of those domains raised KeyError inside ReassemblingBuffer's synchronous parse callback, aborting reassembly of any further already-buffered messages in that same GATT notification.
    • Fix: look the queue up with .get(), drop unrecognized domains with a debug log instead of crashing.
  • Testability refactor: extracted the hardcoded 2s ACK-follow-up wait in _send into a named _ack_followup_timeout class attribute (default unchanged) so tests can drive that path fast instead of a real 2s sleep.
  • New regression tests (mock only the GATT client/queue boundary, no real BLE - not the ble_mocked_transport.py base, which mocks _send itself 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.
  • Classification: SAFE-TO-SELF-MERGE - both changes only affect handling of already-received bytes and a test seam; neither changes what bytes go to the car.
  • Live-verified read-only over BLE against the test car (VIN 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.
    • Also added an info-roundtrip subcommand to the git-excluded local BLE harness to support the ping+charge_state check (harness never committed).
  • Docs: added an AGENTS.md Code Style bullet documenting the Domain/_queues mismatch as a durable gotcha for future BLE work.

What Changed

  • Hardened BLE message routing so received messages for unsupported protobuf domains are dropped with debug logging instead of raising during reassembly.
  • Made the BLE ACK follow-up wait configurable for tests while preserving the existing default timeout behavior.
  • Added regression coverage for BLE frame reassembly, domain routing, and _send ACK/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 _send round-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
{
  "notification_reassembly_and_routing": {
    "info_payload": "info-frame",
    "info_queue_size_after_framed_notifications": 1,
    "unhandled_broadcast_domain_dropped_without_exception": true,
    "vcsec_payload": "vcsec-frame",
    "vcsec_queue_size_after_framed_notifications": 1
  },
  "scenario": "Mocked BLE transport evidence for production BLE plumbing: framed GATT notifications, domain routing, broadcast drop, and _send round-trips for both signed-command domains.",
  "send_round_trips": [
    {
      "domain": "DOMAIN_VEHICLE_SECURITY",
      "mode": "ack_then_data",
      "returned_from_domain": "DOMAIN_VEHICLE_SECURITY",
      "returned_payload": "vcsec-readonly-state",
      "write_count": 1,
      "writes": [
        {
          "payload_len": 18,
          "response": true,
          "uuid": "00000212-b2d1-43f0-9b88-960cebf8b91e"
        }
      ]
    },
    {
      "domain": "DOMAIN_INFOTAINMENT",
      "mode": "data",
      "returned_from_domain": "DOMAIN_INFOTAINMENT",
      "returned_payload": "info-readonly-state",
      "write_count": 1,
      "writes": [
        {
          "payload_len": 18,
          "response": true,
          "uuid": "00000212-b2d1-43f0-9b88-960cebf8b91e"
        }
      ]
    }
  ],
  "vin_fixture": "5YJXCAE43LF123456"
}
Live BLE evidence (read-only, against the real car)

Target: VIN LRW3F7EK4NC716336 via m5-btproxy.local (ESPHome proxy, --no-key), signing key already whitelisted from a prior pairing session.

$ ble_harness.py --no-key --esphome-address m5-btproxy.local state --vin LRW3F7EK4NC716336
vehicleLockState: VEHICLELOCKSTATE_LOCKED
vehicleSleepStatus: VEHICLE_SLEEP_STATUS_ASLEEP
userPresence: VEHICLE_USER_PRESENCE_NOT_PRESENT

$ ble_harness.py --no-key --esphome-address m5-btproxy.local wake --vin LRW3F7EK4NC716336
{'response': {'result': True, 'reason': ''}}

$ ble_harness.py --no-key --esphome-address m5-btproxy.local info-roundtrip --vin LRW3F7EK4NC716336
ping: {'response': {'result': True, 'reason': seconds: 1783585948
nanos: 953000000
}}
charge_state: charging_state { Complete {} } ... (full ChargeState proto decoded)

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 in tesla_fleet_api/tesla/vehicle/bluetooth.py so 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, ReassemblingBuffer framing/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 inherited Commands method on VehicleBluetooth (parity is achieved by inheritance, per the plan's design rule 1).

Found and fixed one real bug: _on_message indexed VehicleBluetooth._queues directly by msg.from_destination.domain, but the Domain protobuf enum has values (DOMAIN_BROADCAST, DOMAIN_AUTHD, DOMAIN_ENERGY_DEVICE, DOMAIN_ENERGY_DEVICE_AUTH) with no corresponding _queues entry (only DOMAIN_VEHICLE_SECURITY/DOMAIN_INFOTAINMENT exist) - a message addressed to us from one of those domains would raise KeyError inside the ReassemblingBuffer'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 _send into a named _ack_followup_timeout class 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 via ReassemblingBuffer.discard_packet's packet_starts tracking), tests/test_ble_message_routing.py (VCSEC vs INFO domain routing through _on_message, broadcast filtering, and regression coverage for the KeyError fix), and tests/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-timeout BluetoothTimeout path). All new tests construct VehicleBluetooth directly and mock only the GATT client/queue boundary (no real BLE), following the existing test_command_counter_lock.py/test_find_vehicle_scan_filter.py pattern in this repo, not the ble_mocked_transport.py base (which mocks _send itself 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.local ESPHome 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(), then ping() and charge_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-roundtrip subcommand to the git-excluded local BLE harness (scripts/ble-harness/, never committed) to support that ping+charge_state live check, and added a short AGENTS.md Code Style bullet documenting the Domain/_queues mismatch 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.py
  • uv 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.py
  • uv run python - <<'PY' > /tmp/no-mistakes-evidence/01KX306HDMK0H8H01DWPM5WE2K/ble_transport_roundtrip_evidence.json
  • uv run pytest tests
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

firstmate crewmate 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.
@Bre77
Bre77 merged commit c28d86e into main Jul 9, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant