diff --git a/AGENTS.md b/AGENTS.md index a9503dd..e3de334 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -126,8 +126,9 @@ Source protobuf definitions live in `proto/`; generated Python files live in `te - **`set_scheduled_departure`'s `preconditioning_enabled`/`off_peak_charging_enabled` args are dead**: live-verified - `ScheduledDepartureAction` (`proto/car_server.proto`) has no fields for them, only `preconditioning_times`/`off_peak_charging_times` (weekday-recurrence only, no on/off). Passing `preconditioning_enabled=False` has no effect on the vehicle's observed state. Not a library bug to "fix" without a wider protocol capability; document, don't rely on these args to gate the feature. - **`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 write-vs-read reliability asymmetry**: on the test rig, plain GATT reads (state readers, `wake_up()`) succeeded consistently, but every signed-command *write* (e.g. `adjust_volume`) reliably hit `aioesphomeapi.core.TimeoutAPIError` waiting for `BluetoothGATTWriteResponse` (or, once, GATT error 133) across 5 separate fresh connections in one session. Reads never showed the same failure. A client-side write timeout did not leave the car mutated (re-read after each failure showed the pre-command value unchanged), so treat it as the write never landing rather than a lost response to an applied change - but always re-read to confirm before assuming so. Root cause not identified; worth a dedicated transport probe before the next live-actuation attempt on this rig. +- **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` (~5.5-5.9s) 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. - **`wake_up()`'s own ACK is an unreliable signal - almost always a false-negative timeout**: live-verified 9/9 across two independent sessions - `wake_up()` raises `BluetoothTimeout` regardless of whether the vehicle was asleep, already awake, freshly connected, or held open for minutes, yet the wake (or no-op) reliably took effect - an immediate follow-up state read on the same connection succeeds. Never treat a `wake_up()` `BluetoothTimeout` as command failure; call it best-effort (catch and ignore) 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. - **`navigation_gps_request`'s `order` param is a raw int, not a callable enum**: `commands.py` used to build it as `NavigationGpsRequest.RemoteNavTripOrder(order)`, treating the protobuf nested-enum wrapper (`EnumTypeWrapper`) as if it were a callable Python `enum.IntEnum` class - it isn't, so every call raised `TypeError` before any message was sent (found live during PR-8; this method had never been exercised over BLE before). Fixed to pass `order=order` directly (matching the working sibling `navigation_gps_destination_request`), which protobuf accepts as a bare int for an enum field at runtime. ## Maintaining this file diff --git a/docs/bluetooth_vehicles.md b/docs/bluetooth_vehicles.md index 679cbf3..8dfac6d 100644 --- a/docs/bluetooth_vehicles.md +++ b/docs/bluetooth_vehicles.md @@ -99,6 +99,20 @@ failures and response-wait `BluetoothTimeout` failures with one library error hierarchy, or catch `BluetoothTransportError` separately when you need to distinguish a transport failure from a vehicle timeout. +## Mutating Command Timeouts + +A `BluetoothTimeout` from a mutating BLE command is inconclusive, not proof that +the command failed. The vehicle can apply the command even when its +acknowledgement does not reach the client. For commands that change vehicle +state, snapshot the relevant state before acting and verify the outcome with a +follow-up state read after any timeout. + +Do not blind-retry non-idempotent commands, such as media toggles, volume steps, +or schedule add/remove operations, on timeout alone. The signed-command retry +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. + ## Climate Commands Bluetooth vehicles support the same signed climate command methods as @@ -289,10 +303,9 @@ such as `now_playing_artist`, `now_playing_title`, and the so track/favorite navigation is best verified by the command acknowledgement and paired with the inverse command when an exact state fingerprint is unavailable. -If a BLE write times out, re-read the relevant state before assuming the command -applied. A timeout while waiting for the GATT write response may mean the -command never reached the vehicle, even when plain BLE reads on the same -connection are succeeding. +If a BLE write times out, re-read the relevant state before assuming whether the +command applied. A timeout while waiting for the GATT write response is not +enough to prove either failure or success. `remote_boombox(sound)` also uses the INFO-domain signed-command transport and plays through the vehicle external speaker. Use it only when someone is present diff --git a/docs/fleet_api_signed_commands.md b/docs/fleet_api_signed_commands.md index 3daf444..9dfa3e7 100644 --- a/docs/fleet_api_signed_commands.md +++ b/docs/fleet_api_signed_commands.md @@ -178,6 +178,15 @@ not map to fields in the vehicle's signed-command protobuf. charge limit already equals `charge_limit_soc_std`, the vehicle may reject the command with `already_standard`. +## Signed Command Retries + +Signed commands can be re-signed and re-sent by the library after a WAIT status +or an epoch/token fault, up to a bounded retry limit. For non-idempotent commands, +that retry window can apply the same command more than once if the first attempt +actually executed despite the WAIT/fault reply. Verify commands such as media +toggles, volume steps, and schedule add/remove operations by reading absolute +state after the call instead of relying on the number of send attempts. + ## Flash Lights You can flash the lights of a specific vehicle using its VIN: diff --git a/tesla_fleet_api/tesla/vehicle/bluetooth.py b/tesla_fleet_api/tesla/vehicle/bluetooth.py index a5c2c6b..3983d46 100644 --- a/tesla_fleet_api/tesla/vehicle/bluetooth.py +++ b/tesla_fleet_api/tesla/vehicle/bluetooth.py @@ -182,6 +182,15 @@ class VehicleBluetooth(Commands[BluetoothParentT], Generic[BluetoothParentT]): connect/write transport failures surface as ``BluetoothTransportError`` and a response-wait timeout as ``BluetoothTimeout``, both ``TeslaFleetError`` subclasses with the original transport exception chained as their cause. + + A ``BluetoothTimeout`` raised by a *mutating* command (RKE/closure + actions, HVAC/media/charging commands, ``wake_up``) is inconclusive, not + a failure - the vehicle can execute the command without its ack reaching + the client. Callers must snapshot state before acting and verify the + outcome with a follow-up state read after any timeout, and must never + 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. """ ble_name: str diff --git a/tesla_fleet_api/tesla/vehicle/commands.py b/tesla_fleet_api/tesla/vehicle/commands.py index d193dd7..0cca9d2 100644 --- a/tesla_fleet_api/tesla/vehicle/commands.py +++ b/tesla_fleet_api/tesla/vehicle/commands.py @@ -354,7 +354,13 @@ def validate_msg(self, msg: RoutableMessage) -> None: async def _command( self, domain: Domain, command: bytes, attempt: int = 0 ) -> dict[str, Any]: - """Serialize a message and send to the signed command endpoint.""" + """Serialize a message and send to the signed command endpoint. + + On a WAIT status or an epoch/token fault, this re-signs and re-sends + the identical command (bounded at 3 attempts) - for a non-idempotent + command that window can apply it twice if the first attempt actually + executed despite the WAIT/fault reply. + """ session = self._sessions[domain] if not session.ready: await self._handshake(domain)