feat(vehicle): add BLE command confirmation controls#67
Merged
Conversation
added 2 commits
July 12, 2026 12:32
Adds step 1 of the BLE command-confirmation ladder (write -> ack wait -> verify_commands -> unconfirmed outcome), building on PR #66's BluetoothUnconfirmedCommand: - optimistic (default False): a mutating command returns success as soon as its GATT write is confirmed, skipping the ack wait and verify_commands entirely. Only a write/transport failure still raises. - raise_unconfirmed (default True, current behavior): when False, an exhausted confirmation ladder resolves as a best-effort success instead of raising BluetoothUnconfirmedCommand. A car-side rejection, a verify_commands state mismatch, and write failures are unaffected and always raise. Both are BLE-only constructor/factory knobs threaded through Vehicles/VehiclesBluetooth.create*, matching verify_commands. Router needs no changes: optimistic's only exception is a generic BluetoothTransportError that already fails over correctly, and raise_unconfirmed=False's best-effort success is just a normal return value. Broadcast confirmation (the next rung of the ladder) is a separate follow-up.
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
Captain-decided BLE command-confirmation ladder, step 1 of 2. Building on the merged BluetoothUnconfirmedCommand change (PR #66), add two consumer-facing knobs to VehicleBluetooth that together shape the outcome of an ambiguous BLE ack timeout: (1) optimistic (default False) - a mutating BLE command returns success as soon as the GATT write is confirmed, with no ack wait and no verify_commands consultation; only a write/transport failure (BluetoothTransportError) still raises. This is deliberately 'pure speed mode' where the caller owns any state verification it wants. (2) raise_unconfirmed (default True, preserving current behavior) - when False, an exhausted confirmation ladder (ack wait expired, verify_commands off/no-plan/read-failed) resolves as a best-effort success instead of raising BluetoothUnconfirmedCommand. Deliberately, a car-side rejection carried in an ack, a verify_commands state mismatch (state read succeeded but disagrees with the requested value), and write/transport failures must ALL still raise regardless of this flag - it only converts the true 'could not determine what happened' case. This required reshaping the internal _resolve_timeout() to return dict|None instead of always raising, so the mismatch case (raise directly, unaffected by raise_unconfirmed) stays distinguishable from the ambiguous case (routed through a new _unconfirmed_outcome() helper that applies raise_unconfirmed). Both knobs are BLE-only, threaded through Vehicles/VehiclesBluetooth.create* and the Teslemetry/Tessie NotImplementedError stub signatures, matching the existing verify_commands idiom exactly - the cloud path (VehicleSigned, commands.py, signed.py) is deliberately untouched, no cloud behavior change. Router required zero code changes by design: optimistic's only possible exception (BluetoothTransportError) is a generic exception that already fails over correctly since it's not BluetoothUnconfirmedCommand, and raise_unconfirmed=False's best-effort success is just an ordinary return value with no exception at all - this was verified by re-reading router/base.py's _dispatch failover logic rather than assumed. Explicitly out of scope for this PR: broadcast confirmation (the next rung of the ladder) is a separate follow-up task and was not implemented; timeouts were not changed; the cloud REST/Fleet-signed path was not touched. Added a dedicated test file (tests/test_ble_optimistic_and_best_effort.py, 15 tests) covering: optimistic returns without waiting for both VCSEC and infotainment commands, optimistic still raises on write failure, optimistic is unaffected for the one non-mutating command (ping) and never consults verify_commands even when it's enabled, raise_unconfirmed=False converts the no-verify case and the verify_commands-enabled-but-unresolvable case (no plan, and separately a failed prover read) to success, raise_unconfirmed=False leaves a genuine verify_commands mismatch raising, and defaults (both flags at their default) preserve today's exact behavior including exception chaining (cause). Extended the make_vehicle() test helper in ble_mocked_transport.py with optimistic/raise_unconfirmed params. Updated docs/bluetooth_vehicles.md with a new section documenting the full ladder, the two knobs, and which BLE commands are 'best-effort by design' (those with no verify_commands plan - true toggles, relative steps, ack-only actions) versus which get a reliable state-backed check; also updated the debug-logging section for the new raise_unconfirmed=False log line. Updated AGENTS.md with a bullet capturing the design rationale (why _resolve_timeout was reshaped, why Router needed no changes) for future agents. Full test suite (248 tests), ruff check, ruff format, and pyright strict all pass clean; pre-existing ruff format issues in unrelated files were confirmed pre-existing via git stash before this work and are not part of this change.
What Changed
optimisticandraise_unconfirmedcommand confirmation options, threaded through Bluetooth vehicle factories and service-specific vehicle stubs.Risk Assessment
✅ Low: I found no material correctness, security, performance, or error-handling issues in the scoped diff; the new BLE knobs are isolated to VehicleBluetooth, defaults preserve existing behavior, and the factory/test updates match the intended surfaces.
Testing
Exercised the new BLE knobs through the dedicated 15-test suite, recorded a mocked end-user caller transcript showing success and preserved failure outcomes, then ran the full 248-test suite; all passed, and transient worktree artifacts were cleaned up.
Evidence: BLE confirmation ladder transcript
optimistic=True door_lock result: {'response': {'result': True, 'reason': ''}} optimistic=True _send await count: 1 raise_unconfirmed=False timeout result: {'response': {'result': True, 'reason': ''}} verify_commands mismatch exception: BluetoothUnconfirmedCommand optimistic=True write failure exception: BluetoothTransportErrorPipeline
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_optimistic_and_best_effort.pymkdir -p /tmp/no-mistakes-evidence/01KXA2RM0PVH4R8MSFJ4YXR116 && uv run python - <<'PY' | tee /tmp/no-mistakes-evidence/01KXA2RM0PVH4R8MSFJ4YXR116/ble_confirmation_ladder_evidence.txt ... PYuv run pytest tests✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.