diff --git a/AGENTS.md b/AGENTS.md index b0c2552..bcd0784 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -76,7 +76,7 @@ Commands (vehicle/commands.py) - protobuf-based signed command implementation (A `Vehicles` (vehicle/vehicles.py) is a `dict[str, Vehicle]` with factory methods: - `createFleet(vin)` → `VehicleFleet` - `createSigned(vin)` → `VehicleSigned` -- `createBluetooth(vin, verify_commands=False)` → `VehicleBluetooth` +- `createBluetooth(vin, verify_commands=False, keepalive_interval=..., optimistic=False, raise_unconfirmed=True)` → `VehicleBluetooth` Teslemetry/Tessie override `Vehicles` with their own vehicle classes (`TeslemetryVehicle`, `TessieVehicle`) extending `VehicleFleet` with service-specific commands (e.g., `closure()`, `seat_heater()` for Teslemetry; `wake()`, `lock()` for Tessie). @@ -127,7 +127,8 @@ 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. -- **Opt-in `verify_commands` resolves the inconclusive mutating-timeout inside `VehicleBluetooth`**: constructor knob `verify_commands` (default off, `bluetooth.py`; threaded through `Vehicles`/`VehiclesBluetooth.createBluetooth`) that, on a `BluetoothUnconfirmedCommand` from a mutating command whose expected post-state is derivable from its args, reads the mapped prover state and returns a normal success dict if it matches or re-raises the unconfirmed timeout if not. Verify-on-timeout only (not verify-always): the post-PR-59 happy path returns on the terminal ack, so the prover read is paid only in the ambiguous case - "faster and still reliable". Intercepts at the `_sendVehicleSecurity`/`_sendInfotainment` seam (each called exactly once per public mutation; reads go through `_get*`, so a verification read never recurses into verification). Lives here rather than upstream because BLE is the only transport with the false-negative-ack problem; with it enabled, an unresolved timeout still reaches the caller as `BluetoothUnconfirmedCommand` (see below), so `VehicleRouter` never double-executes regardless of whether verification confirms, denies, or can't tell. Mapping tables and predicates are in `bluetooth.py` (`_vcsec_verify_plan`/`_INFOTAINMENT_VERIFY_PLANS`); only clearly-derivable, absolute commands are covered (lock/unlock, `set_charge_limit`, `set_charging_amps`, `adjust_volume` absolute, `set_temps`, `auto_conditioning_start/stop`). True toggles, relative volume steps, and ack-only actions have no plan and raise `BluetoothUnconfirmedCommand`. The VCSEC lock prover reads while asleep; INFO provers need the car awake and never wake it - an asleep-car read failure re-raises the unconfirmed timeout. See `docs/bluetooth_vehicles.md` for the user-facing table. +- **Opt-in `verify_commands` resolves the inconclusive mutating-timeout inside `VehicleBluetooth`**: constructor knob `verify_commands` (default off, `bluetooth.py`; threaded through `Vehicles`/`VehiclesBluetooth.createBluetooth`) that, on a `BluetoothUnconfirmedCommand` from a mutating command whose expected post-state is derivable from its args, reads the mapped prover state and returns a normal success dict if it matches, raises on a state mismatch, or leaves the outcome unresolved when there is no plan/read. Verify-on-timeout only (not verify-always): the post-PR-59 happy path returns on the terminal ack, so the prover read is paid only in the ambiguous case - "faster and still reliable". Intercepts at the `_sendVehicleSecurity`/`_sendInfotainment` seam (each called exactly once per public mutation; reads go through `_get*`, so a verification read never recurses into verification). Lives here rather than upstream because BLE is the only transport with the false-negative-ack problem; with it enabled and `raise_unconfirmed=True`, an unresolved timeout still reaches the caller as `BluetoothUnconfirmedCommand` (see below), so `VehicleRouter` never double-executes regardless of whether verification confirms, denies, or can't tell. Mapping tables and predicates are in `bluetooth.py` (`_vcsec_verify_plan`/`_INFOTAINMENT_VERIFY_PLANS`); only clearly-derivable, absolute commands are covered (lock/unlock, `set_charge_limit`, `set_charging_amps`, `adjust_volume` absolute, `set_temps`, `auto_conditioning_start/stop`). With default `raise_unconfirmed=True`, true toggles, relative volume steps, and ack-only actions have no plan and raise `BluetoothUnconfirmedCommand`. The VCSEC lock prover reads while asleep; INFO provers need the car awake and never wake it - an asleep-car read failure leaves the outcome unresolved. See `docs/bluetooth_vehicles.md` for the user-facing table. +- **`optimistic` and `raise_unconfirmed` are the two consumer-facing knobs on the same confirmation ladder as `verify_commands`**: both constructor/factory args (default off/on respectively - current behavior - threaded through `Vehicles`/`VehiclesBluetooth.create*` like `verify_commands`), both BLE-only, cloud paths untouched. `optimistic=True` short-circuits `_sendVehicleSecurity`/`_sendInfotainment` (`bluetooth.py`) to `_send_optimistic()`, which signs and writes but never waits for any reply - only a write/transport failure (`BluetoothTransportError`) can raise, and `verify_commands`/`raise_unconfirmed` are never consulted. `raise_unconfirmed=False` only changes what an *exhausted, ambiguous* ladder resolves to (best-effort success instead of raising `BluetoothUnconfirmedCommand`) - a car-side rejection, a `verify_commands` state mismatch, and write failures are unaffected and always raise regardless. `_resolve_timeout()` was reshaped to return `dict | None` (`None` = "couldn't even attempt verification", the ambiguous case) instead of always raising, so the mismatch branch (raises `timeout` directly) stays distinguishable from the ambiguous one (routed through `_unconfirmed_outcome()`, which applies `raise_unconfirmed`). `Router` needed zero changes: `optimistic`'s only exception (`BluetoothTransportError`) already fails over correctly as a generic exception, and `raise_unconfirmed=False`'s best-effort success is just a normal return value. - **`BluetoothUnconfirmedCommand` (`exceptions.py`) is the ack-timeout-after-write case, and `Router` treats it specially**: `_sendVehicleSecurity`/`_sendInfotainment` (`bluetooth.py`, the mutating-command seam, unconditionally, not gated by `verify_commands`) wrap a caught `BluetoothTimeout` into this subclass before it can reach a caller - it means the write succeeded and the vehicle may have executed the command despite the lost ack. 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`. Subclassing `BluetoothTimeout` keeps every existing `except BluetoothTimeout` site (verify_commands, `pair()`'s fast path) working unchanged. `Router._dispatch` (`router/base.py`) special-cases this one exception type to skip its normal per-command failover and re-raise immediately instead - replaying an already-possibly-executed mutating command on the next backend is exactly the double-execution `verify_commands` above exists to prevent, so a BLE-primary/cloud-fallback `VehicleRouter` no longer risks it even with `verify_commands` off. - **`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 `BluetoothUnconfirmedCommand` is still only an inconclusive wake signal, not command failure. Call it best-effort (catch and ignore timeout) and 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. diff --git a/README.md b/README.md index 03d12da..e5771c7 100644 --- a/README.md +++ b/README.md @@ -174,10 +174,13 @@ disable keepalive when vehicle sleep is preferred. BLE connect/notify and GATT write failures from `VehicleBluetooth` raise `BluetoothTransportError`, a `TeslaFleetError` subclass, with the original -transport exception chained as `__cause__`. A response-wait timeout from a -mutating BLE command raises `BluetoothUnconfirmedCommand`, a +transport exception chained as `__cause__`. By default, a response-wait timeout +from a mutating BLE command raises `BluetoothUnconfirmedCommand`, a `BluetoothTimeout` subclass, because the vehicle may have executed it despite a -lost acknowledgement. Catch `TeslaFleetError` to handle Bluetooth transport +lost acknowledgement. The BLE-only `optimistic` and `raise_unconfirmed` +factory options can instead resolve some ambiguous timeouts as best-effort +successes; see [Bluetooth for Vehicles](docs/bluetooth_vehicles.md) for the +full confirmation ladder. Catch `TeslaFleetError` to handle Bluetooth transport failures (including `bleak.exc.BleakError` and builtin `TimeoutError` from ESPHome proxies) and response-wait timeouts through the same library error hierarchy. @@ -231,7 +234,7 @@ await router.set_operation(...) # local first, cloud on failure Enable `DEBUG` logging for `tesla_fleet_api` to see which backend served a routed call and why failover happened. -> **Warning:** Because a failed call is replayed on the next backend, a non-idempotent command (e.g. `honk_horn`, `actuate_trunk`, `door_unlock`, `charge_start`) that fails _mid-flight_ — after a backend may have already partially applied it — can be **double-executed** (or executed more than once across a longer chain) when it is retried on the next backend. This is a deliberate tradeoff of per-command failover. `BluetoothUnconfirmedCommand` is the exception: it propagates without failover because the BLE command may already have executed. When the primary is `VehicleBluetooth`, pass `verify_commands=True` to resolve supported mutating command timeouts by state before they reach the router; callers needing exactly-once semantics for other commands should gate dispatch with an explicit `health` check or call the underlying backends directly. +> **Warning:** Because a failed call is replayed on the next backend, a non-idempotent command (e.g. `honk_horn`, `actuate_trunk`, `door_unlock`, `charge_start`) that fails _mid-flight_ — after a backend may have already partially applied it — can be **double-executed** (or executed more than once across a longer chain) when it is retried on the next backend. This is a deliberate tradeoff of per-command failover. `BluetoothUnconfirmedCommand` is the exception: it propagates without failover because the BLE command may already have executed. When the primary is `VehicleBluetooth`, pass `verify_commands=True` to resolve supported mutating command timeouts by state before they reach the router, or `raise_unconfirmed=False` only when a best-effort success is acceptable; callers needing exactly-once semantics for other commands should gate dispatch with an explicit `health` check or call the underlying backends directly. > > Dispatch is implemented via `__getattr__`, which does not proxy dunder methods, so `async with Router(...)` does **not** manage a backend's BLE connection lifecycle (`__aenter__`/`__aexit__`). Commands still auto-connect on send; for explicit connect/disconnect reach through `router.primary` (or `router.backends`). diff --git a/docs/bluetooth_vehicles.md b/docs/bluetooth_vehicles.md index 343a5ad..1d53390 100644 --- a/docs/bluetooth_vehicles.md +++ b/docs/bluetooth_vehicles.md @@ -139,12 +139,14 @@ 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. -A response-wait timeout for a *mutating* command (RKE/closure actions, -HVAC/media/charging commands, `wake_up()`) raises `BluetoothUnconfirmedCommand` -instead of plain `BluetoothTimeout` - see "Mutating Command Timeouts" below. A -response-wait timeout for anything else (a state read) still raises plain -`BluetoothTimeout`. `BluetoothUnconfirmedCommand` subclasses `BluetoothTimeout`, -so existing `except BluetoothTimeout` handling keeps working; catch +By default, a response-wait timeout for a *mutating* command (RKE/closure +actions, HVAC/media/charging commands, `wake_up()`) raises +`BluetoothUnconfirmedCommand` instead of plain `BluetoothTimeout` - see +"Mutating Command Timeouts" below for the `verify_commands`, `optimistic`, and +`raise_unconfirmed` knobs that can change that outcome. A response-wait timeout +for anything else (a state read) still raises plain `BluetoothTimeout`. +`BluetoothUnconfirmedCommand` subclasses `BluetoothTimeout`, so existing +`except BluetoothTimeout` handling keeps working; catch `BluetoothUnconfirmedCommand` separately when you need to tell "the command may have executed" apart from "nothing happened." @@ -190,14 +192,16 @@ case. When a mutating command times out and its expected post-state can be derived from its own arguments, the same held connection reads the mapped prover state and either returns a normal success result (`{"response": {"result": True, -"reason": ""}}`) when the state matches or re-raises the -`BluetoothUnconfirmedCommand` when it does not. The read rides the existing -connection and never wakes the vehicle; if an infotainment prover cannot be -read because the car is asleep, the unconfirmed command timeout is re-raised. - -Commands whose outcome cannot be derived or read - true toggles, relative volume -steps, and ack-only actions such as `flash_lights()` or `trigger_homelink()` - -raise `BluetoothUnconfirmedCommand`, exactly as with verification off. Currently +"reason": ""}}`) when the state matches or raises +`BluetoothUnconfirmedCommand` when a completed read does not match. The read +rides the existing connection and never wakes the vehicle; if an infotainment +prover cannot be read because the car is asleep, the outcome stays unresolved +and falls through to the `raise_unconfirmed` setting. + +With the default `raise_unconfirmed=True`, commands whose outcome cannot be +derived or read - true toggles, relative volume steps, and ack-only actions such +as `flash_lights()` or `trigger_homelink()` - raise +`BluetoothUnconfirmedCommand`, exactly as with verification off. Currently verified commands and their provers: | Command | Prover | Confirmed when | @@ -213,6 +217,48 @@ The VCSEC lock prover is readable while the vehicle is asleep; the infotainment provers require the vehicle awake. This feature does not change behavior for any command not in the table, and it is inert unless `verify_commands=True`. +### Skipping the wait (`optimistic`) and defaulting to success (`raise_unconfirmed`) + +The full confirmation ladder for a mutating BLE command is: write the GATT +characteristic, wait for the ack, resolve via `verify_commands` on a timeout, +then decide the final outcome. Two more constructor/factory knobs (default off +and default on, respectively - both preserve today's behavior unless set) +shape that last step: + +- `optimistic=True` returns success as soon as the GATT write is confirmed, + skipping the ack wait and `verify_commands` entirely 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. +- `raise_unconfirmed=False` changes what happens only when the ladder is + exhausted without a definite answer - `verify_commands` is off, has no plan + for this command, or its prover read itself could not complete (e.g. the + car is asleep). Instead of raising `BluetoothUnconfirmedCommand`, that + ambiguous case resolves as a best-effort success + (`{"response": {"result": True, "reason": ""}}`). A car-side rejection + carried in an ack, a `verify_commands` state mismatch (the prover read + completed and does not show the requested value), and write failures are + unchanged and always raise - this flag converts only the "could not + determine what happened" outcome. + +Commands with no `verify_commands` plan - true toggles, relative volume steps, +and ack-only actions such as `flash_lights()` or `trigger_homelink()` (every +command not in the table above) - have nothing to mismatch against, so under +`raise_unconfirmed=False` a timeout on any of them always resolves best-effort. +Treat those commands as best-effort by design; rely on `verify_commands`'s +table for a reliable, state-backed confirmation on the commands it covers. + +```python +vehicle = tesla_bluetooth.vehicles.create( + "", optimistic=True +) +# or, to keep waiting for an ack but never raise on an inconclusive outcome: +vehicle = tesla_bluetooth.vehicles.create( + "", verify_commands=True, raise_unconfirmed=False +) +``` + ## Climate Commands Bluetooth vehicles support the same signed climate command methods as @@ -441,7 +487,9 @@ REST commands it is the endpoint's final path segment (e.g. `set_charge_limit`). For BLE commands run with `verify_commands=True`, a resolved timeout logs a second line with `verify_commands=resolved` and the confirmed result; an unresolved timeout logs `verify_commands=unresolved` before the -`BluetoothUnconfirmedCommand` propagates. `Router` additionally logs +`BluetoothUnconfirmedCommand` propagates. With `raise_unconfirmed=False`, an +exhausted ladder logs `raise_unconfirmed=False result=success (best-effort)` +instead of propagating that exception. `Router` additionally logs `command=... backend= result=...` for each backend it tries, or `result=unconfirmed` when it stops instead of failing over, so a BLE-primary/cloud-fallback setup shows exactly which backend served each call diff --git a/tesla_fleet_api.egg-info/PKG-INFO b/tesla_fleet_api.egg-info/PKG-INFO index a14300c..679b352 100644 --- a/tesla_fleet_api.egg-info/PKG-INFO +++ b/tesla_fleet_api.egg-info/PKG-INFO @@ -197,10 +197,13 @@ disable keepalive when vehicle sleep is preferred. BLE connect/notify and GATT write failures from `VehicleBluetooth` raise `BluetoothTransportError`, a `TeslaFleetError` subclass, with the original -transport exception chained as `__cause__`. A response-wait timeout from a -mutating BLE command raises `BluetoothUnconfirmedCommand`, a +transport exception chained as `__cause__`. By default, a response-wait timeout +from a mutating BLE command raises `BluetoothUnconfirmedCommand`, a `BluetoothTimeout` subclass, because the vehicle may have executed it despite a -lost acknowledgement. Catch `TeslaFleetError` to handle Bluetooth transport +lost acknowledgement. The BLE-only `optimistic` and `raise_unconfirmed` +factory options can instead resolve some ambiguous timeouts as best-effort +successes; see [Bluetooth for Vehicles](docs/bluetooth_vehicles.md) for the +full confirmation ladder. Catch `TeslaFleetError` to handle Bluetooth transport failures (including `bleak.exc.BleakError` and builtin `TimeoutError` from ESPHome proxies) and response-wait timeouts through the same library error hierarchy. @@ -254,7 +257,7 @@ await router.set_operation(...) # local first, cloud on failure Enable `DEBUG` logging for `tesla_fleet_api` to see which backend served a routed call and why failover happened. -> **Warning:** Because a failed call is replayed on the next backend, a non-idempotent command (e.g. `honk_horn`, `actuate_trunk`, `door_unlock`, `charge_start`) that fails _mid-flight_ — after a backend may have already partially applied it — can be **double-executed** (or executed more than once across a longer chain) when it is retried on the next backend. This is a deliberate tradeoff of per-command failover. `BluetoothUnconfirmedCommand` is the exception: it propagates without failover because the BLE command may already have executed. When the primary is `VehicleBluetooth`, pass `verify_commands=True` to resolve supported mutating command timeouts by state before they reach the router; callers needing exactly-once semantics for other commands should gate dispatch with an explicit `health` check or call the underlying backends directly. +> **Warning:** Because a failed call is replayed on the next backend, a non-idempotent command (e.g. `honk_horn`, `actuate_trunk`, `door_unlock`, `charge_start`) that fails _mid-flight_ — after a backend may have already partially applied it — can be **double-executed** (or executed more than once across a longer chain) when it is retried on the next backend. This is a deliberate tradeoff of per-command failover. `BluetoothUnconfirmedCommand` is the exception: it propagates without failover because the BLE command may already have executed. When the primary is `VehicleBluetooth`, pass `verify_commands=True` to resolve supported mutating command timeouts by state before they reach the router, or `raise_unconfirmed=False` only when a best-effort success is acceptable; callers needing exactly-once semantics for other commands should gate dispatch with an explicit `health` check or call the underlying backends directly. > > Dispatch is implemented via `__getattr__`, which does not proxy dunder methods, so `async with Router(...)` does **not** manage a backend's BLE connection lifecycle (`__aenter__`/`__aexit__`). Commands still auto-connect on send; for explicit connect/disconnect reach through `router.primary` (or `router.backends`). diff --git a/tesla_fleet_api/tesla/vehicle/bluetooth.py b/tesla_fleet_api/tesla/vehicle/bluetooth.py index dcd7a7b..bdeb986 100644 --- a/tesla_fleet_api/tesla/vehicle/bluetooth.py +++ b/tesla_fleet_api/tesla/vehicle/bluetooth.py @@ -309,6 +309,28 @@ class VehicleBluetooth(Commands[BluetoothParentT], Generic[BluetoothParentT]): steps, and ack-only actions - raise the unconfirmed timeout, exactly as with verification off (the default). + Two more knobs shape that same ladder (write -> ack wait -> ``verify_commands`` + -> unconfirmed outcome): + + - ``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 + (``BluetoothTransportError``) still raises. This is pure speed mode - the + caller owns any state verification it wants afterward - and it overrides + ``verify_commands`` and ``raise_unconfirmed`` for every mutating send. + - ``raise_unconfirmed`` (default ``True``, current behavior): when ``False``, + an exhausted ladder - the ack wait expired and ``verify_commands`` is off, + has no plan for this command, or could not complete its read - resolves as + a best-effort success instead of raising ``BluetoothUnconfirmedCommand``. + A car-side rejection carried in an ack, a ``verify_commands`` state + mismatch (the read completed and does not show the requested value), and + write failures are unaffected and always raise - this flag only converts + the "could not determine what happened" outcome. Commands with no + ``verify_commands`` plan (true toggles, relative steps, and ack-only + actions - see the table in ``docs/bluetooth_vehicles.md``) are always + resolved best-effort under this flag, since there is nothing to mismatch + against. + ``keepalive_interval`` (default ~20s, ``None``/``0`` disables) keeps an otherwise idle held connection from dropping: after that many seconds with no real GATT traffic, a minimal passive characteristic read is issued to @@ -323,6 +345,8 @@ class VehicleBluetooth(Commands[BluetoothParentT], Generic[BluetoothParentT]): ble_name: str verify_commands: bool + optimistic: bool + raise_unconfirmed: bool keepalive_interval: float | None device: BLEDevice | None = None client: BleakClient | None = None @@ -349,10 +373,14 @@ def __init__( device: BLEDevice | None = None, verify_commands: bool = False, keepalive_interval: float | None = DEFAULT_KEEPALIVE_INTERVAL, + optimistic: bool = False, + raise_unconfirmed: bool = True, ): super().__init__(parent, vin, key) self.verify_commands = verify_commands self.keepalive_interval = keepalive_interval + self.optimistic = optimistic + self.raise_unconfirmed = raise_unconfirmed self.ble_name = "S" + hashlib.sha1(vin.encode("utf-8")).hexdigest()[:16] + "C" self._queues = { Domain.DOMAIN_VEHICLE_SECURITY: asyncio.Queue(), @@ -536,12 +564,15 @@ async def _send( expects_data: bool = True, *, timeout: float | None = None, + optimistic: bool = False, ) -> RoutableMessage: """Serialize a message and send to the vehicle and wait for a response. When ``expects_data`` is False the reply is a single terminal ack (a VCSEC actuation, no data frame follows), so the ack is returned as soon - as it arrives and a shorter total timeout applies. + as it arrives and a shorter total timeout applies. ``optimistic`` skips + waiting for any reply at all - once the GATT write is confirmed, an + empty ``RoutableMessage`` is returned immediately. """ if timeout is None: @@ -569,6 +600,9 @@ async def _send( except (BleakError, TimeoutError) as e: raise BluetoothTransportError from e + if optimistic: + return RoutableMessage() + # Process the response try: async with asyncio.timeout(timeout): @@ -760,6 +794,45 @@ 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]: + """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. + """ + session = self._sessions[domain] + async with session.lock: + if self._auth_method == "hmac": + msg = await self._commandHmac(session, command) + else: + msg = await self._commandAes(session, command) + await self._send(msg, "protobuf_message_as_bytes", optimistic=True) + return {"response": {"result": True, "reason": ""}} + + def _unconfirmed_outcome( + self, + name: str, + unconfirmed: BluetoothUnconfirmedCommand, + *, + cause: BaseException, + ) -> dict[str, Any]: + """Resolve an exhausted confirmation ladder per ``raise_unconfirmed``. + + Reached only once nothing could determine what happened - verification + is off, has no plan for this command, or its read could not complete. + A verify mismatch (the read completed and does not show the requested + value) is unambiguous negative evidence and is raised by the caller + directly, never routed through here. + """ + if self.raise_unconfirmed: + raise unconfirmed from cause + LOGGER.debug( + "command=%s transport=%s raise_unconfirmed=False result=success (best-effort)", + name, + self._transport_name, + ) + return {"response": {"result": True, "reason": ""}} + async def _sendVehicleSecurity(self, command: UnsignedMessage) -> dict[str, Any]: """Send a VCSEC actuation. @@ -767,90 +840,105 @@ async def _sendVehicleSecurity(self, command: UnsignedMessage) -> dict[str, Any] failed, so a timed-out wait raises ``BluetoothUnconfirmedCommand`` rather than plain ``BluetoothTimeout`` - see that exception's docstring. With ``verify_commands`` on, that ambiguity is resolved by - reading back state before it can reach the caller. + reading back state before it can reach the caller. With + ``optimistic`` on, the write itself is the whole outcome and none of + this ladder runs. With ``raise_unconfirmed`` off, an exhausted ladder + resolves as a best-effort success instead of raising - see the class + docstring. """ await self._ensure_handshake(Domain.DOMAIN_VEHICLE_SECURITY) + if self.optimistic: + return await self._send_optimistic( + Domain.DOMAIN_VEHICLE_SECURITY, command.SerializeToString() + ) try: return await super()._sendVehicleSecurity(command) except BluetoothTimeout as timeout: unconfirmed = BluetoothUnconfirmedCommand(timeout.data, timeout.status) - if not self.verify_commands: - raise unconfirmed from timeout name = vcsec_command_name(command) - try: + if self.verify_commands: result = await self._resolve_timeout( _vcsec_verify_plan(command), unconfirmed ) - except BluetoothTimeout: + if result is not None: + LOGGER.debug( + "command=%s transport=%s verify_commands=resolved result=%s", + name, + self._transport_name, + result.get("response", {}).get("result"), + ) + return result LOGGER.debug( "command=%s transport=%s verify_commands=unresolved", name, self._transport_name, ) - raise - LOGGER.debug( - "command=%s transport=%s verify_commands=resolved result=%s", - name, - self._transport_name, - result.get("response", {}).get("result"), - ) - return result + return self._unconfirmed_outcome(name, unconfirmed, cause=timeout) async def _sendInfotainment( self, command: Action, *, mutating: bool = True ) -> dict[str, Any]: """Send an infotainment command. - Same unconfirmed-ack semantics as ``_sendVehicleSecurity`` - see there. + Same unconfirmed-ack, ``optimistic``, and ``raise_unconfirmed`` + semantics as ``_sendVehicleSecurity`` - see there. ``mutating=False`` + (``ping()`` only) is exempt from all three: it always waits for its + real reply. """ await self._ensure_handshake(Domain.DOMAIN_INFOTAINMENT) + if self.optimistic and mutating: + return await self._send_optimistic( + Domain.DOMAIN_INFOTAINMENT, command.SerializeToString() + ) try: return await super()._sendInfotainment(command) except BluetoothTimeout as timeout: if not mutating: raise unconfirmed = BluetoothUnconfirmedCommand(timeout.data, timeout.status) - if not self.verify_commands: - raise unconfirmed from timeout name = infotainment_command_name(command) - resolver = _INFOTAINMENT_VERIFY_PLANS.get( - command.vehicleAction.WhichOneof("vehicle_action_msg") - ) - plan = resolver(command.vehicleAction) if resolver else None - try: + if self.verify_commands: + resolver = _INFOTAINMENT_VERIFY_PLANS.get( + command.vehicleAction.WhichOneof("vehicle_action_msg") + ) + plan = resolver(command.vehicleAction) if resolver else None result = await self._resolve_timeout(plan, unconfirmed) - except BluetoothTimeout: + if result is not None: + LOGGER.debug( + "command=%s transport=%s verify_commands=resolved result=%s", + name, + self._transport_name, + result.get("response", {}).get("result"), + ) + return result LOGGER.debug( "command=%s transport=%s verify_commands=unresolved", name, self._transport_name, ) - raise - LOGGER.debug( - "command=%s transport=%s verify_commands=resolved result=%s", - name, - self._transport_name, - result.get("response", {}).get("result"), - ) - return result + return self._unconfirmed_outcome(name, unconfirmed, cause=timeout) async def _resolve_timeout( self, plan: VerifyPlan | None, timeout: BluetoothTimeout - ) -> dict[str, Any]: - """Confirm a timed-out mutation by state, or re-raise if unverifiable. - - The prover read rides the same held connection. An INFO-domain prover - needs the vehicle awake; if the read cannot complete (e.g. the car is - asleep) it surfaces a ``TeslaFleetError`` and the unconfirmed timeout is - re-raised rather than waking the car just to verify. + ) -> dict[str, Any] | None: + """Confirm a timed-out mutation by state. + + Returns the confirmed success result, or ``None`` if verification + could not even be attempted - no plan for this command, or the prover + read itself failed (an INFO-domain prover needs the vehicle awake; a + ``TeslaFleetError`` from a sleeping car is not woken just to verify). + ``None`` leaves the caller to resolve the outcome via + ``raise_unconfirmed``. A verify mismatch - the read completed and does + not show the requested value - is unambiguous negative evidence and + raises ``timeout`` directly instead, regardless of that setting. """ if plan is None: - raise timeout + return None reader_name, executed = plan try: state = await getattr(self, reader_name)() except TeslaFleetError: - raise timeout + return None if executed(state): return {"response": {"result": True, "reason": ""}} raise timeout diff --git a/tesla_fleet_api/tesla/vehicle/vehicles.py b/tesla_fleet_api/tesla/vehicle/vehicles.py index 16f0746..71e0d89 100644 --- a/tesla_fleet_api/tesla/vehicle/vehicles.py +++ b/tesla_fleet_api/tesla/vehicle/vehicles.py @@ -48,6 +48,8 @@ def createBluetooth( vin: str, verify_commands: bool = False, keepalive_interval: float | None = DEFAULT_KEEPALIVE_INTERVAL, + optimistic: bool = False, + raise_unconfirmed: bool = True, ) -> VehicleBluetooth[FleetParentT]: """Creates a bluetooth vehicle that uses command protocol. @@ -56,12 +58,20 @@ def createBluetooth( command timeout. ``keepalive_interval`` seconds of GATT idleness triggers a passive read to hold the link open (``None``/``0`` disables). + ``optimistic`` returns success as soon as a mutating command's GATT + write is confirmed, skipping the ack wait and ``verify_commands``. + ``raise_unconfirmed=False`` resolves an exhausted confirmation ladder + as a best-effort success instead of raising + ``BluetoothUnconfirmedCommand``. See ``VehicleBluetooth``'s docstring + for the full ladder and what each flag does and does not affect. """ vehicle = self.Bluetooth( self._parent, vin, verify_commands=verify_commands, keepalive_interval=keepalive_interval, + optimistic=optimistic, + raise_unconfirmed=raise_unconfirmed, ) self[vin] = vehicle return vehicle @@ -91,6 +101,8 @@ def create( device: BLEDevice | None = None, verify_commands: bool = False, keepalive_interval: float | None = DEFAULT_KEEPALIVE_INTERVAL, + optimistic: bool = False, + raise_unconfirmed: bool = True, ) -> VehicleBluetooth[BluetoothClientT]: """Creates a bluetooth vehicle that uses command protocol. @@ -99,9 +111,21 @@ def create( command timeout. ``keepalive_interval`` seconds of GATT idleness triggers a passive read to hold the link open (``None``/``0`` disables). + ``optimistic`` returns success as soon as a mutating command's GATT + write is confirmed, skipping the ack wait and ``verify_commands``. + ``raise_unconfirmed=False`` resolves an exhausted confirmation ladder + as a best-effort success instead of raising + ``BluetoothUnconfirmedCommand``. See ``VehicleBluetooth``'s docstring + for the full ladder and what each flag does and does not affect. """ return self.createBluetooth( - vin, key, device, verify_commands, keepalive_interval + vin, + key, + device, + verify_commands, + keepalive_interval, + optimistic, + raise_unconfirmed, ) def createBluetooth( @@ -111,6 +135,8 @@ def createBluetooth( device: BLEDevice | None = None, verify_commands: bool = False, keepalive_interval: float | None = DEFAULT_KEEPALIVE_INTERVAL, + optimistic: bool = False, + raise_unconfirmed: bool = True, ) -> VehicleBluetooth[BluetoothClientT]: """Creates a bluetooth vehicle that uses command protocol. @@ -119,6 +145,12 @@ def createBluetooth( command timeout. ``keepalive_interval`` seconds of GATT idleness triggers a passive read to hold the link open (``None``/``0`` disables). + ``optimistic`` returns success as soon as a mutating command's GATT + write is confirmed, skipping the ack wait and ``verify_commands``. + ``raise_unconfirmed=False`` resolves an exhausted confirmation ladder + as a best-effort success instead of raising + ``BluetoothUnconfirmedCommand``. See ``VehicleBluetooth``'s docstring + for the full ladder and what each flag does and does not affect. """ vehicle = self.Bluetooth( self._parent, @@ -127,6 +159,8 @@ def createBluetooth( device, verify_commands=verify_commands, keepalive_interval=keepalive_interval, + optimistic=optimistic, + raise_unconfirmed=raise_unconfirmed, ) self[vin] = vehicle return vehicle diff --git a/tesla_fleet_api/teslemetry/vehicle.py b/tesla_fleet_api/teslemetry/vehicle.py index 40b7a96..d10619c 100644 --- a/tesla_fleet_api/teslemetry/vehicle.py +++ b/tesla_fleet_api/teslemetry/vehicle.py @@ -308,6 +308,8 @@ def createBluetooth( vin: str, verify_commands: bool = False, keepalive_interval: float | None = None, + optimistic: bool = False, + raise_unconfirmed: bool = True, ) -> Any: """Not supported; parameters match the Fleet API Bluetooth factory.""" raise NotImplementedError("Teslemetry cannot use local Bluetooth") diff --git a/tesla_fleet_api/tessie/vehicle.py b/tesla_fleet_api/tessie/vehicle.py index 675b064..e7e638a 100644 --- a/tesla_fleet_api/tessie/vehicle.py +++ b/tesla_fleet_api/tessie/vehicle.py @@ -1204,6 +1204,8 @@ def createBluetooth( vin: str, verify_commands: bool = False, keepalive_interval: float | None = None, + optimistic: bool = False, + raise_unconfirmed: bool = True, ) -> Any: """Not supported; parameters match the Fleet API Bluetooth factory.""" raise NotImplementedError("Tessie cannot use local Bluetooth") diff --git a/tests/ble_mocked_transport.py b/tests/ble_mocked_transport.py index 6ac1b66..139a908 100644 --- a/tests/ble_mocked_transport.py +++ b/tests/ble_mocked_transport.py @@ -95,18 +95,29 @@ class MockedBleTransportTestCase(IsolatedAsyncioTestCase): VIN = VIN def make_vehicle( - self, verify_commands: bool = False + self, + verify_commands: bool = False, + optimistic: bool = False, + raise_unconfirmed: bool = True, ) -> tuple[VehicleBluetooth[Any], AsyncMock]: """Build a VehicleBluetooth whose ``_send`` and connection are fully mocked. Returns the vehicle plus the ``AsyncMock`` standing in for ``_send`` - set ``send.return_value``/``side_effect`` to script replies. Pass ``verify_commands=True`` to exercise the opt-in post-timeout state - verification. + verification, ``optimistic=True`` to skip the ack wait entirely, or + ``raise_unconfirmed=False`` to resolve an exhausted ladder as a + best-effort success. """ parent = MagicMock() parent.private_key = ec.generate_private_key(ec.SECP256R1()) - vehicle = VehicleBluetooth(parent, self.VIN, verify_commands=verify_commands) + vehicle = VehicleBluetooth( + parent, + self.VIN, + verify_commands=verify_commands, + optimistic=optimistic, + raise_unconfirmed=raise_unconfirmed, + ) # Mark both signed-command sessions ready so _command skips the # handshake round-trip (which would otherwise also go through _send). diff --git a/tests/test_ble_optimistic_and_best_effort.py b/tests/test_ble_optimistic_and_best_effort.py new file mode 100644 index 0000000..e09fe23 --- /dev/null +++ b/tests/test_ble_optimistic_and_best_effort.py @@ -0,0 +1,188 @@ +"""Tests for the ``optimistic`` and ``raise_unconfirmed`` consumer knobs. + +Both form one outcome contract on top of the existing confirmation ladder +(write -> ack wait -> ``verify_commands`` -> unconfirmed outcome): + +- ``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. +- ``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. +""" + +from __future__ import annotations + +from tesla_fleet_api.exceptions import ( + BluetoothTimeout, + BluetoothTransportError, + BluetoothUnconfirmedCommand, +) +from tesla_fleet_api.tesla.vehicle.proto.vcsec_pb2 import ( + VehicleLockState_E, + VehicleStatus, +) + +from ble_mocked_transport import ( + MockedBleTransportTestCase, + infotainment_action_ok_reply, + vcsec_vehicle_status_reply, +) + + +class OptimisticModeTests(MockedBleTransportTestCase): + """``optimistic=True`` returns on a confirmed write with no ack wait.""" + + async def test_vcsec_returns_on_write_without_waiting(self) -> None: + vehicle, send = self.make_vehicle(optimistic=True) + # No return_value/side_effect scripted: if the code waited on the + # reply it would receive a bare MagicMock and fail decoding it. + + result = await vehicle.door_lock() + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + send.assert_awaited_once() + + async def test_infotainment_returns_on_write_without_waiting(self) -> None: + vehicle, send = self.make_vehicle(optimistic=True) + + result = await vehicle.honk_horn() + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + send.assert_awaited_once() + + async def test_write_failure_still_raises(self) -> None: + vehicle, send = self.make_vehicle(optimistic=True) + send.side_effect = BluetoothTransportError() + + with self.assertRaises(BluetoothTransportError): + await vehicle.door_lock() + + async def test_non_mutating_infotainment_is_unaffected(self) -> None: + # ping() always waits for its real reply; optimistic mode is only for + # mutating commands. + vehicle, send = self.make_vehicle(optimistic=True) + send.return_value = infotainment_action_ok_reply() + + result = await vehicle.ping() + + self.assertEqual(result["response"]["result"], True) + send.assert_awaited_once() + + async def test_verify_commands_is_never_consulted(self) -> None: + # Even with verify_commands=True, optimistic mode must never issue a + # second (prover) send - it never waits long enough to time out. + vehicle, send = self.make_vehicle(optimistic=True) + vehicle.verify_commands = True + + await vehicle.door_lock() + + send.assert_awaited_once() + + +class RaiseUnconfirmedTests(MockedBleTransportTestCase): + """``raise_unconfirmed=False`` converts only the truly-unconfirmed case.""" + + async def test_default_still_raises(self) -> None: + vehicle, send = self.make_vehicle() + send.side_effect = BluetoothTimeout() + + with self.assertRaises(BluetoothUnconfirmedCommand): + await vehicle.door_lock() + + async def test_no_verify_commands_resolves_best_effort(self) -> None: + vehicle, send = self.make_vehicle(raise_unconfirmed=False) + send.side_effect = BluetoothTimeout() + + result = await vehicle.door_lock() + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + + async def test_infotainment_resolves_best_effort(self) -> None: + vehicle, send = self.make_vehicle(raise_unconfirmed=False) + send.side_effect = BluetoothTimeout() + + result = await vehicle.honk_horn() + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + + async def test_verify_commands_unresolvable_resolves_best_effort(self) -> None: + # verify_commands on, but the command has no verify plan at all + # (honk_horn is ack-only) - still exhausted, still convertible. + vehicle, send = self.make_vehicle(verify_commands=True, raise_unconfirmed=False) + send.side_effect = BluetoothTimeout() + + result = await vehicle.honk_horn() + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + + async def test_verify_commands_read_failure_resolves_best_effort(self) -> None: + # verify_commands on, has a plan, but the prover read itself fails + # (e.g. asleep car) - still an ambiguous, convertible outcome. + vehicle, send = self.make_vehicle(verify_commands=True, raise_unconfirmed=False) + send.side_effect = [BluetoothTimeout(), BluetoothTimeout()] + + result = await vehicle.door_lock() + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + + async def test_verify_commands_confirmed_success_is_unaffected(self) -> None: + vehicle, send = self.make_vehicle(verify_commands=True, raise_unconfirmed=False) + send.side_effect = [ + BluetoothTimeout(), + vcsec_vehicle_status_reply( + VehicleStatus( + vehicleLockState=VehicleLockState_E.VEHICLELOCKSTATE_LOCKED + ) + ), + ] + + result = await vehicle.door_lock() + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + + async def test_verify_commands_mismatch_still_raises(self) -> None: + # A prover read that disagrees is affirmative evidence, not mere + # absence of confirmation - it must raise regardless of + # raise_unconfirmed. + vehicle, send = self.make_vehicle(verify_commands=True, raise_unconfirmed=False) + send.side_effect = [ + BluetoothTimeout(), + vcsec_vehicle_status_reply( + VehicleStatus( + vehicleLockState=VehicleLockState_E.VEHICLELOCKSTATE_UNLOCKED + ) + ), + ] + + with self.assertRaises(BluetoothUnconfirmedCommand): + await vehicle.door_lock() + + async def test_write_failure_still_raises(self) -> None: + vehicle, send = self.make_vehicle(raise_unconfirmed=False) + send.side_effect = BluetoothTransportError() + + with self.assertRaises(BluetoothTransportError): + await vehicle.door_lock() + + async def test_non_mutating_timeout_still_raises_plain_timeout(self) -> None: + vehicle, send = self.make_vehicle(raise_unconfirmed=False) + send.side_effect = BluetoothTimeout() + + with self.assertRaises(BluetoothTimeout) as ctx: + await vehicle.ping() + + self.assertNotIsInstance(ctx.exception, BluetoothUnconfirmedCommand) + + async def test_read_timeout_still_raises_plain_timeout(self) -> None: + # A read (no mutation) is unaffected by raise_unconfirmed. + vehicle, send = self.make_vehicle(raise_unconfirmed=False) + send.side_effect = BluetoothTimeout() + + with self.assertRaises(BluetoothTimeout) as ctx: + await vehicle.charge_state() + + self.assertNotIsInstance(ctx.exception, BluetoothUnconfirmedCommand)