Skip to content

feat(vehicle): add BLE confirmation ladder#68

Merged
Bre77 merged 5 commits into
mainfrom
fm/tfa-ble-broadcast-confirm-c2
Jul 12, 2026
Merged

feat(vehicle): add BLE confirmation ladder#68
Bre77 merged 5 commits into
mainfrom
fm/tfa-ble-broadcast-confirm-c2

Conversation

@Bre77

@Bre77 Bre77 commented Jul 12, 2026

Copy link
Copy Markdown
Member

Intent

Step 2 of the BLE command-confirmation ladder (step 1 was PRs #66/#67, already merged): complete the ladder to write -> ack-or-broadcast -> state-verify -> unconfirmed-per-config, and fold in two scope additions the captain approved mid-task.

  1. Broadcast-as-confirmation (the original brief): for VCSEC lock/unlock actuations, race the addressed-ack wait against the vehicle's own unsolicited VCSEC status broadcast on the same notification subscription (evidence: the timeout-rate study showed the vehicle keeps emitting status broadcasts even when it drops the addressed ack, and 100% of verifiable ack-loss timeouts had actually executed). Whichever confirms first wins; a broadcast showing a DIFFERENT state does not fail fast (it's tracked, not acted on immediately) since a later broadcast in the same window could still confirm success - deliberate design choice to avoid false positives from a broadcast that arrives before the vehicle finishes actuating. Reuses the exact same expected-state predicate (_vcsec_verify_plan) as the existing verify_commands feature - one source of truth for both the read-based and broadcast-based confirmation. Scoped to lock/unlock only, since that's the only VCSEC actuation with an observed status broadcast; commands with no such broadcast (closures, wake, INFO-domain commands) are unaffected - they keep exactly the prior ack-wait behavior.

  2. Design-review finding folded in mid-task (captain-approved): a verify_commands state MISMATCH (command PROVEN not applied) used to re-raise BluetoothUnconfirmedCommand, the same type as a genuinely-ambiguous timeout - so Router refused to fail over on the one outcome where failover is unambiguously safe. Added a new BluetoothCommandFailed exception (does NOT subclass BluetoothTimeout/BluetoothUnconfirmedCommand) for 'proven not applied': raised by (a) a verify_commands post-timeout read mismatch, and (b) a mismatching broadcast that is STILL the last word once the whole ack/broadcast wait window elapses with nothing else confirming (deliberately NOT raised on the first mismatching broadcast, to preserve the anti-false-positive design from part 1 - only fires once the vehicle has had its full window to actually complete the actuation). Router's existing generic except clause already fails over on BluetoothCommandFailed correctly since it isn't a BluetoothUnconfirmedCommand subclass - no Router code changes were needed, only the new exception's placement outside that hierarchy.

  3. Second scope addition (captain-approved): replaced the optimistic/verify_commands boolean pair with a single confirmation enum (Literal['optimistic','ack','verify'], default 'ack') since they were really one linear ladder-depth choice with several nonsensical corners (optimistic silently dominated the other two). raise_unconfirmed default flipped from True to False, backed by the timeout study's finding that ~100% of BLE ack-loss timeouts had actually executed - raising on that routine, benign event was training callers to blanket-catch-and-ignore, which also swallows real errors. optimistic/verify_commands kept as deprecated keyword-only constructor/factory aliases (DeprecationWarning, map onto confirmation, old dominance order preserved when both are passed) for one release; backward-compat read-only properties (vehicle.optimistic / vehicle.verify_commands) still work for existing callers. Updated all four createBluetooth-family factory signatures (Vehicles, VehiclesBluetooth x2, teslemetry/tessie stubs) to match.

Updated docs/bluetooth_vehicles.md and AGENTS.md to describe the confirmation enum + raise_unconfirmed as the CURRENT surface (not a changelog), documented which command families are reliably confirmed (broadcast: lock/unlock only; verify: the wider prover table already documented) vs best-effort (everything else, unchanged). Existing tests asserting the old verify-mismatch-raises-BluetoothUnconfirmedCommand behavior and the old raise_unconfirmed=True default were updated to match the new, intentional behavior change. New tests cover: broadcast confirms before ack (real un-mocked _send race, not the usual mocked-_send harness), ack rejection still wins over flowing broadcasts, stateless commands never arm a broadcast watcher, a mismatch standing at window-end raises BluetoothCommandFailed while total silence still raises BluetoothUnconfirmedCommand, Router fails over on BluetoothCommandFailed but not on BluetoothUnconfirmedCommand, the confirmation enum's three rungs, the deprecated-arg mapping + warning, and the new production defaults. Full suite (272 tests), ruff, ruff format, and pyright strict are all clean.

What Changed

  • Added BLE command confirmation modes with the new confirmation ladder (optimistic, ack, verify), deprecated compatibility aliases, runtime validation, and best-effort unconfirmed behavior by default.
  • Added broadcast-as-confirmation for BLE lock/unlock commands, including race handling with addressed replies and distinct BluetoothCommandFailed handling for proven non-application.
  • Updated Bluetooth documentation and expanded BLE/router tests for broadcast confirmation, confirmation mode compatibility, unconfirmed outcomes, and router failover behavior.

Risk Assessment

✅ Low: The changes are well-scoped to BLE confirmation behavior and compatibility shims, with targeted tests covering the main race and fallback semantics; I did not find a material merge-blocking or follow-up issue in the changed code.

Testing

Inspected the change scope, ran the focused BLE confirmation/router tests and the full test suite, retried the evidence script with uv run python after bare python was unavailable, captured a direct behavior transcript, and cleaned pytest cache artifacts from the worktree; all validation passed.

Evidence: BLE confirmation behavior transcript
BLE confirmation ladder evidence
default confirmation='ack', raise_unconfirmed=False
door_lock with no addressed ack + matching unsolicited locked broadcast:
  result={'response': {'result': True, 'reason': ''}}
  gatt_writes=1, addressed_ack_delivered=False
door_lock with no addressed ack + final mismatching unlocked broadcast:
  exception=BluetoothCommandFailed
door_lock with no addressed ack and total silence:
  exception=BluetoothUnconfirmedCommand
VehicleRouter fallback behavior:
  BluetoothCommandFailed result: fallback:13
  BluetoothCommandFailed calls: (1, 1)
  BluetoothUnconfirmedCommand result: BluetoothUnconfirmedCommand
  BluetoothUnconfirmedCommand calls: (1, 0)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 3 issues found → auto-fixed (3) ✅
  • 🚨 tesla_fleet_api/tesla/vehicle/bluetooth.py:419 - The deprecated boolean API is not actually backward compatible for positional callers: the old verify_commands slot is now confirmation, so VehicleBluetooth(parent, vin, key, device, True) silently becomes confirmation=True and behaves like plain ack mode with no deprecation warning. The same positional break exists in the factory methods that moved optimistic/raise_unconfirmed slots. Preserve/migrate old positional booleans for one release or make this an explicit breaking change.
  • ⚠️ tesla_fleet_api/tesla/vehicle/bluetooth.py:831 - Broadcast watching is armed only after write_gatt_char completes and _await_broadcast_confirmation runs, so a fast status broadcast emitted while the write await is still in progress is dropped by _on_message. That can turn an actually confirmed lock/unlock into an unconfirmed timeout or unnecessary verify read. Register the watcher before writing, then tear it down after the response/broadcast race completes.
  • ⚠️ tesla_fleet_api/tesla/vehicle/bluetooth.py:785 - If the addressed wait times out in the same event-loop turn that the broadcast task receives a matching confirmation, this branch always reads response_task.result() first and raises BluetoothTimeout, ignoring the completed successful broadcast. When both tasks are done, prefer a successful broadcast over a response timeout while still preserving addressed rejections/successes.

🔧 Fix: Fix BLE confirmation compatibility and races
1 warning still open:

  • ⚠️ tesla_fleet_api/tesla/vehicle/bluetooth.py:863 - Because the broadcast watcher is now armed before write_gatt_char completes, a periodic stale broadcast showing the old lock state can be appended to mismatches before the command is actually written. If the addressed ack is then lost and no later broadcast arrives, _await_response_or_broadcast upgrades that pre-write stale state to BluetoothCommandFailed, which can incorrectly report a proven failure and trigger router failover even though the BLE command may have applied. Keep early matching broadcasts as confirmations, but only treat mismatches observed after the write completes as final proof of non-application.

🔧 Fix: Fix prewrite broadcast mismatch handling
1 warning still open:

  • ⚠️ tesla_fleet_api/tesla/vehicle/bluetooth.py:460 - The new string-based confirmation API is not validated at runtime, so a typo like confirmation="verfy" silently falls through to ack-mode behavior and skips the caller’s intended optimistic/verify semantics. After applying deprecated bool/alias mapping, reject values outside "optimistic", "ack", and "verify" with ValueError.

🔧 Fix: Reject invalid BLE confirmation modes
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • git diff --stat a51e934ba577f52daa18887b923fdcac19262aac..3180e2322ee00d547b7032ce8773d43cac03d522
  • uv run pytest tests/test_ble_broadcast_confirmation.py tests/test_ble_confirmation_mode.py tests/test_ble_command_verification.py tests/test_ble_optimistic_and_best_effort.py tests/test_ble_unconfirmed_command.py tests/test_router.py
  • uv run pytest tests
  • uv run python - <<'PY' | tee /tmp/no-mistakes-evidence/01KXA6QJ2TSNMV5N1QVG2EY24H/ble-confirmation-transcript.txt
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

firstmate crewmate added 5 commits July 12, 2026 13:41
…der redesign

Completes the BLE command-confirmation ladder (write -> ack-or-broadcast ->
state verify -> unconfirmed-per-config):

- VCSEC lock/unlock actuations race the ack wait against the vehicle's own
  unsolicited status broadcast on the same subscription; a broadcast already
  showing the requested state confirms success before the ack timeout, and a
  mismatching broadcast standing at the end of the window is now-final proof
  of non-application.
- New BluetoothCommandFailed exception distinguishes "proven not applied"
  (verify_commands read mismatch, or a broadcast mismatch at window-end) from
  "genuinely unresolved" (BluetoothUnconfirmedCommand) - a fallback Router
  fails over on the former but not the latter, since only the former carries
  no double-execution risk.
- optimistic/verify_commands booleans are replaced by a single confirmation
  enum ("optimistic"|"ack"|"verify"), with the old booleans kept as
  deprecated constructor/factory aliases (DeprecationWarning, map onto the
  enum). raise_unconfirmed now defaults to False, matching the ~100%
  executed-on-timeout evidence for BLE ack loss.

Docs (docs/bluetooth_vehicles.md) and AGENTS.md updated to describe the
current confirmation surface and which command families are reliably
confirmed (broadcast: lock/unlock; verify: the wider prover table) vs
best-effort (everything else).
@Bre77
Bre77 merged commit fcaed9e into main Jul 12, 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