diff --git a/AGENTS.md b/AGENTS.md index 24faf39..8c904ef 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)` → `VehicleBluetooth` +- `createBluetooth(vin, verify_commands=False)` → `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,6 +127,7 @@ 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 `BluetoothTimeout` 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 a `BluetoothTimeout` 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 `BluetoothTimeout` 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 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, `VehicleRouter`'s blind BLE->cloud failover no longer double-executes non-idempotent commands (a verified-executed command returns success so no failover fires; a verified-not-executed one raises so failover is genuinely safe - Router needs no changes). 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 re-raise unchanged. 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 original timeout. See `docs/bluetooth_vehicles.md` for the user-facing table. - **`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 a `BluetoothTimeout` 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. - **`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 it raises `BluetoothTimeout` 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 c283e46..b35f052 100644 --- a/README.md +++ b/README.md @@ -189,7 +189,7 @@ async def main(): # Primary: local Bluetooth tesla_bluetooth = TeslaBluetooth() await tesla_bluetooth.get_private_key("path/to/private_key.pem") - primary = tesla_bluetooth.vehicles.create("") + primary = tesla_bluetooth.vehicles.create("", verify_commands=True) # Secondary (fallback): Teslemetry cloud teslemetry = Teslemetry(access_token="", session=session) @@ -220,7 +220,7 @@ await router.set_operation(...) # local first, cloud on failure `Router`, `VehicleRouter`, and `EnergySiteRouter` are all importable from `tesla_fleet_api.router` (and, for backward compatibility, from `tesla_fleet_api.tesla`). -> **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. Callers needing exactly-once semantics for such 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. When the primary is `VehicleBluetooth`, pass `verify_commands=True` to resolve supported mutating command timeouts by state before failover; 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 0025a2a..819547c 100644 --- a/docs/bluetooth_vehicles.md +++ b/docs/bluetooth_vehicles.md @@ -127,6 +127,41 @@ inside the library can also re-send an identical command after a WAIT status or epoch/token fault, so verify by absolute state rather than by counting command attempts. +### Opt-in state verification (`verify_commands`) + +Construct a `VehicleBluetooth` with `verify_commands=True` (also accepted by +`vehicles.create(...)` / `vehicles.createBluetooth(...)`, default off) to have +the class resolve that ambiguity for you. Verification runs only on a timeout, +so the normal path - which returns as soon as the terminal ack arrives - keeps +its speed and pays no extra read; the prover read happens only in the ambiguous +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 `BluetoothTimeout` 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 original +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()` - +re-raise the timeout unchanged, exactly as with verification off. Currently +verified commands and their provers: + +| Command | Prover | Confirmed when | +| --- | --- | --- | +| `door_lock()` / `door_unlock()` | `vehicle_state()` | lock state matches | +| `set_charge_limit(percent)` | `charge_state()` | `charge_limit_soc` matches | +| `set_charging_amps(amps)` | `charge_state()` | `charging_amps` matches | +| `adjust_volume(volume)` | `media_state()` | `audio_volume` matches | +| `set_temps(driver, passenger)` | `climate_state()` | both temp settings match | +| `auto_conditioning_start()` / `auto_conditioning_stop()` | `climate_state()` | `is_climate_on` matches | + +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`. + ## Climate Commands Bluetooth vehicles support the same signed climate command methods as diff --git a/tesla_fleet_api/tesla/vehicle/bluetooth.py b/tesla_fleet_api/tesla/vehicle/bluetooth.py index 9c441be..9095841 100644 --- a/tesla_fleet_api/tesla/vehicle/bluetooth.py +++ b/tesla_fleet_api/tesla/vehicle/bluetooth.py @@ -20,6 +20,7 @@ WHITELIST_OPERATION_STATUS, BluetoothTimeout, BluetoothTransportError, + TeslaFleetError, WhitelistOperationStatus, ) from tesla_fleet_api.tesla.vehicle.commands import Commands @@ -60,6 +61,7 @@ PublicKey, RKEAction_E, UnsignedMessage, + VehicleLockState_E, VehicleStatus, WhitelistOperation, ) @@ -194,6 +196,78 @@ def discard_packet(self): self.expected_length = None +# Optional post-timeout command verification. +# +# A lost ack from a mutating BLE command is inconclusive: the vehicle may have +# executed it anyway. When ``verify_commands`` is on, a timed-out mutation whose +# outcome can be derived from its own arguments is confirmed by reading the +# mapped state instead of surfacing the ambiguous timeout. A verify "plan" pairs +# the state reader to call with a predicate that checks the observed state +# against the requested value. Commands absent from these tables - true toggles, +# relative steps, and ack-only actions whose outcome cannot be derived from the +# request - have no plan and re-raise the timeout unchanged. +VerifyPlan = tuple[str, Callable[[Any], bool]] + + +def _vcsec_verify_plan(command: UnsignedMessage) -> VerifyPlan | None: + """Derive a VCSEC verify plan (read ``vehicle_state``) from a lost actuation.""" + if command.WhichOneof("sub_message") != "RKEAction": + return None + if command.RKEAction == RKEAction_E.RKE_ACTION_LOCK: + return "vehicle_state", ( + lambda s: s.vehicleLockState == VehicleLockState_E.VEHICLELOCKSTATE_LOCKED + ) + if command.RKEAction == RKEAction_E.RKE_ACTION_UNLOCK: + return "vehicle_state", ( + lambda s: s.vehicleLockState == VehicleLockState_E.VEHICLELOCKSTATE_UNLOCKED + ) + return None + + +def _plan_charge_limit(action: VehicleAction) -> VerifyPlan | None: + percent = action.chargingSetLimitAction.percent + return "charge_state", lambda s: s.charge_limit_soc == percent + + +def _plan_charging_amps(action: VehicleAction) -> VerifyPlan | None: + amps = action.setChargingAmpsAction.charging_amps + return "charge_state", lambda s: s.charging_amps == amps + + +def _plan_volume(action: VehicleAction) -> VerifyPlan | None: + # A relative volume step is not derivable without a pre-read, so only the + # absolute set is verifiable. + if action.mediaUpdateVolume.WhichOneof("media_volume") != "volume_absolute_float": + return None + target = action.mediaUpdateVolume.volume_absolute_float + return "media_state", lambda s: s.audio_volume == target + + +def _plan_temps(action: VehicleAction) -> VerifyPlan | None: + driver = action.hvacTemperatureAdjustmentAction.driver_temp_celsius + passenger = action.hvacTemperatureAdjustmentAction.passenger_temp_celsius + return "climate_state", ( + lambda s: ( + s.driver_temp_setting == driver and s.passenger_temp_setting == passenger + ) + ) + + +def _plan_auto_conditioning(action: VehicleAction) -> VerifyPlan | None: + power_on = action.hvacAutoAction.power_on + return "climate_state", lambda s: s.is_climate_on == power_on + + +# Keyed by the ``VehicleAction`` oneof field an infotainment mutation sets. +_INFOTAINMENT_VERIFY_PLANS: dict[str, Callable[[VehicleAction], VerifyPlan | None]] = { + "chargingSetLimitAction": _plan_charge_limit, + "setChargingAmpsAction": _plan_charging_amps, + "mediaUpdateVolume": _plan_volume, + "hvacTemperatureAdjustmentAction": _plan_temps, + "hvacAutoAction": _plan_auto_conditioning, +} + + class VehicleBluetooth(Commands[BluetoothParentT], Generic[BluetoothParentT]): """Class describing the Tesla Fleet API vehicle endpoints and commands for a specific vehicle with command signing. @@ -211,9 +285,19 @@ class VehicleBluetooth(Commands[BluetoothParentT], Generic[BluetoothParentT]): blind-retry a non-idempotent command (toggles, volume steps, schedule add/remove) on a timeout alone. The inherited WAIT/fault retry (``Commands._command``) can also re-send an already-executed command. + + Pass ``verify_commands=True`` to resolve that ambiguity inside this class: + on a timeout from a mutating command whose expected post-state is derivable + from its arguments, the same held connection reads the mapped prover state + and either returns a normal success result (the command executed) or + re-raises the ``BluetoothTimeout`` (it did not). Commands whose outcome + cannot be derived or read - true toggles, relative steps, and ack-only + actions - re-raise the timeout unchanged, exactly as with verification off + (the default). """ ble_name: str + verify_commands: bool device: BLEDevice | None = None client: BleakClient | None = None _queues: dict[Domain, asyncio.Queue[RoutableMessage]] @@ -232,8 +316,10 @@ def __init__( vin: str, key: ec.EllipticCurvePrivateKey | None = None, device: BLEDevice | None = None, + verify_commands: bool = False, ): super().__init__(parent, vin, key) + self.verify_commands = verify_commands self.ble_name = "S" + hashlib.sha1(vin.encode("utf-8")).hexdigest()[:16] + "C" self._queues = { Domain.DOMAIN_VEHICLE_SECURITY: asyncio.Queue(), @@ -582,6 +668,49 @@ async def _command( domain, command, attempt, expects_data=expects_data ) + async def _sendVehicleSecurity(self, command: UnsignedMessage) -> dict[str, Any]: + """Send a VCSEC actuation, optionally resolving a lost-ack timeout by state.""" + if not self.verify_commands: + return await super()._sendVehicleSecurity(command) + try: + return await super()._sendVehicleSecurity(command) + except BluetoothTimeout as timeout: + return await self._resolve_timeout(_vcsec_verify_plan(command), timeout) + + async def _sendInfotainment(self, command: Action) -> dict[str, Any]: + """Send an infotainment command, optionally resolving a lost-ack timeout by state.""" + if not self.verify_commands: + return await super()._sendInfotainment(command) + try: + return await super()._sendInfotainment(command) + except BluetoothTimeout as timeout: + resolver = _INFOTAINMENT_VERIFY_PLANS.get( + command.vehicleAction.WhichOneof("vehicle_action_msg") + ) + plan = resolver(command.vehicleAction) if resolver else None + return await self._resolve_timeout(plan, 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 original timeout is + re-raised rather than waking the car just to verify. + """ + if plan is None: + raise timeout + reader_name, executed = plan + try: + state = await getattr(self, reader_name)() + except TeslaFleetError: + raise timeout + if executed(state): + return {"response": {"result": True, "reason": ""}} + raise timeout + async def pair( self, role: Role = Role.ROLE_OWNER, diff --git a/tesla_fleet_api/tesla/vehicle/vehicles.py b/tesla_fleet_api/tesla/vehicle/vehicles.py index d715155..d7160fa 100644 --- a/tesla_fleet_api/tesla/vehicle/vehicles.py +++ b/tesla_fleet_api/tesla/vehicle/vehicles.py @@ -40,9 +40,15 @@ def createSigned(self, vin: str) -> VehicleSigned[FleetParentT]: self[vin] = vehicle return vehicle - def createBluetooth(self, vin: str) -> VehicleBluetooth[FleetParentT]: - """Creates a bluetooth vehicle that uses command protocol.""" - vehicle = self.Bluetooth(self._parent, vin) + def createBluetooth( + self, vin: str, verify_commands: bool = False + ) -> VehicleBluetooth[FleetParentT]: + """Creates a bluetooth vehicle that uses command protocol. + + Set ``verify_commands`` to confirm supported mutating BLE command + timeouts by reading the resulting state before surfacing the timeout. + """ + vehicle = self.Bluetooth(self._parent, vin, verify_commands=verify_commands) self[vin] = vehicle return vehicle @@ -69,17 +75,29 @@ def create( vin: str, key: ec.EllipticCurvePrivateKey | None = None, device: BLEDevice | None = None, + verify_commands: bool = False, ) -> VehicleBluetooth[BluetoothClientT]: - """Creates a bluetooth vehicle that uses command protocol.""" - return self.createBluetooth(vin, key, device) + """Creates a bluetooth vehicle that uses command protocol. + + Set ``verify_commands`` to confirm supported mutating BLE command + timeouts by reading the resulting state before surfacing the timeout. + """ + return self.createBluetooth(vin, key, device, verify_commands) def createBluetooth( self, vin: str, key: ec.EllipticCurvePrivateKey | None = None, device: BLEDevice | None = None, + verify_commands: bool = False, ) -> VehicleBluetooth[BluetoothClientT]: - """Creates a bluetooth vehicle that uses command protocol.""" - vehicle = self.Bluetooth(self._parent, vin, key, device) + """Creates a bluetooth vehicle that uses command protocol. + + Set ``verify_commands`` to confirm supported mutating BLE command + timeouts by reading the resulting state before surfacing the timeout. + """ + vehicle = self.Bluetooth( + self._parent, vin, key, device, verify_commands=verify_commands + ) self[vin] = vehicle return vehicle diff --git a/tesla_fleet_api/teslemetry/vehicle.py b/tesla_fleet_api/teslemetry/vehicle.py index 68b3826..ae88174 100644 --- a/tesla_fleet_api/teslemetry/vehicle.py +++ b/tesla_fleet_api/teslemetry/vehicle.py @@ -16,11 +16,11 @@ class TeslemetryVehicle(VehicleFleet["Teslemetry"]): parent: Teslemetry - async def server_side_polling( - self, value: bool | None = None - ) -> bool | None: + async def server_side_polling(self, value: bool | None = None) -> bool | None: """Get or set Auto mode.""" - LOGGER.warning("This method is deprecated and will be removed in a future release.") + LOGGER.warning( + "This method is deprecated and will be removed in a future release." + ) if value is True: return ( await self._request( @@ -303,6 +303,6 @@ def createSigned(self, vin: str) -> Any: """Creates a specific vehicle.""" raise NotImplementedError("Teslemetry cannot use Fleet API directly") - def createBluetooth(self, vin: str) -> Any: + def createBluetooth(self, vin: str, verify_commands: bool = False) -> Any: """Creates a specific vehicle.""" raise NotImplementedError("Teslemetry cannot use local Bluetooth") diff --git a/tesla_fleet_api/tessie/vehicle.py b/tesla_fleet_api/tessie/vehicle.py index 39537cb..639f75c 100644 --- a/tesla_fleet_api/tessie/vehicle.py +++ b/tesla_fleet_api/tessie/vehicle.py @@ -662,9 +662,7 @@ async def remote_boombox( return await self._request( Method.POST, f"{self.vin}/command/remote_boombox", - params=self._command_params( - wait_for_completion, max_attempts, sound=sound - ), + params=self._command_params(wait_for_completion, max_attempts, sound=sound), ) async def set_speed_limit( @@ -1180,6 +1178,7 @@ async def update_plate( payload["state"] = state return await self._request(Method.POST, f"{self.vin}/plate", json=payload) + class TessieVehicles(Vehicles["Tessie"]): """Class containing and creating vehicles.""" @@ -1200,6 +1199,6 @@ def createSigned(self, vin: str) -> Any: """Creates a specific vehicle.""" raise NotImplementedError("Tessie cannot use Fleet API directly") - def createBluetooth(self, vin: str) -> Any: + def createBluetooth(self, vin: str, verify_commands: bool = False) -> Any: """Creates a specific vehicle.""" raise NotImplementedError("Tessie cannot use local Bluetooth") diff --git a/tests/ble_mocked_transport.py b/tests/ble_mocked_transport.py index c3e0646..6ac1b66 100644 --- a/tests/ble_mocked_transport.py +++ b/tests/ble_mocked_transport.py @@ -94,15 +94,19 @@ class MockedBleTransportTestCase(IsolatedAsyncioTestCase): VIN = VIN - def make_vehicle(self) -> tuple[VehicleBluetooth[Any], AsyncMock]: + def make_vehicle( + self, verify_commands: bool = False + ) -> 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. + set ``send.return_value``/``side_effect`` to script replies. Pass + ``verify_commands=True`` to exercise the opt-in post-timeout state + verification. """ parent = MagicMock() parent.private_key = ec.generate_private_key(ec.SECP256R1()) - vehicle = VehicleBluetooth(parent, self.VIN) + vehicle = VehicleBluetooth(parent, self.VIN, verify_commands=verify_commands) # 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_command_verification.py b/tests/test_ble_command_verification.py new file mode 100644 index 0000000..b8700f6 --- /dev/null +++ b/tests/test_ble_command_verification.py @@ -0,0 +1,257 @@ +"""Tests for opt-in post-timeout command verification on ``VehicleBluetooth``. + +A ``BluetoothTimeout`` from a mutating BLE command is inconclusive - the vehicle +can execute the command without its ack reaching the client. With +``verify_commands=True``, a timed-out mutation whose expected post-state is +derivable from its arguments is confirmed by reading the mapped prover state: +verified-executed returns a normal success result, verified-not-executed +re-raises the timeout, and an unverifiable command re-raises unchanged. With the +default (``verify_commands=False``) every path is byte-identical to today, and no +verification read is ever issued. + +The single mocked ``_send`` is scripted with a list ``side_effect``: the first +entry answers the command send (raise ``BluetoothTimeout`` to force the +ambiguous case), and the second, when present, answers the prover read. +""" + +from __future__ import annotations + +from tesla_fleet_api.exceptions import BluetoothTimeout +from tesla_fleet_api.tesla.vehicle.proto.vehicle_pb2 import ( + ChargeState, + ClimateState, + MediaState, + VehicleData, +) +from tesla_fleet_api.tesla.vehicle.proto.vcsec_pb2 import ( + VehicleLockState_E, + VehicleStatus, +) + +from ble_mocked_transport import ( + MockedBleTransportTestCase, + infotainment_action_ok_reply, + infotainment_vehicle_data_reply, + vcsec_ok_reply, + vcsec_vehicle_status_reply, +) + + +class VcsecVerificationTests(MockedBleTransportTestCase): + """Lock/unlock verify against the VCSEC ``vehicle_state`` lock field.""" + + async def test_verified_executed_returns_success(self) -> None: + vehicle, send = self.make_vehicle(verify_commands=True) + 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": ""}}) + # Command send + one prover read. + self.assertEqual(send.await_count, 2) + + async def test_verified_not_executed_reraises(self) -> None: + vehicle, send = self.make_vehicle(verify_commands=True) + send.side_effect = [ + BluetoothTimeout(), + vcsec_vehicle_status_reply( + VehicleStatus( + vehicleLockState=VehicleLockState_E.VEHICLELOCKSTATE_UNLOCKED + ) + ), + ] + + with self.assertRaises(BluetoothTimeout): + await vehicle.door_lock() + self.assertEqual(send.await_count, 2) + + async def test_unlock_verified_executed(self) -> None: + vehicle, send = self.make_vehicle(verify_commands=True) + send.side_effect = [ + BluetoothTimeout(), + vcsec_vehicle_status_reply( + VehicleStatus( + vehicleLockState=VehicleLockState_E.VEHICLELOCKSTATE_UNLOCKED + ) + ), + ] + + result = await vehicle.door_unlock() + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + + async def test_unverifiable_vcsec_command_reraises_without_read(self) -> None: + # A closure move has no derivable lock prover. + vehicle, send = self.make_vehicle(verify_commands=True) + send.side_effect = [BluetoothTimeout()] + + with self.assertRaises(BluetoothTimeout): + await vehicle.charge_port_door_open() + # No prover read issued for an unverifiable command. + self.assertEqual(send.await_count, 1) + + +class InfotainmentVerificationTests(MockedBleTransportTestCase): + async def test_set_charge_limit_verified_executed(self) -> None: + vehicle, send = self.make_vehicle(verify_commands=True) + send.side_effect = [ + BluetoothTimeout(), + infotainment_vehicle_data_reply( + VehicleData(charge_state=ChargeState(charge_limit_soc=80)) + ), + ] + + result = await vehicle.set_charge_limit(80) + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + self.assertEqual(send.await_count, 2) + + async def test_set_charge_limit_mismatch_reraises(self) -> None: + vehicle, send = self.make_vehicle(verify_commands=True) + send.side_effect = [ + BluetoothTimeout(), + infotainment_vehicle_data_reply( + VehicleData(charge_state=ChargeState(charge_limit_soc=70)) + ), + ] + + with self.assertRaises(BluetoothTimeout): + await vehicle.set_charge_limit(80) + + async def test_set_charging_amps_verified_executed(self) -> None: + vehicle, send = self.make_vehicle(verify_commands=True) + send.side_effect = [ + BluetoothTimeout(), + infotainment_vehicle_data_reply( + VehicleData(charge_state=ChargeState(charging_amps=16)) + ), + ] + + result = await vehicle.set_charging_amps(16) + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + + async def test_adjust_volume_verified_executed(self) -> None: + vehicle, send = self.make_vehicle(verify_commands=True) + send.side_effect = [ + BluetoothTimeout(), + infotainment_vehicle_data_reply( + VehicleData(media_state=MediaState(audio_volume=5.0)) + ), + ] + + result = await vehicle.adjust_volume(5.0) + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + + async def test_relative_volume_step_is_unverifiable(self) -> None: + # media_volume_up sends a relative delta - not derivable without a + # pre-read, so it stays inconclusive and re-raises. + vehicle, send = self.make_vehicle(verify_commands=True) + send.side_effect = [BluetoothTimeout()] + + with self.assertRaises(BluetoothTimeout): + await vehicle.media_volume_up() + self.assertEqual(send.await_count, 1) + + async def test_set_temps_verified_executed(self) -> None: + vehicle, send = self.make_vehicle(verify_commands=True) + send.side_effect = [ + BluetoothTimeout(), + infotainment_vehicle_data_reply( + VehicleData( + climate_state=ClimateState( + driver_temp_setting=21.0, passenger_temp_setting=21.0 + ) + ) + ), + ] + + result = await vehicle.set_temps(21.0, 21.0) + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + + async def test_auto_conditioning_start_verified_executed(self) -> None: + vehicle, send = self.make_vehicle(verify_commands=True) + send.side_effect = [ + BluetoothTimeout(), + infotainment_vehicle_data_reply( + VehicleData(climate_state=ClimateState(is_climate_on=True)) + ), + ] + + result = await vehicle.auto_conditioning_start() + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + + async def test_unverifiable_infotainment_command_reraises_without_read( + self, + ) -> None: + # honk_horn is ack-only with no derivable prover. + vehicle, send = self.make_vehicle(verify_commands=True) + send.side_effect = [BluetoothTimeout()] + + with self.assertRaises(BluetoothTimeout): + await vehicle.honk_horn() + self.assertEqual(send.await_count, 1) + + async def test_prover_read_timeout_reraises_original(self) -> None: + # The prover read itself times out (e.g. car asleep for an INFO read): + # fall back to raising rather than waking the car to verify. + vehicle, send = self.make_vehicle(verify_commands=True) + send.side_effect = [BluetoothTimeout(), BluetoothTimeout()] + + with self.assertRaises(BluetoothTimeout): + await vehicle.set_charge_limit(80) + self.assertEqual(send.await_count, 2) + + +class VerificationDisabledTests(MockedBleTransportTestCase): + """Default (verify off) must be byte-identical to today.""" + + async def test_timeout_reraises_without_verification_read(self) -> None: + vehicle, send = self.make_vehicle() # verify_commands defaults to False + send.side_effect = [BluetoothTimeout()] + + with self.assertRaises(BluetoothTimeout): + await vehicle.door_lock() + # No prover read when verification is disabled. + self.assertEqual(send.await_count, 1) + + async def test_infotainment_timeout_reraises_without_read(self) -> None: + vehicle, send = self.make_vehicle() + send.side_effect = [BluetoothTimeout()] + + with self.assertRaises(BluetoothTimeout): + await vehicle.set_charge_limit(80) + self.assertEqual(send.await_count, 1) + + +class HappyPathTimingTests(MockedBleTransportTestCase): + """With verify on, a normal ack must NOT trigger a verification read.""" + + async def test_vcsec_success_issues_no_verification_read(self) -> None: + vehicle, send = self.make_vehicle(verify_commands=True) + send.return_value = vcsec_ok_reply() + + result = await vehicle.door_lock() + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + # Only the command send; the happy path never reads the prover. + self.assertEqual(send.await_count, 1) + + async def test_infotainment_success_issues_no_verification_read(self) -> None: + vehicle, send = self.make_vehicle(verify_commands=True) + send.return_value = infotainment_action_ok_reply() + + result = await vehicle.set_charge_limit(80) + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + self.assertEqual(send.await_count, 1) diff --git a/uv.lock b/uv.lock index e39b0d9..ed7e1ec 100644 --- a/uv.lock +++ b/uv.lock @@ -728,7 +728,7 @@ wheels = [ [[package]] name = "tesla-fleet-api" -version = "1.5.4" +version = "1.6.0" source = { editable = "." } dependencies = [ { name = "aiofiles" },