diff --git a/AGENTS.md b/AGENTS.md index fd63907..c3cfbf7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -99,7 +99,7 @@ No release-please or version-bump automation. To ship: bump `version` in `pyproj `exceptions.py` maps HTTP status codes and error keys to specific exception classes. `raise_for_status()` parses responses and raises the appropriate exception. Signed command faults have separate hierarchies: `TeslaFleetInformationFault`, `TeslaFleetMessageFault`, `SignedMessageInformationFault`, `WhitelistOperationStatus`. -All exceptions inherit from `TeslaFleetError(BaseException)`, deliberately **not** `Exception` — a bare `except Exception` (e.g. in retry/backoff loops around BLE reads) silently fails to catch `BluetoothTimeout` and every other library error. Catch `TeslaFleetError` (or `BaseException`) explicitly. `VehicleBluetooth` wraps transport-layer failures (`connect`/`connect_if_needed`, notification setup, the GATT write in `_send`) in `BluetoothTransportError`, a `TeslaFleetError` subclass chaining the original transport exception as its cause — so `except TeslaFleetError` alone now catches BLE transport failures too, not just response-wait `BluetoothTimeout`/`BluetoothUnconfirmedCommand`. These catch sites deliberately catch **both** `bleak.exc.BleakError` and builtin `TimeoutError`: bleak-esphome converts an aioesphomeapi GATT/connect/notify timeout into a bare `TimeoutError` (not a `BleakError`), which would otherwise escape the wrap as a non-`TeslaFleetError`. +All exceptions inherit from `TeslaFleetError(BaseException)`, deliberately **not** `Exception` — a bare `except Exception` (e.g. in retry/backoff loops around BLE reads) silently fails to catch `BluetoothTimeout` and every other library error. Catch `TeslaFleetError` (or `BaseException`) explicitly. `VehicleBluetooth` wraps transport-layer failures (`connect`/`connect_if_needed`, notification setup) in `BluetoothTransportError`, a `TeslaFleetError` subclass chaining the original transport exception as its cause — so `except TeslaFleetError` alone now catches BLE transport failures too, not just response-wait `BluetoothTimeout`/`BluetoothUnconfirmedCommand`. The GATT write in `_send` is *not* unconditionally wrapped into `BluetoothTransportError` — see the write-delivery-certainty entry below for the split. These catch sites deliberately catch **both** `bleak.exc.BleakError` and builtin `TimeoutError`: bleak-esphome converts an aioesphomeapi GATT/connect/notify timeout into a bare `TimeoutError` (not a `BleakError`), which would otherwise escape the wrap as a non-`TeslaFleetError`. ### Protobuf @@ -127,9 +127,10 @@ Source protobuf definitions live in `proto/`; generated Python files live in `te - **`charge_standard()` rejects `already_standard`**: live-verified - calling it while `charge_state().charge_limit_soc` already equals `charge_limit_soc_std` gets `{"result": False, "reason": "already_standard"}` rather than a no-op success. Not a library bug; callers/tests exercising this command need the limit to actually differ from the std preset first (e.g. via `charge_max_range()` or `set_charge_limit()`). - **BLE media state-observability gotcha**: `MediaState.now_playing_artist/title` and all of `MediaDetailState` (`now_playing_album/station/source_string/elapsed/duration`) were observed empty/zero on the test car while Spotify was actively `Playing` at nonzero volume - these legacy fields are apparently only populated for certain sources (e.g. USB/Bluetooth), not Spotify. Don't assume `media_next_track`/`media_prev_track`/`media_next_fav`/`media_prev_fav` are state-observable via these readers; verify by ACK (`{"result": True}`) and pair with the inverse command when the fingerprint doesn't change. `audio_volume`/`media_playback_status` (for `adjust_volume`/`media_volume_up`/`media_volume_down`/`media_toggle_playback`) were populated correctly and are reliable provers. - **BLE mutating-command timeout is inconclusive - never assume "the write didn't land"**: this corrects an earlier version of this note, which observed `adjust_volume` write timeouts on one rig leaving the car unmutated and concluded a write timeout means the write never landed. Later live testing disproved that as a general rule: `door_unlock` and `door_lock` each raised a timeout yet both physically executed - a VCSEC state read after each confirmed the lock state had flipped. Reads (state readers, `wake_up()`'s effect) came back reliably all night; only mutating VCSEC/RKE actions showed this false-negative pattern, most likely because the vehicle doesn't reliably return an ack `_send()` observes within the timeout, not because the write failed. Treat `BluetoothUnconfirmedCommand` (a `BluetoothTimeout` subclass) from any mutating BLE command as **inconclusive, not failure**: snapshot state before acting, then verify the outcome with a follow-up state read whenever a mutation times out. Never blind-retry a non-idempotent command (toggles like `media_toggle_playback`, volume steps, schedule add/remove) on timeout alone - see the retry-double-execution entry below for why the library's own retry has the same exposure. The `adjust_volume`/`TimeoutAPIError` root cause from the original observation is still unexplained; it just isn't evidence that timed-out writes never land. VCSEC actuations now use the shorter `_actuation_timeout` when their terminal ack is lost. -- **The BLE mutating-command confirmation ladder is one `confirmation` enum + one `raise_unconfirmed` bool**: `VehicleBluetooth.confirmation` (`"optimistic" | "ack" | "verify"`, default `"ack"`; threaded through `Vehicles`/`VehiclesBluetooth.create*`) picks how many of write → ack-or-broadcast wait → state-read confirmation run; `raise_unconfirmed` (default `False`) picks what happens when the ladder still can't tell. `"optimistic"` short-circuits `_sendVehicleSecurity`/`_sendInfotainment` (`bluetooth.py`) to `_send_optimistic()`, which signs and writes but never waits for any reply - only `BluetoothTransportError` can raise, and nothing else in the ladder is consulted. `"verify"` adds a post-timeout state-read rung: on an unresolved ack/broadcast wait, `_resolve_timeout()` reads the mapped prover state (`_vcsec_verify_plan`/`_INFOTAINMENT_VERIFY_PLANS` in `bluetooth.py`; only clearly-derivable absolute commands are covered - lock/unlock, `set_charge_limit`, `set_charging_amps`, `adjust_volume` absolute, `set_temps`, `auto_conditioning_start/stop`) and returns success on a match, raises `BluetoothCommandFailed` on a proven mismatch, or returns `None` (still unresolved) if the read itself couldn't complete - `None` falls through to `raise_unconfirmed`. Commands with no plan (true toggles, relative steps, ack-only actions) always fall through regardless of `confirmation`. The legacy `optimistic`/`verify_commands` boolean surface is deprecated: both warn (`DeprecationWarning`) and map onto `confirmation` (a positional bool in the `confirmation` slot is treated as old `verify_commands`; dominance order preserved - `optimistic=True` wins if both are set), and remain as read-only properties (`confirmation == "optimistic"`/`"verify"`) for existing readers. See `docs/bluetooth_vehicles.md` for the user-facing table and defaults. +- **The BLE mutating-command confirmation ladder is one `confirmation` enum + one `raise_unconfirmed` bool**: `VehicleBluetooth.confirmation` (`"optimistic" | "ack" | "verify"`, default `"ack"`; threaded through `Vehicles`/`VehiclesBluetooth.create*`) picks how many of write → ack-or-broadcast wait → state-read confirmation run; `raise_unconfirmed` (default `False`) picks what happens when the ladder still can't tell. `"optimistic"` short-circuits `_sendVehicleSecurity`/`_sendInfotainment` (`bluetooth.py`) to `_send_optimistic()`, which signs and writes but never waits for any reply - a provably pre-submission write failure still raises `BluetoothTransportError` unconditionally, but a submitted-then-ambiguous write (see the write-delivery-certainty entry below) follows `raise_unconfirmed` like every other rung instead of being consulted as "nothing else." `"verify"` adds a post-timeout state-read rung: on an unresolved ack/broadcast wait, `_resolve_timeout()` reads the mapped prover state (`_vcsec_verify_plan`/`_INFOTAINMENT_VERIFY_PLANS` in `bluetooth.py`; only clearly-derivable absolute commands are covered - lock/unlock, `set_charge_limit`, `set_charging_amps`, `adjust_volume` absolute, `set_temps`, `auto_conditioning_start/stop`) and returns success on a match, raises `BluetoothCommandFailed` on a proven mismatch, or returns `None` (still unresolved) if the read itself couldn't complete - `None` falls through to `raise_unconfirmed`. Commands with no plan (true toggles, relative steps, ack-only actions) always fall through regardless of `confirmation`. The legacy `optimistic`/`verify_commands` boolean surface is deprecated: both warn (`DeprecationWarning`) and map onto `confirmation` (a positional bool in the `confirmation` slot is treated as old `verify_commands`; dominance order preserved - `optimistic=True` wins if both are set), and remain as read-only properties (`confirmation == "optimistic"`/`"verify"`) for existing readers. See `docs/bluetooth_vehicles.md` for the user-facing table and defaults. - **Broadcast-as-confirmation races the ack wait for lock/unlock**: the vehicle keeps emitting unsolicited VCSEC status broadcasts on the same notification subscription even when it emits no addressed ack for a lock/unlock actuation - live-verified at scale (see the timeout-rate study referenced from `docs/bluetooth_vehicles.md`). `_send`'s `confirm_broadcast` param (threaded through `Commands._command`/`_sendVehicleSecurity`, ignored by the Fleet-signed transport) arms a per-domain watcher in `_on_message` (`_broadcast_watchers`, `bluetooth.py`) that decodes broadcast frames via `_decode_vcsec_status` and races them against the addressed-reply wait in `_await_response_or_broadcast`; first to satisfy the plan's predicate wins, and only the addressed-reply path can raise a car-side rejection. A mismatching broadcast doesn't fail fast (it's appended to `mismatches`, not resolved) since a later broadcast in the same window could still confirm success - but if the whole window elapses with a mismatch as the last word and nothing else confirming, `_await_response_or_broadcast` raises `BluetoothCommandFailed` instead of the ambiguous timeout. This reuses the exact same `_vcsec_verify_plan` predicate as the `"verify"` rung above (one source of truth), applied to a broadcast's decoded `VehicleStatus` instead of a follow-up read; it currently covers only lock/unlock, the one VCSEC actuation with an observed status broadcast. Low-level race tests drive the real (unmocked) `_send` state machine directly - see `tests/test_ble_broadcast_confirmation.py`. -- **`BluetoothUnconfirmedCommand` vs `BluetoothCommandFailed` (`exceptions.py`), and how `Router` treats each**: `_sendVehicleSecurity`/`_sendInfotainment` (`bluetooth.py`, the mutating-command seam, unconditionally) wrap a caught `BluetoothTimeout` into `BluetoothUnconfirmedCommand` when the ladder is genuinely unresolved - the write succeeded and the vehicle may have executed the command despite the lost ack/broadcast. With default `raise_unconfirmed=False` that unresolved outcome returns best-effort success; with `raise_unconfirmed=True` the `BluetoothUnconfirmedCommand` reaches the caller. `BluetoothCommandFailed` is the other, distinct outcome: a state check (the `"verify"` rung's read, or a mismatching broadcast still standing at window-end) actively *proved* the command did not apply - it deliberately does **not** subclass `BluetoothTimeout`/`BluetoothUnconfirmedCommand`. `Router._dispatch` (`router/base.py`) special-cases only `BluetoothUnconfirmedCommand` to skip its normal per-command failover and re-raise immediately - replaying an already-possibly-executed command risks double-execution. `BluetoothCommandFailed` carries no such risk (the command is proven not to have applied), so it falls through `Router`'s ordinary `except (Exception, TeslaFleetError)` clause and fails over like any other error - no `Router` code change was needed for this, only the exception type's placement outside the `BluetoothTimeout` hierarchy. A plain read (`_getVehicleSecurity`/`_getInfotainment`) still raises unadorned `BluetoothTimeout` on the same kind of wait timeout, since a read has no side effect to be unconfirmed about, and a pre-write failure still raises the unrelated `BluetoothTransportError`. +- **`BluetoothUnconfirmedCommand` vs `BluetoothCommandFailed` (`exceptions.py`), and how `Router` treats each**: `_sendVehicleSecurity`/`_sendInfotainment` (`bluetooth.py`, the mutating-command seam, unconditionally) wrap a caught `BluetoothTimeout` into `BluetoothUnconfirmedCommand` when the ladder is genuinely unresolved - either the write succeeded but the ack/broadcast was lost, or the write entered backend I/O and failed with delivery unprovable, so the vehicle may have executed the command. With default `raise_unconfirmed=False` that unresolved outcome returns best-effort success; with `raise_unconfirmed=True` the `BluetoothUnconfirmedCommand` reaches the caller. `BluetoothCommandFailed` is the other, distinct outcome: a state check (the `"verify"` rung's read, or a mismatching broadcast still standing at window-end) actively *proved* the command did not apply - it deliberately does **not** subclass `BluetoothTimeout`/`BluetoothUnconfirmedCommand`. `Router._dispatch` (`router/base.py`) special-cases only `BluetoothUnconfirmedCommand` to skip its normal per-command failover and re-raise immediately - replaying an already-possibly-executed command risks double-execution. `BluetoothCommandFailed` carries no such risk (the command is proven not to have applied), so it falls through `Router`'s ordinary `except (Exception, TeslaFleetError)` clause and fails over like any other error - no `Router` code change was needed for this, only the exception type's placement outside the `BluetoothTimeout` hierarchy. A plain read (`_getVehicleSecurity`/`_getInfotainment`) still raises unadorned `BluetoothTimeout` on the same kind of wait timeout, since a read has no side effect to be unconfirmed about, and a *provably pre-submission* write failure still raises the unrelated `BluetoothTransportError` (see the write-delivery-certainty entry below - a submitted-then-ambiguous write is not this case). +- **Write-delivery certainty splits `BluetoothTransportError` from `BluetoothTimeout` at the GATT write in `_send`**: `write_gatt_char` failures are not uniformly `BluetoothTransportError`. `BleakCharacteristicNotFoundError` (bleak resolves `WRITE_UUID` synchronously, before any backend I/O) is the only case provably pre-submission, so it alone stays `BluetoothTransportError` and is safe for `Router` to retry. Every other `BleakError`/`TimeoutError` from that call happens inside backend I/O (D-Bus/CoreBluetooth/an ESPHome proxy) where delivery can't be proven either way - field data measured 2 of 3 such write failures had already executed on the vehicle - so `_send` instead races any already-armed broadcast watcher for the rest of the window (a matching broadcast can still confirm success despite the failed write) and, failing that, raises plain `BluetoothTimeout`. Because `BluetoothUnconfirmedCommand` subclasses `BluetoothTimeout`, this lands in the exact same `except BluetoothTimeout` ladder in `_sendVehicleSecurity`/`_sendInfotainment` as a lost post-write ack, with no separate exception type needed; `_send_optimistic` gets the equivalent treatment explicitly since it bypasses that ladder (see above). A read is unaffected - `_getVehicleSecurity`/`_getInfotainment` don't override the ladder, so a write-ambiguous read propagates plain `BluetoothTimeout` and `Router` fails over on it normally, which is safe since a read has no double-execution risk. Tests: `tests/test_ble_send_transport.py` (`SendTransportErrorTests`), `tests/test_ble_broadcast_confirmation.py` (`WriteFailureBroadcastRaceTests`), `tests/test_ble_write_timeout_router.py`. - **`wake_up()` is best-effort; confirm readiness with an INFO read**: `wake_up()` is a VCSEC actuation, so a terminal ack now returns promptly when observed, but an unresolved wake remains only an inconclusive wake signal, not command failure (`BluetoothUnconfirmedCommand` when `raise_unconfirmed=True`, best-effort success by default). Confirm readiness by retrying a cheap INFO read instead (see the boot-delay gotcha above for why the first INFO read still needs its own retry/backoff). Hold one connection across a whole batch of related commands rather than reconnecting between each - reconnecting costs ~123% more per operation with no demonstrated wake-preservation benefit from the connection alone. - **The signed-command retry in `Commands._command` can double-execute a mutating command**: on an `OPERATIONSTATUS_WAIT` reply or an `INCORRECT_EPOCH`/`INVALID_TOKEN` fault, `_command` (`commands.py`) re-signs and re-sends the identical command, bounded at 3 attempts then a clean `{"result": False, "reason": "Too many retries"}` - the cap itself is safe and doesn't loop. Live-verified deterministically: a WAIT-then-OK sequence produces 2 physical sends of the same command on the wire. Combined with the mutating-timeout-is-inconclusive gotcha above (a command can execute despite a WAIT/fault reply), this retry is a latent double-apply window. Harmless for a naturally idempotent command (lock/unlock), a real correctness risk for toggles and step commands (`media_toggle_playback`, `media_volume_up`/`down`, schedule add/remove) - verify those by absolute state after the call, never by counting invocations or trusting the retry to be safe. - **`expects_data` splits BLE reply-waiting: VCSEC actuations return on the terminal ack**: a VCSEC read replies with a bare ACK **then** a data frame, but a VCSEC actuation (RKE/closure/wake via `_sendVehicleSecurity`) replies with a **single bare ACK only** - `_send` cannot tell the two apart at transport level, so the caller declares it. `_sendVehicleSecurity` passes `expects_data=False` down through `_command` into `_send` (`commands.py`/`bluetooth.py`); everything else (VCSEC reads via `_getVehicleSecurity`, all infotainment via `_send/_getInfotainment`, `_handshake`, `pair`) keeps the default `expects_data=True`. With `expects_data=False`, `_send` returns immediately on the matching ACK instead of waiting out `_ack_followup_timeout` (was a flat ~2s tax on every VCSEC actuation), and on a lost ack the public mutating-command wrapper reaches the unresolved `raise_unconfirmed` outcome after the shorter `_actuation_timeout` (2s) rather than `_default_timeout` (5s) - the verify-by-state contract makes a longer wait pointless. Infotainment actuations never paid the tax (their `actionStatus` rides in `protobuf_message_as_bytes`, so `_send` returns on that data frame). The WAIT/fault retry threads `expects_data` through unchanged, so the double-execute exposure noted above is unaffected. diff --git a/README.md b/README.md index 09ad9ea..defbe67 100644 --- a/README.md +++ b/README.md @@ -172,12 +172,14 @@ default with a passive GATT read about every 20 seconds. Pass leaving it enabled can keep an already-awake car awake longer, so disconnect or disable keepalive when vehicle sleep is preferred. -BLE connect/notify and GATT write failures from `VehicleBluetooth` raise +BLE connect/notify failures, and GATT writes rejected before backend I/O, raise `BluetoothTransportError`, a `TeslaFleetError` subclass, with the original -transport exception chained as `__cause__`. Mutating BLE commands use a -confirmation ladder controlled by `confirmation` (`"ack"` by default) and -`raise_unconfirmed` (`False` by default): an inconclusive lost acknowledgement -resolves as best-effort success unless you opt in to +transport exception chained as `__cause__` when available. A GATT write that +entered backend I/O and then failed or timed out is delivery-ambiguous and +raises `BluetoothTimeout`/`BluetoothUnconfirmedCommand` instead. Mutating BLE +commands use a confirmation ladder controlled by `confirmation` (`"ack"` by +default) and `raise_unconfirmed` (`False` by default): an inconclusive lost +acknowledgement resolves as best-effort success unless you opt in to `BluetoothUnconfirmedCommand`, while a command proven not to have applied raises `BluetoothCommandFailed`. See [Bluetooth for Vehicles](docs/bluetooth_vehicles.md) for the full ladder. Catch `TeslaFleetError` to handle Bluetooth transport diff --git a/docs/bluetooth_vehicles.md b/docs/bluetooth_vehicles.md index 075cb44..c18df66 100644 --- a/docs/bluetooth_vehicles.md +++ b/docs/bluetooth_vehicles.md @@ -130,15 +130,25 @@ retry `BluetoothTimeout` with backoff. Keep one BLE connection open across related commands when possible instead of reconnecting for each command. `VehicleBluetooth` raises `BluetoothTransportError`, a `TeslaFleetError` -subclass, when the BLE connection, notification setup, or GATT command write -fails before a vehicle response can be awaited. The original transport -exception is available as the exception's `__cause__`; this includes -`bleak.exc.BleakError` and builtin `TimeoutError` from ESPHome proxy connect, -notify, or write timeouts. Catch `TeslaFleetError` to handle both transport -failures and response-wait timeout failures with one library error hierarchy, -or catch `BluetoothTransportError` separately when you need to distinguish a -transport failure - the command never reached the vehicle - from a vehicle -timeout after the command or request was written. +subclass, when the BLE connection, notification setup, or GATT characteristic +resolution fails *before any write reaches the backend* - provably before the +command could have reached the vehicle. The original transport exception is +available as the exception's `__cause__`. Catch `TeslaFleetError` to handle +both transport failures and response-wait timeout failures with one library +error hierarchy, or catch `BluetoothTransportError` separately when you need +to distinguish a provable pre-submission failure from a vehicle timeout after +the command or request may have been written. + +A GATT write that enters the backend (D-Bus, CoreBluetooth, an ESPHome proxy) +and then fails or times out - `bleak.exc.BleakError`, or builtin `TimeoutError` +from an ESPHome proxy write timeout - is treated as delivery-ambiguous rather +than a transport failure: field measurements show such writes reaching the +vehicle about as often as not, so it raises `BluetoothTimeout` (or, for a +mutating command, `BluetoothUnconfirmedCommand` through the same ladder as a +lost ack - see "Mutating Command Timeouts" below) instead of +`BluetoothTransportError`. A `Router`/`VehicleRouter` primary-fallback chain +(see the top-level README) must not replay a command on this exception, which +is exactly why it is *not* classified as a transport failure. A response-wait timeout for a *mutating* command (RKE/closure actions, HVAC/media/charging commands, `wake_up()`) that stays genuinely unresolved is @@ -195,6 +205,19 @@ outcome. `confirmation` (constructor/factory arg, default `"ack"`) picks how many of those steps run; `raise_unconfirmed` (default `False`) picks what the last step does when nothing above it settled the question. +| Rung | Runs under | On success | On failure/ambiguity | +| --- | --- | --- | --- | +| GATT write | every level, incl. `"optimistic"` | proceeds to the next rung (or returns, under `"optimistic"`) | pre-submission (e.g. characteristic not found): `BluetoothTransportError`, always raises. Submitted-then-failed/timed-out: ambiguous, races any armed broadcast watcher, then falls to the "genuinely unresolved" outcome below | +| Addressed ack + broadcast race (lock/unlock only) | `"ack"`, `"verify"` | returns a confirmed result | a proven mismatch at window end raises `BluetoothCommandFailed`; a lost ack with nothing else confirming falls to the next rung | +| State-read verification | `"verify"` only | returns a confirmed result | a proven mismatch raises `BluetoothCommandFailed`; an unreadable prover (e.g. asleep car) falls to the next rung | +| Genuinely unresolved outcome | every level | - | `raise_unconfirmed=False` (default): best-effort success. `raise_unconfirmed=True`: raises `BluetoothUnconfirmedCommand` | + +The GATT write rung's ambiguous case is not hypothetical: field measurements +of write-level transport errors found some had already executed on the +vehicle despite the local write call failing, so a submitted-then-ambiguous +write is deliberately routed into the same unresolved-outcome handling as a +lost ack rather than treated as a safe-to-retry transport failure. + **Broadcast confirmation (lock/unlock, `"ack"` and `"verify"`).** The vehicle keeps emitting unsolicited VCSEC status broadcasts on the same notification subscription even when it emits no addressed ack for a lock/unlock actuation. @@ -244,20 +267,25 @@ falls through to `raise_unconfirmed` regardless of `confirmation`. - `confirmation="optimistic"` returns success as soon as the GATT write is confirmed, consulting nothing else for every mutating command. This is pure - speed mode: the caller owns any state verification it wants afterward. Only - a write/transport failure (`BluetoothTransportError`) still raises. `ping()` - (the one non-mutating infotainment send) is exempt and always waits for its - real reply. + speed mode: the caller owns any state verification it wants afterward. A + write provably rejected before submission (`BluetoothTransportError`) always + raises; a submitted-then-ambiguous write instead follows `raise_unconfirmed` + like every other rung, since even this mode must not let a fallback router + blind-retry a command that may already have reached the car. `ping()` (the + one non-mutating infotainment send) is exempt and always waits for its real + reply. - `raise_unconfirmed=True` changes what happens only when the whole ladder is exhausted without a definite answer - the ack/broadcast wait timed out and, under `confirmation="verify"`, the prover read itself could not complete - (e.g. the car is asleep). Instead of the default best-effort success - (`{"response": {"result": True, "reason": ""}}`), that ambiguous case raises - `BluetoothUnconfirmedCommand`. A car-side rejection carried in an ack, any - proven non-application (`BluetoothCommandFailed`), and write failures are - unaffected and always raise - this flag converts only the "could not - determine what happened" outcome. It is moot under - `confirmation="optimistic"`, which is never inconclusive. + (e.g. the car is asleep) - or the GATT write itself entered backend I/O and + then failed/timed out with delivery unprovable either way. Instead of the + default best-effort success (`{"response": {"result": True, "reason": ""}}`), + that ambiguous case raises `BluetoothUnconfirmedCommand`. A car-side + rejection carried in an ack, any proven non-application + (`BluetoothCommandFailed`), and a write provably rejected before submission + (`BluetoothTransportError`) are unaffected and always raise - this flag + converts only the "could not determine what happened" outcome, and applies + under every `confirmation` level including `"optimistic"`. ```python vehicle = tesla_bluetooth.vehicles.create( diff --git a/tesla_fleet_api/const.py b/tesla_fleet_api/const.py index e90154d..ec0ea8d 100644 --- a/tesla_fleet_api/const.py +++ b/tesla_fleet_api/const.py @@ -8,9 +8,9 @@ Region = Literal["na", "eu", "cn"] -# BLE command-confirmation ladder depth: "optimistic" consults nothing (fire -# and forget), "ack" waits for an addressed ack or a matching state broadcast, -# "verify" additionally reads back state on an ack/broadcast timeout. +# BLE command-confirmation ladder depth: "optimistic" skips reply waits after a +# confirmed write, "ack" waits for an addressed ack or a matching state +# broadcast, "verify" additionally reads back state on an ack/broadcast timeout. BluetoothConfirmation = Literal["optimistic", "ack", "verify"] SERVERS: dict[Region, str] = { diff --git a/tesla_fleet_api/exceptions.py b/tesla_fleet_api/exceptions.py index 2996e25..2d5cc2b 100644 --- a/tesla_fleet_api/exceptions.py +++ b/tesla_fleet_api/exceptions.py @@ -29,17 +29,19 @@ class BluetoothTimeout(TeslaFleetError): class BluetoothUnconfirmedCommand(BluetoothTimeout): - """A mutating Bluetooth command timed out after it was written to the vehicle. + """A mutating Bluetooth command has an unresolved delivery outcome. - The write succeeded, so the vehicle may have executed the command even - though its ack was lost - lock/unlock have both been observed to execute - despite this exception. Treat the outcome as unknown, not failed: verify - by reading state back when possible, and never blind-retry or re-issue - the same command on another transport, since it may already have run. + The write either completed and its ack was lost, or it entered backend I/O + and then failed/timed out in a way that cannot prove whether the vehicle + received it. The vehicle may have executed the command - lock/unlock have + both been observed to execute despite this exception. Treat the outcome as + unknown, not failed: verify by reading state back when possible, and never + blind-retry or re-issue the same command on another transport, since it may + already have run. Subclasses ``BluetoothTimeout`` so existing ``except BluetoothTimeout`` handling still catches it, while remaining distinguishable from it (and - from ``BluetoothTransportError``, the genuine pre-write transport + from ``BluetoothTransportError``, the provable pre-submission transport failure) for callers that want to react to the ambiguity specifically - e.g. a BLE-primary/cloud-fallback router should not fail over on this exception, since failing over risks double-executing the command. @@ -71,7 +73,16 @@ class BluetoothCommandFailed(TeslaFleetError): class BluetoothTransportError(TeslaFleetError): - """The Bluetooth transport (connect, notify, or GATT write) failed before a vehicle response could be awaited.""" + """The Bluetooth transport failed provably before the write reached the vehicle. + + Covers connect, notify-subscribe, and GATT characteristic-resolution + failures - all raised by the local BLE stack before any bytes go out over + the air, so a fallback router can safely retry the same command + elsewhere. A GATT write that entered backend I/O and then failed or timed + out is delivery-ambiguous instead (the write may have reached the + vehicle) and raises ``BluetoothTimeout``/``BluetoothUnconfirmedCommand``, + not this class - see those exceptions. + """ message = ( "The Bluetooth transport failed before a vehicle response could be awaited." diff --git a/tesla_fleet_api/router/base.py b/tesla_fleet_api/router/base.py index 5509f9a..fec2d48 100644 --- a/tesla_fleet_api/router/base.py +++ b/tesla_fleet_api/router/base.py @@ -51,12 +51,12 @@ class Router(Generic[PrimaryT, SecondaryT]): Dispatch to a callable performs *per-command* failover: the backends that expose the method are attempted in order, and if one raises any exception other than ``BluetoothUnconfirmedCommand`` (a connection failure or a - mid-command transport error such as a write/notify failure or a disconnect), - the same call is automatically retried on the next backend that has it, with - the same arguments. The error only propagates when every applicable backend - fails, in which case the last error is raised. Each attempted backend emits - a ``DEBUG`` log line with the routed command name, backend class, and - success/error result. + provably pre-submission transport error such as notify setup or GATT + characteristic resolution failure), the same call is automatically retried + on the next backend that has it, with the same arguments. The error only + propagates when every applicable backend fails, in which case the last error + is raised. Each attempted backend emits a ``DEBUG`` log line with the routed + command name, backend class, and success/error result. .. warning:: diff --git a/tesla_fleet_api/tesla/tesla.py b/tesla_fleet_api/tesla/tesla.py index 8803968..ca1a53c 100644 --- a/tesla_fleet_api/tesla/tesla.py +++ b/tesla_fleet_api/tesla/tesla.py @@ -15,6 +15,7 @@ from cryptography.hazmat.primitives import serialization from cryptography.hazmat.backends import default_backend + class Tesla: """Base class describing interactions with Tesla products.""" @@ -30,7 +31,11 @@ class Tesla: async def get_private_key( self, path: str = "private_key.pem" ) -> ec.EllipticCurvePrivateKey: - """Get or create the private key.""" + """Get or create the private key. + + The private key is stored as an unencrypted PEM file with permissions + 0o600 when created. + """ if not exists(path): self.private_key = ec.generate_private_key( ec.SECP256R1(), default_backend() @@ -43,6 +48,12 @@ async def get_private_key( ) async with aiofiles.open(path, "wb") as key_file: await key_file.write(pem) + try: + from os import chmod + + chmod(path, 0o600) + except OSError: + pass else: try: async with aiofiles.open(path, "rb") as key_file: @@ -70,20 +81,28 @@ def public_pem(self) -> str: """Get the public key in PEM format.""" if self.private_key is None: raise ValueError("Private key is not set") - return self.private_key.public_key().public_bytes( - encoding=serialization.Encoding.PEM, - format=serialization.PublicFormat.SubjectPublicKeyInfo, - ).decode('utf-8') + return ( + self.private_key.public_key() + .public_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PublicFormat.SubjectPublicKeyInfo, + ) + .decode("utf-8") + ) @property def public_uncompressed_point(self) -> str: """Get the public key in uncompressed point format.""" if self.private_key is None: raise ValueError("Private key is not set") - return self.private_key.public_key().public_bytes( - encoding=serialization.Encoding.X962, - format=serialization.PublicFormat.UncompressedPoint, - ).hex() + return ( + self.private_key.public_key() + .public_bytes( + encoding=serialization.Encoding.X962, + format=serialization.PublicFormat.UncompressedPoint, + ) + .hex() + ) async def get_rsa_private_key( self, path: str = "tedapi_rsa_private.pem", key_size: int = 4096 @@ -153,7 +172,11 @@ def rsa_public_pem(self) -> str: """Get the RSA public key in PEM (SubjectPublicKeyInfo) format.""" if self.rsa_private_key is None: raise ValueError("RSA private key is not set") - return self.rsa_private_key.public_key().public_bytes( - encoding=serialization.Encoding.PEM, - format=serialization.PublicFormat.SubjectPublicKeyInfo, - ).decode("utf-8") + return ( + self.rsa_private_key.public_key() + .public_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PublicFormat.SubjectPublicKeyInfo, + ) + .decode("utf-8") + ) diff --git a/tesla_fleet_api/tesla/vehicle/bluetooth.py b/tesla_fleet_api/tesla/vehicle/bluetooth.py index 3447d57..204f224 100644 --- a/tesla_fleet_api/tesla/vehicle/bluetooth.py +++ b/tesla_fleet_api/tesla/vehicle/bluetooth.py @@ -11,7 +11,7 @@ from bleak import BleakClient, BleakScanner from bleak.backends.characteristic import BleakGATTCharacteristic from bleak.backends.device import BLEDevice -from bleak.exc import BleakError +from bleak.exc import BleakCharacteristicNotFoundError, BleakError from bleak_retry_connector import MAX_CONNECT_ATTEMPTS, establish_connection from cryptography.hazmat.primitives.asymmetric import ec from google.protobuf.message import DecodeError @@ -302,16 +302,27 @@ class VehicleBluetooth(Commands[BluetoothParentT], Generic[BluetoothParentT]): """Class describing the Tesla Fleet API vehicle endpoints and commands for a specific vehicle with command signing. Callers can catch failures from this class with a single ``TeslaFleetError``, - but distinct outcomes hide behind that base: a connect/notify/write - transport failure before any command reached the vehicle - (``BluetoothTransportError``), an ack-wait timeout for a *mutating* - command that was already written to the vehicle and whose outcome is - genuinely unresolved (``BluetoothUnconfirmedCommand``), a mutating command - *proven* not to have taken effect (``BluetoothCommandFailed``), and a - plain response-wait timeout for anything else, e.g. a state read + but distinct outcomes hide behind that base: a connect/notify failure or a + GATT write provably rejected before it reached backend I/O + (``BluetoothTransportError``), an ack-wait timeout or a submitted-then- + ambiguous GATT write for a *mutating* command whose outcome is genuinely + unresolved (``BluetoothUnconfirmedCommand``), a mutating command *proven* + not to have taken effect (``BluetoothCommandFailed``), and a plain + response-wait timeout - including a write that entered backend I/O and + then failed or timed out - for anything else, e.g. a state read (``BluetoothTimeout``). Each preserves the underlying failure in its cause chain when one exists. + The write/transport split matters because a GATT write that reaches the + backend (D-Bus, CoreBluetooth, an ESPHome proxy) cannot be proven to have + missed the vehicle just because the local call then failed or timed out - + field data shows such writes executing on the car about as often as not. + Only a rejection the local BLE stack raises before touching the backend + (e.g. ``BleakCharacteristicNotFoundError`` resolving the write + characteristic) is provably pre-submission and safe for a fallback router + to retry; everything past that point is treated as ambiguous, exactly + like a lost ack. + A ``BluetoothUnconfirmedCommand`` (RKE/closure actions, HVAC/media/charging commands, ``wake_up``) means the vehicle can have executed the command without its ack reaching the client - it is unconfirmed, not failed. @@ -332,9 +343,13 @@ class VehicleBluetooth(Commands[BluetoothParentT], Generic[BluetoothParentT]): flag per rung: - ``"optimistic"``: return success as soon as the GATT write is confirmed, - consulting nothing else. Only a write/transport failure - (``BluetoothTransportError``) still raises. Pure speed mode - the caller - owns any state verification it wants afterward. + consulting nothing else. A write provably rejected before submission + still raises ``BluetoothTransportError`` unconditionally; a + submitted-then-ambiguous write instead follows ``raise_unconfirmed`` + like every other rung, since even this "don't wait" mode must not let a + fallback router blind-retry a command that may already have reached + the car. Otherwise the caller owns any state verification it wants + afterward. - ``"ack"`` (default): wait for the addressed ack. For a VCSEC actuation with a derivable expected end state (currently lock/unlock only), the wait also races a matching status broadcast against the ack: the vehicle @@ -370,13 +385,16 @@ class VehicleBluetooth(Commands[BluetoothParentT], Generic[BluetoothParentT]): ``raise_unconfirmed`` (default ``False``) is the orthogonal question of what to do once a ladder genuinely cannot resolve - the ack/broadcast wait (and, under ``"verify"``, the prover read) neither confirmed nor - contradicted the request. Default ``False`` resolves that case as a - best-effort success; ``True`` raises ``BluetoothUnconfirmedCommand`` - instead. It is moot under ``confirmation="optimistic"``, which is never - inconclusive. A car-side rejection carried in an ack, any proven - non-application (``BluetoothCommandFailed``), and write failures are - unaffected by this flag and always raise - it only converts the "could not - determine what happened" outcome. + contradicted the request, or the GATT write itself entered backend I/O and + then failed/timed out with delivery unprovable either way. Default + ``False`` resolves that case as a best-effort success; ``True`` raises + ``BluetoothUnconfirmedCommand`` instead. This applies under every + ``confirmation`` level, including ``"optimistic"`` - a write's delivery + can be ambiguous even when no reply is ever awaited. A car-side rejection + carried in an ack, any proven non-application (``BluetoothCommandFailed``), + and a write provably rejected before submission + (``BluetoothTransportError``) are unaffected by this flag and always + raise - it only converts the "could not determine what happened" outcome. ``keepalive_interval`` (default ~20s, ``None``/``0`` disables) keeps an otherwise idle held connection from dropping: after that many seconds with @@ -700,16 +718,52 @@ async def _send( broadcast_future, broadcast_watcher = self._arm_broadcast_confirmation( domain, confirm_broadcast, mismatches, lambda: write_complete ) + if not self.client.is_connected: + self._disarm_broadcast_confirmation(domain, broadcast_watcher) + raise BluetoothTransportError try: await self.client.write_gatt_char(WRITE_UUID, payload, True) write_complete = True self._last_activity = time.monotonic() - # bleak-esphome converts an aioesphomeapi write timeout into a - # builtin TimeoutError, not a BleakError, so catch both to keep the - # GATT-write transport failure within TeslaFleetError. - except (BleakError, TimeoutError) as e: + except BleakCharacteristicNotFoundError as e: + # Raised synchronously while bleak resolves WRITE_UUID against + # the connected GATT server, strictly before any backend I/O - + # the write never reached the transport, so a fallback router + # can safely retry it on another backend. self._disarm_broadcast_confirmation(domain, broadcast_watcher) raise BluetoothTransportError from e + except (BleakError, TimeoutError) as e: + # Every other failure here originates inside backend I/O (the + # D-Bus/CoreBluetooth/ESPHome-proxy write call) where delivery + # can't be proven either way - bleak-esphome maps an + # aioesphomeapi write timeout to a bare TimeoutError, and even + # a BleakError can follow actual radio transmission (field + # data: 2 of 3 such write failures had executed). Give an + # already-armed broadcast watcher the rest of the window to + # resolve it, then raise BluetoothTimeout - not + # BluetoothTransportError - so a mutating command's + # confirmation ladder treats this like a lost ack instead of a + # proven miss a router could safely retry. + if optimistic: + self._disarm_broadcast_confirmation(domain, broadcast_watcher) + raise BluetoothTimeout from e + try: + if confirm_broadcast is None: + return await self._await_response( + domain, msg, requires, expects_data, timeout + ) + return await self._await_response_or_broadcast( + domain, + msg, + requires, + expects_data, + timeout, + broadcast_future, + broadcast_watcher, + mismatches, + ) + except BluetoothTimeout as timeout_exc: + raise BluetoothTimeout(timeout_exc.data, timeout_exc.status) from e if optimistic: return RoutableMessage() @@ -1032,11 +1086,17 @@ async def _ensure_handshake(self, domain: Domain) -> None: if not self._sessions[domain].ready: raise BluetoothTimeout() - async def _send_optimistic(self, domain: Domain, command: bytes) -> dict[str, Any]: + async def _send_optimistic( + self, domain: Domain, command: bytes, name: str + ) -> dict[str, Any]: """Sign and write a mutating command without waiting for any reply. - Only a write/transport failure (``BluetoothTransportError``) raises; - the caller owns any state verification it wants afterward. + A write provably rejected before submission (``BluetoothTransportError``) + always raises. A write that entered backend I/O and then failed or + timed out is delivery-ambiguous - even this "don't wait" mode must not + let a fallback router blind-retry a command that may already have + reached the car - so it follows ``raise_unconfirmed`` like every other + rung instead of raising unconditionally. """ session = self._sessions[domain] async with session.lock: @@ -1044,7 +1104,11 @@ async def _send_optimistic(self, domain: Domain, command: bytes) -> dict[str, An msg = await self._commandHmac(session, command) else: msg = await self._commandAes(session, command) - await self._send(msg, "protobuf_message_as_bytes", optimistic=True) + try: + await self._send(msg, "protobuf_message_as_bytes", optimistic=True) + except BluetoothTimeout as timeout: + unconfirmed = BluetoothUnconfirmedCommand(timeout.data, timeout.status) + return self._unconfirmed_outcome(name, unconfirmed, cause=timeout) return {"response": {"result": True, "reason": ""}} def _unconfirmed_outcome( @@ -1101,15 +1165,19 @@ async def _sendVehicleSecurity( non-application and raises ``BluetoothCommandFailed`` - not the ambiguous ``BluetoothUnconfirmedCommand`` - since a fallback router can safely fail over on a command proven not to have applied. With - ``confirmation="optimistic"``, the write itself is the whole outcome - and none of this ladder runs. With ``raise_unconfirmed`` off, only a - ladder that is still genuinely unresolved resolves as a best-effort - success instead of raising - see the class docstring. + ``confirmation="optimistic"``, a confirmed write is the whole outcome + and no ack/broadcast/verify rung runs - but a delivery-ambiguous write + failure still follows ``raise_unconfirmed`` (see ``_send_optimistic``). + With ``raise_unconfirmed`` off, only a ladder that is still genuinely + unresolved resolves as a best-effort success instead of raising - see + the class docstring. """ await self._ensure_handshake(Domain.DOMAIN_VEHICLE_SECURITY) if self.confirmation == "optimistic": return await self._send_optimistic( - Domain.DOMAIN_VEHICLE_SECURITY, command.SerializeToString() + Domain.DOMAIN_VEHICLE_SECURITY, + command.SerializeToString(), + vcsec_command_name(command), ) plan = _vcsec_verify_plan(command) try: @@ -1149,7 +1217,9 @@ async def _sendInfotainment( await self._ensure_handshake(Domain.DOMAIN_INFOTAINMENT) if self.confirmation == "optimistic" and mutating: return await self._send_optimistic( - Domain.DOMAIN_INFOTAINMENT, command.SerializeToString() + Domain.DOMAIN_INFOTAINMENT, + command.SerializeToString(), + infotainment_command_name(command), ) try: return await super()._sendInfotainment(command) diff --git a/tesla_fleet_api/tesla/vehicle/vehicles.py b/tesla_fleet_api/tesla/vehicle/vehicles.py index 8bf695c..822acf8 100644 --- a/tesla_fleet_api/tesla/vehicle/vehicles.py +++ b/tesla_fleet_api/tesla/vehicle/vehicles.py @@ -57,9 +57,9 @@ def createBluetooth( """Creates a bluetooth vehicle that uses command protocol. ``confirmation`` sets the confirmation ladder depth: ``"optimistic"`` - consults nothing, ``"ack"`` (default) waits for an addressed ack or a - matching state broadcast, ``"verify"`` additionally reads back state - on an ack/broadcast timeout. + skips reply waits after a confirmed write, ``"ack"`` (default) waits + for an addressed ack or a matching state broadcast, ``"verify"`` + additionally reads back state on an ack/broadcast timeout. ``keepalive_interval`` seconds of GATT idleness triggers a passive read to hold the link open (``None``/``0`` disables). ``raise_unconfirmed=True`` raises ``BluetoothUnconfirmedCommand`` @@ -113,9 +113,9 @@ def create( """Creates a bluetooth vehicle that uses command protocol. ``confirmation`` sets the confirmation ladder depth: ``"optimistic"`` - consults nothing, ``"ack"`` (default) waits for an addressed ack or a - matching state broadcast, ``"verify"`` additionally reads back state - on an ack/broadcast timeout. + skips reply waits after a confirmed write, ``"ack"`` (default) waits + for an addressed ack or a matching state broadcast, ``"verify"`` + additionally reads back state on an ack/broadcast timeout. ``keepalive_interval`` seconds of GATT idleness triggers a passive read to hold the link open (``None``/``0`` disables). ``raise_unconfirmed=True`` raises ``BluetoothUnconfirmedCommand`` @@ -150,9 +150,9 @@ def createBluetooth( """Creates a bluetooth vehicle that uses command protocol. ``confirmation`` sets the confirmation ladder depth: ``"optimistic"`` - consults nothing, ``"ack"`` (default) waits for an addressed ack or a - matching state broadcast, ``"verify"`` additionally reads back state - on an ack/broadcast timeout. + skips reply waits after a confirmed write, ``"ack"`` (default) waits + for an addressed ack or a matching state broadcast, ``"verify"`` + additionally reads back state on an ack/broadcast timeout. ``keepalive_interval`` seconds of GATT idleness triggers a passive read to hold the link open (``None``/``0`` disables). ``raise_unconfirmed=True`` raises ``BluetoothUnconfirmedCommand`` diff --git a/tests/test_ble_broadcast_confirmation.py b/tests/test_ble_broadcast_confirmation.py index 949ce37..e513502 100644 --- a/tests/test_ble_broadcast_confirmation.py +++ b/tests/test_ble_broadcast_confirmation.py @@ -16,6 +16,7 @@ from unittest import IsolatedAsyncioTestCase from unittest.mock import AsyncMock, MagicMock +from bleak.exc import BleakError from cryptography.hazmat.primitives.asymmetric import ec from tesla_fleet_api.exceptions import ( @@ -354,3 +355,73 @@ async def deliver_verify_read_reply() -> None: result = await asyncio.wait_for(task, timeout=2.0) self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + + +class WriteFailureBroadcastRaceTests(IsolatedAsyncioTestCase): + """A GATT write failure/timeout still lets an already-armed broadcast resolve it. + + The broadcast watcher is armed before ``write_gatt_char`` is even called, + so a matching broadcast that arrives while (or shortly after) + ``write_gatt_char`` raises must still confirm the command - the write + failure doesn't prove the command never reached the vehicle. + """ + + async def test_broadcast_confirms_despite_write_failure(self) -> None: + vehicle = _make_vehicle() + vehicle._actuation_timeout = 5.0 + + async def write_raises_then_broadcasts(*_: Any) -> None: + vehicle._on_message(_locked_broadcast()) + await asyncio.sleep(0) + raise BleakError("write failed") + + vehicle.client.write_gatt_char = AsyncMock( + side_effect=write_raises_then_broadcasts + ) + + result = await asyncio.wait_for(vehicle.door_lock(), timeout=0.5) + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + + async def test_addressed_rejection_surfaces_despite_write_failure(self) -> None: + vehicle = _make_vehicle() + vehicle._actuation_timeout = 5.0 + + async def write_raises_after_rejection(*_: Any) -> None: + vehicle._on_message(_addressed_rejection_ack(vehicle)) + await asyncio.sleep(0) + raise BleakError("write failed") + + vehicle.client.write_gatt_char = AsyncMock( + side_effect=write_raises_after_rejection + ) + + result = await asyncio.wait_for(vehicle.door_lock(), timeout=0.5) + + self.assertEqual(result["response"]["result"], False) + + async def test_addressed_ack_surfaces_despite_write_failure(self) -> None: + vehicle = _make_vehicle() + vehicle._actuation_timeout = 5.0 + + async def write_raises_after_ack(*_: Any) -> None: + vehicle._on_message(_addressed_ok_ack(vehicle)) + await asyncio.sleep(0) + raise BleakError("write failed") + + vehicle.client.write_gatt_char = AsyncMock(side_effect=write_raises_after_ack) + + result = await asyncio.wait_for(vehicle.door_lock(), timeout=0.5) + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + + async def test_write_timeout_with_no_broadcast_raises_unconfirmed(self) -> None: + vehicle = _make_vehicle(raise_unconfirmed=True) + vehicle._actuation_timeout = 0.05 + + vehicle.client.write_gatt_char = AsyncMock( + side_effect=TimeoutError("write timed out") + ) + + with self.assertRaises(BluetoothUnconfirmedCommand): + await asyncio.wait_for(vehicle.door_lock(), timeout=1.0) diff --git a/tests/test_ble_optimistic_and_best_effort.py b/tests/test_ble_optimistic_and_best_effort.py index f984fda..91b6668 100644 --- a/tests/test_ble_optimistic_and_best_effort.py +++ b/tests/test_ble_optimistic_and_best_effort.py @@ -5,13 +5,14 @@ - ``optimistic`` (default off) skips the ack wait and ``verify_commands`` entirely for a mutating command - a confirmed GATT write is the whole - outcome. Only a write/transport failure still raises. + outcome. A write provably rejected before submission always raises; a + submitted-then-ambiguous write instead follows ``raise_unconfirmed``. - ``raise_unconfirmed`` (default on, i.e. current behavior) controls what happens when the ladder is exhausted without a positive or negative answer: off resolves that ambiguous case 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. + ``verify_commands`` state mismatch, and a write provably rejected before + submission are unaffected and always raise. """ from __future__ import annotations @@ -62,6 +63,27 @@ async def test_write_failure_still_raises(self) -> None: with self.assertRaises(BluetoothTransportError): await vehicle.door_lock() + async def test_ambiguous_write_follows_raise_unconfirmed_true(self) -> None: + # A submitted-then-timed-out write is delivery-ambiguous, not a + # provable pre-submission failure, so it must not raise + # BluetoothTransportError - it follows raise_unconfirmed like every + # other rung, even though optimistic mode never waits for a reply. + vehicle, send = self.make_vehicle(optimistic=True, raise_unconfirmed=True) + send.side_effect = BluetoothTimeout() + + with self.assertRaises(BluetoothUnconfirmedCommand): + await vehicle.door_lock() + + async def test_ambiguous_write_resolves_best_effort_when_raise_unconfirmed_off( + self, + ) -> None: + vehicle, send = self.make_vehicle(optimistic=True, raise_unconfirmed=False) + send.side_effect = BluetoothTimeout() + + result = await vehicle.door_lock() + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + async def test_non_mutating_infotainment_is_unaffected(self) -> None: # ping() always waits for its real reply; optimistic mode is only for # mutating commands. diff --git a/tests/test_ble_send_transport.py b/tests/test_ble_send_transport.py index f0a7c94..1196bbe 100644 --- a/tests/test_ble_send_transport.py +++ b/tests/test_ble_send_transport.py @@ -13,7 +13,7 @@ from unittest import IsolatedAsyncioTestCase from unittest.mock import AsyncMock, MagicMock, patch -from bleak.exc import BleakError +from bleak.exc import BleakCharacteristicNotFoundError, BleakError from cryptography.hazmat.primitives.asymmetric import ec from tesla_fleet_api.exceptions import BluetoothTimeout, BluetoothTransportError @@ -245,12 +245,21 @@ async def test_no_response_raises_bluetooth_timeout(self) -> None: class SendTransportErrorTests(IsolatedAsyncioTestCase): - async def test_mid_write_gatt_failure_raises_bluetooth_transport_error( + """Write-level failures split on delivery certainty. + + A characteristic-resolution failure is raised by bleak synchronously, + before any backend I/O - provably pre-submission, so it stays + ``BluetoothTransportError``. Everything else from ``write_gatt_char`` + happens inside backend I/O, where delivery can't be proven either way, so + it is treated like a lost ack (``BluetoothTimeout``), not a provable miss. + """ + + async def test_characteristic_not_found_raises_bluetooth_transport_error( self, ) -> None: vehicle = _make_vehicle() msg = _outgoing() - underlying = BleakError("write failed") + underlying = BleakCharacteristicNotFoundError("write-uuid") vehicle.client.write_gatt_char = AsyncMock(side_effect=underlying) with self.assertRaises(BluetoothTransportError) as ctx: @@ -258,7 +267,34 @@ async def test_mid_write_gatt_failure_raises_bluetooth_transport_error( self.assertIs(ctx.exception.__cause__, underlying) - async def test_write_gatt_timeout_raises_bluetooth_transport_error( + async def test_disconnected_client_raises_bluetooth_transport_error_without_write( + self, + ) -> None: + vehicle = _make_vehicle() + msg = _outgoing() + vehicle.client.is_connected = False + vehicle.client.write_gatt_char = AsyncMock() + + with self.assertRaises(BluetoothTransportError): + await vehicle._send(msg, "protobuf_message_as_bytes") + + vehicle.client.write_gatt_char.assert_not_awaited() + + async def test_mid_write_gatt_failure_raises_bluetooth_timeout( + self, + ) -> None: + vehicle = _make_vehicle() + msg = _outgoing() + underlying = BleakError("write failed") + vehicle.client.write_gatt_char = AsyncMock(side_effect=underlying) + + with self.assertRaises(BluetoothTimeout) as ctx: + await vehicle._send(msg, "protobuf_message_as_bytes") + + self.assertNotIsInstance(ctx.exception, BluetoothTransportError) + self.assertIs(ctx.exception.__cause__, underlying) + + async def test_write_gatt_timeout_raises_bluetooth_timeout( self, ) -> None: # bleak-esphome surfaces an aioesphomeapi GATT-write timeout as a @@ -271,9 +307,10 @@ async def test_write_gatt_timeout_raises_bluetooth_transport_error( ) vehicle.client.write_gatt_char = AsyncMock(side_effect=underlying) - with self.assertRaises(BluetoothTransportError) as ctx: + with self.assertRaises(BluetoothTimeout) as ctx: await vehicle._send(msg, "protobuf_message_as_bytes") + self.assertNotIsInstance(ctx.exception, BluetoothTransportError) self.assertIs(ctx.exception.__cause__, underlying) diff --git a/tests/test_ble_write_timeout_router.py b/tests/test_ble_write_timeout_router.py new file mode 100644 index 0000000..854a1c2 --- /dev/null +++ b/tests/test_ble_write_timeout_router.py @@ -0,0 +1,129 @@ +"""Regression test for the write-timeout double-execution gap. + +Before this fix, a GATT write that was submitted and then timed out waiting +for the ATT write completion raised ``BluetoothTransportError`` - the same +exception as a provable pre-submission failure - so ``Router``/``VehicleRouter`` +treated it as safe to retry and failed over to the cloud secondary. Field data +showed such writes had often already reached the vehicle, so failing over +risked double-executing a non-idempotent command (e.g. door_lock/door_unlock). + +This drives the real (unmocked) ``VehicleBluetooth`` send state machine - +only the GATT client is faked, exactly like ``test_ble_broadcast_confirmation`` +- wrapped in a real ``VehicleRouter`` with a fake cloud fallback, and asserts +the fallback's ``door_lock`` is never invoked when the primary's write is +delivery-ambiguous. +""" + +from __future__ import annotations + +from typing import Any, cast +from unittest import IsolatedAsyncioTestCase +from unittest.mock import AsyncMock, MagicMock + +from bleak.exc import BleakError +from cryptography.hazmat.primitives.asymmetric import ec + +from tesla_fleet_api.exceptions import ( + BluetoothTransportError, + BluetoothUnconfirmedCommand, +) +from tesla_fleet_api.router import VehicleRouter +from tesla_fleet_api.tesla.vehicle.bluetooth import VehicleBluetooth + +VIN = "5YJXCAE43LF123456" + + +def _make_primary(**kwargs: Any) -> VehicleBluetooth[Any]: + """A VehicleBluetooth with real send/ladder logic but a faked GATT client.""" + parent = MagicMock() + parent.private_key = ec.generate_private_key(ec.SECP256R1()) + vehicle = VehicleBluetooth(parent, VIN, **kwargs) + vehicle.connect_if_needed = AsyncMock() # type: ignore[method-assign] + vehicle.client = MagicMock() + + sessions = cast("dict[int, Any]", getattr(vehicle, "_sessions")) + for session in sessions.values(): + session.epoch = b"\x00" * 16 + session.hmac = b"\x00" * 32 + session.delta = 0 + session.sharedKey = b"\x00" * 16 + return vehicle + + +class _FakeCloudFallback: + """A fake cloud secondary tracking whether it was ever invoked.""" + + def __init__(self) -> None: + self.vin = VIN + self.door_lock_calls = 0 + + async def door_lock(self) -> dict[str, Any]: + self.door_lock_calls += 1 + return {"response": {"result": True, "reason": ""}} + + +class WriteTimeoutDoesNotFailOverTests(IsolatedAsyncioTestCase): + async def test_submitted_then_timed_out_write_does_not_replay_on_fallback( + self, + ) -> None: + primary = _make_primary(raise_unconfirmed=True) + primary._actuation_timeout = 0.05 + # No verify plan is armed (confirm_broadcast is only raced when it + # resolves); a bare write timeout with nothing else confirming must + # still be treated as ambiguous, not a provable miss. + primary.client.write_gatt_char = AsyncMock( + side_effect=TimeoutError("write timed out") + ) + fallback = _FakeCloudFallback() + router = VehicleRouter(primary, fallback) + + with self.assertRaises(BluetoothUnconfirmedCommand): + await router.door_lock() + + self.assertEqual(fallback.door_lock_calls, 0) + + async def test_mid_write_bleak_error_does_not_replay_on_fallback(self) -> None: + primary = _make_primary(raise_unconfirmed=True) + primary._actuation_timeout = 0.05 + primary.client.write_gatt_char = AsyncMock( + side_effect=BleakError("write failed") + ) + fallback = _FakeCloudFallback() + router = VehicleRouter(primary, fallback) + + with self.assertRaises(BluetoothUnconfirmedCommand): + await router.door_lock() + + self.assertEqual(fallback.door_lock_calls, 0) + + async def test_provable_pre_submission_failure_still_fails_over(self) -> None: + # Contrast: a failure the local BLE stack raises before any backend + # I/O (never reaches the vehicle) is safe to retry, and still does. + primary = _make_primary(raise_unconfirmed=True) + primary._actuation_timeout = 0.05 + primary.connect_if_needed = AsyncMock( # type: ignore[method-assign] + side_effect=BluetoothTransportError() + ) + fallback = _FakeCloudFallback() + router = VehicleRouter(primary, fallback) + + result = await router.door_lock() + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + self.assertEqual(fallback.door_lock_calls, 1) + + async def test_default_raise_unconfirmed_off_resolves_best_effort_no_failover( + self, + ) -> None: + primary = _make_primary() # raise_unconfirmed defaults False + primary._actuation_timeout = 0.05 + primary.client.write_gatt_char = AsyncMock( + side_effect=TimeoutError("write timed out") + ) + fallback = _FakeCloudFallback() + router = VehicleRouter(primary, fallback) + + result = await router.door_lock() + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + self.assertEqual(fallback.door_lock_calls, 0) diff --git a/tests/test_tesla_private_key.py b/tests/test_tesla_private_key.py new file mode 100644 index 0000000..487ada5 --- /dev/null +++ b/tests/test_tesla_private_key.py @@ -0,0 +1,29 @@ +"""``get_private_key`` must write its PEM file with owner-only permissions. + +The key it creates signs Tesla vehicle commands (Fleet API and BLE) - written +with the process default mode (typically group/world-readable), it leaks +command-signing material to any other local user. ``get_rsa_private_key`` +(the TEDapi/Powerwall key created later in the same module) already chmods to +0o600 after writing; this locks ``get_private_key`` into the same contract. +""" + +from __future__ import annotations + +import stat +import tempfile +from pathlib import Path +from unittest import IsolatedAsyncioTestCase + +from tesla_fleet_api.tesla.tesla import Tesla + + +class GetPrivateKeyPermissionsTests(IsolatedAsyncioTestCase): + async def test_new_key_file_is_owner_only(self) -> None: + with tempfile.TemporaryDirectory() as tmp_dir: + path = str(Path(tmp_dir) / "private_key.pem") + tesla = Tesla() + + await tesla.get_private_key(path) + + mode = stat.S_IMODE(Path(path).stat().st_mode) + self.assertEqual(mode, 0o600) diff --git a/uv.lock b/uv.lock index c6db893..7011709 100644 --- a/uv.lock +++ b/uv.lock @@ -728,7 +728,7 @@ wheels = [ [[package]] name = "tesla-fleet-api" -version = "1.6.4" +version = "1.7.0" source = { editable = "." } dependencies = [ { name = "aiofiles" },