diff --git a/AGENTS.md b/AGENTS.md index 225ae49..24faf39 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -132,6 +132,7 @@ Source protobuf definitions live in `proto/`; generated Python files live in `te - **`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. - **`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. - **`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). +- **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). ## Maintaining this file diff --git a/docs/bluetooth_vehicles.md b/docs/bluetooth_vehicles.md index a953ed0..0025a2a 100644 --- a/docs/bluetooth_vehicles.md +++ b/docs/bluetooth_vehicles.md @@ -309,6 +309,9 @@ commands: - `media_next_fav()` - `media_prev_fav()` +`adjust_volume(volume)` accepts absolute volume values from `0.0` through +`11.0`, matching the Fleet API command validation. + These commands return the signed-command acknowledgement, not a media-state diff. For verification, prefer `media_state().audio_volume` and `media_state().media_playback_status` where they apply. Track identity fields diff --git a/docs/fleet_api_signed_commands.md b/docs/fleet_api_signed_commands.md index 9dfa3e7..dafcc61 100644 --- a/docs/fleet_api_signed_commands.md +++ b/docs/fleet_api_signed_commands.md @@ -187,6 +187,9 @@ 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. +`adjust_volume(volume)` accepts absolute volume values from `0.0` through +`11.0`, matching the Fleet API command validation. + ## Flash Lights You can flash the lights of a specific vehicle using its VIN: diff --git a/tesla_fleet_api/tesla/vehicle/commands.py b/tesla_fleet_api/tesla/vehicle/commands.py index 94013d1..6c73080 100644 --- a/tesla_fleet_api/tesla/vehicle/commands.py +++ b/tesla_fleet_api/tesla/vehicle/commands.py @@ -756,7 +756,9 @@ async def actuate_trunk(self, which_trunk: Trunk | str) -> dict[str, Any]: raise ValueError("Invalid trunk.") async def adjust_volume(self, volume: float) -> dict[str, Any]: - """Adjusts vehicle media playback volume.""" + """Adjusts vehicle media playback volume from 0.0 to 11.0.""" + if volume < 0.0 or volume > 11.0: + raise ValueError("Volume must a number from 0.0 to 11.0") return await self._sendInfotainment( Action( vehicleAction=VehicleAction( diff --git a/tesla_fleet_api/tesla/vehicle/fleet.py b/tesla_fleet_api/tesla/vehicle/fleet.py index c958697..715032c 100644 --- a/tesla_fleet_api/tesla/vehicle/fleet.py +++ b/tesla_fleet_api/tesla/vehicle/fleet.py @@ -42,7 +42,7 @@ async def actuate_trunk(self, which_trunk: Trunk | str) -> dict[str, Any]: ) async def adjust_volume(self, volume: float) -> dict[str, Any]: - """Adjusts vehicle media playback volume.""" + """Adjusts vehicle media playback volume from 0.0 to 11.0.""" if volume < 0.0 or volume > 11.0: raise ValueError("Volume must a number from 0.0 to 11.0") return await self._request( @@ -582,7 +582,8 @@ async def trigger_homelink( data: dict[str, str | float] = {} if token: data["token"] = token - if lat and lon: + # Guard on None, not truthiness: lat/lon of 0.0 are valid coordinates. + if lat is not None and lon is not None: data["lat"] = lat data["lon"] = lon return await self._request( diff --git a/tests/test_cross_transport_parity.py b/tests/test_cross_transport_parity.py new file mode 100644 index 0000000..02a6f74 --- /dev/null +++ b/tests/test_cross_transport_parity.py @@ -0,0 +1,124 @@ +"""Cross-transport parity: the same Python call must build semantically +equivalent vehicle instructions over the Fleet REST (cloud) path and the BLE +signed-command (protobuf) path. + +Response bodies legitimately differ in FORM (cloud returns a REST JSON dict, +BLE returns decoded protobuf), so these tests assert on the *outbound +instruction* each transport constructs from identical arguments - that is where +a parameter-handling divergence between the two code paths would show up. + +The cloud path is exercised through ``VehicleFleet`` with ``_request`` mocked; +the BLE path through ``VehicleBluetooth`` with ``_send`` mocked (see +``ble_mocked_transport``). +""" + +from __future__ import annotations + +from typing import Any, cast +from unittest.mock import AsyncMock, MagicMock + +from tesla_fleet_api.tesla.vehicle.bluetooth import VehicleBluetooth +from tesla_fleet_api.tesla.vehicle.fleet import VehicleFleet +from tesla_fleet_api.tesla.vehicle.proto.car_server_pb2 import Action, VehicleAction +from tesla_fleet_api.tesla.vehicle.proto.universal_message_pb2 import RoutableMessage + +from ble_mocked_transport import ( + MockedBleTransportTestCase, + decrypt_sent_command, + infotainment_action_ok_reply, +) + + +def _make_fleet_vehicle(vin: str) -> tuple[VehicleFleet[Any], AsyncMock]: + """A ``VehicleFleet`` whose ``_request`` is mocked to capture the REST call.""" + parent = MagicMock() + request = AsyncMock(return_value={"response": {"result": True}}) + parent._request = request # pyright: ignore[reportAttributeAccessIssue] + return VehicleFleet(parent, vin), request + + +def _sent_vehicle_action( + vehicle: VehicleBluetooth[Any], send: AsyncMock +) -> VehicleAction: + """Decrypt the signed command the BLE transport was about to send.""" + assert send.await_args is not None + sent_msg = cast("RoutableMessage", send.await_args.args[0]) + plaintext = decrypt_sent_command(vehicle, sent_msg) + return Action.FromString(plaintext).vehicleAction + + +class TriggerHomelinkParityTests(MockedBleTransportTestCase): + """``trigger_homelink`` must carry the given coordinates on both transports. + + Regression: the cloud path used a truthy ``if lat and lon`` check that + silently dropped a valid ``0.0`` coordinate, while BLE (``is not None``) + kept it - a genuine parameter-handling divergence. + """ + + async def test_zero_coordinates_survive_on_both_transports(self) -> None: + # Cloud (REST) + cloud, request = _make_fleet_vehicle(self.VIN) + await cloud.trigger_homelink(token="tok", lat=0.0, lon=0.0) + assert request.await_args is not None + cloud_json = request.await_args.kwargs["json"] + self.assertEqual(cloud_json["lat"], 0.0) + self.assertEqual(cloud_json["lon"], 0.0) + self.assertEqual(cloud_json["token"], "tok") + + # BLE (signed protobuf) + ble, send = self.make_vehicle() + send.return_value = infotainment_action_ok_reply() + await ble.trigger_homelink(token="tok", lat=0.0, lon=0.0) + action = _sent_vehicle_action(ble, send).vehicleControlTriggerHomelinkAction + self.assertEqual(action.location.latitude, 0.0) + self.assertEqual(action.location.longitude, 0.0) + self.assertEqual(action.token, "tok") + + async def test_omitted_coordinates_absent_on_both_transports(self) -> None: + cloud, request = _make_fleet_vehicle(self.VIN) + await cloud.trigger_homelink(token="tok") + assert request.await_args is not None + cloud_json = request.await_args.kwargs["json"] + self.assertNotIn("lat", cloud_json) + self.assertNotIn("lon", cloud_json) + + ble, send = self.make_vehicle() + send.return_value = infotainment_action_ok_reply() + await ble.trigger_homelink(token="tok") + action = _sent_vehicle_action(ble, send).vehicleControlTriggerHomelinkAction + # No location submessage set when coordinates are omitted. + self.assertFalse(action.HasField("location")) + + +class AdjustVolumeParityTests(MockedBleTransportTestCase): + """``adjust_volume`` must reject the same out-of-range values on both + transports. + + Regression: the cloud path validated ``0.0 <= volume <= 11.0`` and raised + ``ValueError``; the BLE path forwarded any float to the car unchecked - a + parameter-validation divergence. + """ + + async def test_out_of_range_rejected_on_both_transports(self) -> None: + for bad in (-1.0, 11.5): + cloud, request = _make_fleet_vehicle(self.VIN) + with self.assertRaises(ValueError): + await cloud.adjust_volume(bad) + request.assert_not_awaited() + + ble, send = self.make_vehicle() + with self.assertRaises(ValueError): + await ble.adjust_volume(bad) + send.assert_not_awaited() + + async def test_in_range_sends_absolute_volume_on_both_transports(self) -> None: + cloud, request = _make_fleet_vehicle(self.VIN) + await cloud.adjust_volume(5.0) + assert request.await_args is not None + self.assertEqual(request.await_args.kwargs["json"], {"volume": 5.0}) + + ble, send = self.make_vehicle() + send.return_value = infotainment_action_ok_reply() + await ble.adjust_volume(5.0) + action = _sent_vehicle_action(ble, send) + self.assertAlmostEqual(action.mediaUpdateVolume.volume_absolute_float, 5.0)