diff --git a/AGENTS.md b/AGENTS.md index 84737f4..b29b269 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -139,7 +139,7 @@ Source protobuf definitions live in `proto/`; generated Python files live in `te - **`ReassemblingBuffer` resets on a >1s inter-chunk gap, not just on decode failure**: `bluetooth.py`'s `ReassemblingBuffer.receive_data` discards any in-progress partial frame if the next chunk arrives more than `STALE_CHUNK_TIMEOUT` (1s) after the previous one, mirroring Tesla's official Go SDK (`teslamotors/vehicle-command`, `pkg/connector/ble/ble.go`'s `rxTimeout`). Without this, a chunk dropped mid-message left a stale partial in the buffer that got prepended to the next message, corrupting it until a lucky decode failure resynced. This is a frame-integrity hardening, not a fix for the separate ack-loss behavior documented above (that's a stalled/silent link, which no buffer-side reset can recover). - **`pair()` confirms whitelisting two ways: one-shot reply OR verify-by-state poll**: the whitelist-op success is a single VCSEC frame, and it lands on a dead session (lost forever) if the BLE link cycles while the user walks to the car to approve - live-observed on HA/macOS CoreBluetooth, where `tesla_fleet_api` logged "Reconnecting to S..." every ~100s during a pending pair, and the plain one-shot wait hung despite car-side completion. `pair()` (`bluetooth.py`) keeps the reply as the fast path (waits one `poll_interval` for it) but, on a lost reply, polls `_pair_probe()` every `poll_interval` until an overall `timeout` (default 300s) elapses. The probe is a VCSEC `_handshake` with our own public key: it succeeds only once the key is whitelisted and faults `NotOnWhitelistFault` until then - `_pair_probe` maps any `TeslaFleetError` (incl. transport failures from a mid-wait reconnect) to "not yet", so polling survives reconnects. The whitelist op is written **exactly once** - never re-sent - because a re-send re-prompts the user (and the retry/double-execute hazard documented above applies). Deadline with neither path confirming raises a typed `BluetoothTimeout`. Default behavior, no new knob (`poll_interval` is a defaulted param). - **Idle BLE keepalive (`keepalive_interval`)**: an idle held BLE link to the vehicle drops at ~42s mean lifetime (link supervision timeout ~720ms underneath, bluetoothd-verified on macOS CoreBluetooth); a single trivial passive GATT read every ~20s extends lifetime ~10x (~400s observed). `VehicleBluetooth.__init__`'s `keepalive_interval` (default `DEFAULT_KEEPALIVE_INTERVAL` = 20.0, `None`/`0` disables; threaded through `Vehicles`/`VehiclesBluetooth.create*`) starts one asyncio task per connection (`_keepalive_loop`, `bluetooth.py`) that reads `VERSION_UUID` only after `keepalive_interval` seconds of genuine GATT idleness. **Idle-triggered, not periodic**: `_last_activity` is bumped on every `_send` write and every `_on_notify` frame, so an active session never gets extra traffic; the loop recomputes the wait each pass and fires only when idle. The read is **bounded** (`_keepalive_timeout`, 2s) and **best-effort** - a prior un-timed RSSI read hung indefinitely against a sleeping car, so every attempt carries a timeout and swallows all failures (`_keepalive_read` catches `Exception`, never `CancelledError`); a failed keepalive never raises into user code, never triggers reconnect (the existing `connect_if_needed` machinery owns recovery), and never wakes the car. Task lifecycle is tied to the connection: started at the end of `connect()` (after `start_notify`), cancelled-and-awaited in `disconnect()` and restarted cleanly on reconnect (`_start_keepalive`/`_stop_keepalive`). **Sleep tradeoff**: these reads keep an *awake* car awake and defer vehicle sleep - consumers wanting the car to sleep should disable keepalive or disconnect when idle. Tests in `tests/test_ble_keepalive.py`. -- **Cross-transport parity (cloud REST `VehicleFleet` vs BLE `Commands`)**: the same-named command on both paths should build a semantically equivalent instruction from identical args - a divergence there is a bug, but response *bodies* legitimately differ (REST JSON dict vs decoded protobuf) and are not. `tests/test_cross_transport_parity.py` locks the equivalence in with mocked-both-transports tests. Known **non-bug FORM differences** (do not "fix"): `set_scheduled_departure`'s `preconditioning_enabled`/`off_peak_charging_enabled` (no proto fields), `window_control` lat/lon and `navigation_sc_request` `id` (no proto fields), `navigation_request`'s `type`/`locale`/`timestamp_ms` (REST share-intent framing), and `media_volume_up` (no Tesla REST endpoint - BLE-only; cloud raises volume via `adjust_volume`). Two **open divergences left unfixed** pending live verification: `clear_pin_to_drive_admin` builds `DrivingClearSpeedLimitPinAction` (speed-limit PIN, not PIN-to-Drive - suspected mismapping, security-sensitive), and `navigation_gps_request`'s `order` is required on BLE but optional on cloud (signature mismatch; null-order wire semantics undecided). +- **Cross-transport parity (cloud REST `VehicleFleet` vs BLE `Commands`)**: the same-named command on both paths should build a semantically equivalent instruction from identical args - a divergence there is a bug, but response *bodies* legitimately differ (REST JSON dict vs decoded protobuf) and are not. `tests/test_cross_transport_parity.py` locks the equivalence in with mocked-both-transports tests. Known **non-bug FORM differences** (do not "fix"): `set_scheduled_departure`'s `preconditioning_enabled`/`off_peak_charging_enabled` (no proto fields), `window_control` lat/lon and `navigation_sc_request` `id` (no proto fields), `navigation_request`'s `type`/`locale`/`timestamp_ms` (REST share-intent framing), and `media_volume_up` (no Tesla REST endpoint - BLE-only; cloud raises volume via `adjust_volume`). One **open divergence left unfixed** pending live verification: `clear_pin_to_drive_admin` builds `DrivingClearSpeedLimitPinAction` (speed-limit PIN, not PIN-to-Drive - suspected mismapping, security-sensitive). `navigation_gps_request`'s prior `order` signature mismatch is resolved: both transports now default `order` to `0` (`REMOTE_NAV_TRIP_ORDER_UNKNOWN`, a defined proto enum value) when the caller omits it, so cloud always sends an explicit `order` rather than `null`. - **Per-command debug logging chokepoints and the `command=` name it derives**: `LOGGER.debug` lines of the form `command= transport= result=...` are emitted from exactly four places, not per-method - `Commands._sendVehicleSecurity`/`_getVehicleSecurity`/`_sendInfotainment`/`_getInfotainment` (`commands.py`, covers both BLE and Fleet-signed) and `TeslaFleetApi._request` (`fleet.py`, covers Fleet/Teslemetry/Tessie REST). `transport` comes from a `_transport_name` `ClassVar` set per concrete class (`"bluetooth"`/`"fleet"`/`"teslemetry"`/`"tessie"`), mirroring the existing `_auth_method` pattern - add that ClassVar to any new `Commands`/`TeslaFleetApi` subclass. For BLE/Fleet-signed, `command` is **not** the Python method name; it's derived from the populated protobuf oneof field (`vcsec_command_name`/`infotainment_command_name` in `commands.py`), e.g. `door_lock()` logs as `RKE_ACTION_LOCK` and `set_charge_limit()` as `chargingSetLimitAction` - deliberately robust to call-site changes since it reads the message being sent, not the call stack. `VehicleBluetooth`'s `verify_commands` resolution logs a second, separate line (`verify_commands=resolved`/`unresolved`) rather than duplicating the base class's raw-attempt line. `Router._dispatch` (`router/base.py`) logs `command=... backend= result=...` per backend tried, independent of the above. See `docs/bluetooth_vehicles.md`'s "Troubleshooting: Enable Debug Logging" section for the user-facing format; `tests/test_command_logging.py` locks in the exact line shapes. - **`_log_request_result` (`fleet.py`) must tolerate any JSON-legal REST body, not just dicts**: it runs after the HTTP request already succeeded, so it's a logging convenience only - a non-dict body (`null`, a list, a bare scalar; live case: Teslemetry's `list_authorized_clients` returning `null`) must never raise there. It guards with `isinstance(data, dict)` before calling `.get()`, logging `result=success` and returning for anything else. Regression tests in `tests/test_command_logging.py` (`test_null_json_body_returns_none_without_raising` etc.) cover null/list/scalar bodies. - **Typed accessor pattern for undocumented raw-dict responses**: `TeslemetryEnergySite.find_authorized_clients()` (`teslemetry/energysite.py`) is the library's first frozen-dataclass typed wrapper over a raw `dict[str, Any]`/`None`/`list` REST response, added so API-parsing logic (envelope unwrap, field lookup, shape validation, enum typing) lives in the library instead of each consumer (e.g. Home Assistant's config flow) reimplementing it. `_authorized_clients_list()` does one precise, non-recursive envelope unwrap (`{"response": {"authorized_clients": [...]}}` or `{"response": {"clients": [...]}}`, or a bare list with no envelope) rather than a multi-key/depth-first search - the `clients` key variant was added after a live capture (Tesla Release 953) showed the endpoint's real key differs from the originally-documented `authorized_clients`, confirmed against a populated 5-entry sample. `AuthorizedClient` models only the two fields a pairing flow actually reads (`public_key`, `state`), each accepting only the specific key-name variant pairs empirically evidenced, not speculative extras. Two rules any future typed accessor over an undocumented response shape must keep, demonstrated here even though `AuthorizedClientState` (`const.py`) has no falsy member: (1) field lookup must check key presence (`key in payload`), never `payload.get(key) or default` - a legal falsy value is not "missing", and a present-but-unrecognized enum value (`_normalize_state()`) is returned raw rather than coerced to `None`; (2) a `None` body and an unrecognized response shape (not a dict/list, or an envelope that unwraps to neither accepted list key) are malformed data, not "zero clients" - Tesla's endpoint intermittently returns HTTP 200 with a null body, so `_authorized_clients_list()` raises `InvalidResponse` (`exceptions.py`) for both rather than collapsing them to `[]`; only a genuinely empty list under either accepted key parses to `AuthorizedClients.clients == []` without raising. Tesla has not published an OpenAPI schema for pairing endpoints, so `const.py`'s enums are the schema of record; widen `AuthorizedClient`'s modeled fields only against a further live sample, not speculatively. The raw `list_authorized_clients()` method is kept alongside as the untyped escape hatch and still returns a null body unchanged rather than raising. Tests: `tests/test_teslemetry_authorized_clients.py`. diff --git a/docs/bluetooth_vehicles.md b/docs/bluetooth_vehicles.md index 358bafd..616e609 100644 --- a/docs/bluetooth_vehicles.md +++ b/docs/bluetooth_vehicles.md @@ -373,7 +373,7 @@ a plugged-in vehicle unless someone can reseat the cable if needed. commands over BLE: - `navigation_request(value)` -- `navigation_gps_request(lat, lon, order)` +- `navigation_gps_request(lat, lon, order=0)` - `navigation_sc_request(order)` - `navigation_waypoints_request(waypoints)` - `navigation_gps_destination_request(lat, lon, destination, order)` @@ -384,6 +384,8 @@ commands over BLE: For the GPS navigation methods, `order` is the Tesla/protobuf remote-nav order integer: `1` replaces the trip, `2` prepends a stop, and `3` appends a stop. +`navigation_gps_request`'s `order` defaults to `0` +(`REMOTE_NAV_TRIP_ORDER_UNKNOWN`) when omitted, matching the cloud path. These commands are ACK-only over BLE; the library returns the vehicle's command acknowledgement, but there is no separate BLE state prover for the navigation destination, dashcam clip save, flash-lights action, or power-mode toggle. diff --git a/docs/fleet_api_signed_commands.md b/docs/fleet_api_signed_commands.md index 15ce55a..c36121c 100644 --- a/docs/fleet_api_signed_commands.md +++ b/docs/fleet_api_signed_commands.md @@ -241,7 +241,7 @@ asyncio.run(main()) Signed commands include the navigation and dashcam command methods: - `navigation_request(value)` -- `navigation_gps_request(lat, lon, order)` +- `navigation_gps_request(lat, lon, order=0)` - `navigation_sc_request(order)` - `navigation_waypoints_request(waypoints)` - `navigation_gps_destination_request(lat, lon, destination, order)` @@ -249,6 +249,8 @@ Signed commands include the navigation and dashcam command methods: For the GPS navigation methods, `order` is the Tesla/protobuf remote-nav order integer: `1` replaces the trip, `2` prepends a stop, and `3` appends a stop. +`navigation_gps_request`'s `order` defaults to `0` +(`REMOTE_NAV_TRIP_ORDER_UNKNOWN`) when omitted, matching the BLE path. ## Honk Horn diff --git a/tesla_fleet_api/tesla/vehicle/commands.py b/tesla_fleet_api/tesla/vehicle/commands.py index a1a7ca4..e032839 100644 --- a/tesla_fleet_api/tesla/vehicle/commands.py +++ b/tesla_fleet_api/tesla/vehicle/commands.py @@ -1105,12 +1105,13 @@ async def media_volume_up(self) -> dict[str, Any]: ) async def navigation_gps_request( - self, lat: float, lon: float, order: int + self, lat: float, lon: float, order: int = 0 ) -> dict[str, Any]: """Start navigation to coordinates. ``order`` is the Tesla/protobuf remote-nav order integer: 1 replaces - the trip, 2 prepends a stop, and 3 appends a stop. + the trip, 2 prepends a stop, and 3 appends a stop. Defaults to 0 + (``REMOTE_NAV_TRIP_ORDER_UNKNOWN``) when omitted, matching the cloud path. """ return await self._sendInfotainment( Action( diff --git a/tesla_fleet_api/tesla/vehicle/fleet.py b/tesla_fleet_api/tesla/vehicle/fleet.py index 715032c..98e3dcd 100644 --- a/tesla_fleet_api/tesla/vehicle/fleet.py +++ b/tesla_fleet_api/tesla/vehicle/fleet.py @@ -194,12 +194,13 @@ async def media_volume_down(self) -> dict[str, Any]: ) async def navigation_gps_request( - self, lat: float, lon: float, order: int | None = None + self, lat: float, lon: float, order: int = 0 ) -> dict[str, Any]: """Start navigation to coordinates. ``order`` is the Tesla remote-nav order integer: 1 replaces the trip, - 2 prepends a stop, and 3 appends a stop. + 2 prepends a stop, and 3 appends a stop. Defaults to 0 + (``REMOTE_NAV_TRIP_ORDER_UNKNOWN``) when omitted, matching the BLE path. """ return await self._request( Method.POST, diff --git a/tests/test_cross_transport_parity.py b/tests/test_cross_transport_parity.py index 02a6f74..b9f457e 100644 --- a/tests/test_cross_transport_parity.py +++ b/tests/test_cross_transport_parity.py @@ -122,3 +122,42 @@ async def test_in_range_sends_absolute_volume_on_both_transports(self) -> None: await ble.adjust_volume(5.0) action = _sent_vehicle_action(ble, send) self.assertAlmostEqual(action.mediaUpdateVolume.volume_absolute_float, 5.0) + + +class NavigationGpsRequestParityTests(MockedBleTransportTestCase): + """``navigation_gps_request``'s ``order`` must default identically on + both transports. + + Regression: cloud's ``order`` was optional (omitted -> ``null`` on the + wire) while BLE's was a required int - a signature mismatch. Both now + default to ``0`` (``REMOTE_NAV_TRIP_ORDER_UNKNOWN``) when omitted. + """ + + async def test_omitted_order_defaults_to_zero_on_both_transports(self) -> None: + cloud, request = _make_fleet_vehicle(self.VIN) + await cloud.navigation_gps_request(37.3230, -122.0322) + assert request.await_args is not None + self.assertEqual( + request.await_args.kwargs["json"], + {"lat": 37.3230, "lon": -122.0322, "order": 0}, + ) + + ble, send = self.make_vehicle() + send.return_value = infotainment_action_ok_reply() + await ble.navigation_gps_request(37.3230, -122.0322) + action = _sent_vehicle_action(ble, send) + self.assertEqual(action.navigationGpsRequest.order, 0) + + async def test_explicit_order_passes_through_unchanged_on_both_transports( + self, + ) -> None: + cloud, request = _make_fleet_vehicle(self.VIN) + await cloud.navigation_gps_request(37.3230, -122.0322, order=2) + assert request.await_args is not None + self.assertEqual(request.await_args.kwargs["json"]["order"], 2) + + ble, send = self.make_vehicle() + send.return_value = infotainment_action_ok_reply() + await ble.navigation_gps_request(37.3230, -122.0322, order=2) + action = _sent_vehicle_action(ble, send) + self.assertEqual(action.navigationGpsRequest.order, 2) diff --git a/uv.lock b/uv.lock index a48a120..af8d36c 100644 --- a/uv.lock +++ b/uv.lock @@ -728,7 +728,7 @@ wheels = [ [[package]] name = "tesla-fleet-api" -version = "1.7.4" +version = "1.7.5" source = { editable = "." } dependencies = [ { name = "aiofiles" },