diff --git a/AGENTS.md b/AGENTS.md index 1711df1..bc4f1fd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -118,6 +118,7 @@ Keep the `tesla-protocol` floor at `>=0.5.0`; earlier releases have generated `. - **Seat indexing gotcha**: two distinct seat enums with different conventions. `Seat` is **0-indexed** (`FRONT_LEFT=0`) and is for the manual seat heater/cooler paths (`remote_seat_heater_request`, `remote_seat_cooler_request`). `AutoSeat` is **1-indexed** (`FRONT_LEFT=1`, `FRONT_RIGHT=2`) and is the correct type for `remote_auto_seat_climate_request` on **both** backends — its values equal Tesla's REST wire values and the proto `AutoSeatPosition_*` enum. Don't mix them; passing a `Seat` to the auto-climate command is off-by-one (issue #11). - **Naming**: camelCase for class instance attributes that mirror API structure (`energySites`, `createFleet`). Snake_case for method names that are API endpoints. - **BLE discovery gotcha**: a Tesla vehicle advertises no 128-bit service UUID pre-connect — only its VIN-derived local name (`^S[a-f0-9]{16}[CDRP]$`), and only in the scan response, not the `ADV_IND`. `SERVICE_UUID` (`tesla_fleet_api/tesla/vehicle/bluetooth.py`) exists only as a GATT service after connecting. Never pass `service_uuids=[SERVICE_UUID]` as a `BleakScanner` discovery-time filter — it hides the vehicle on a direct BlueZ adapter (an ESPHome proxy doesn't enforce that filter the same way, which can mask the bug in testing). Scan unfiltered with active scanning and match by name; keep `SERVICE_UUID` for post-connect GATT use only. +- **`bleak` client/scanner must be resolved dynamically, not import-bound**: both BLE modules (`tesla/vehicle/bluetooth.py`, `tesla/bluetooth.py`) do `import bleak` and reference `bleak.BleakClient`/`bleak.BleakScanner` at call time, never `from bleak import BleakClient`. Home Assistant's habluetooth replaces those `bleak` module attributes at runtime with a multi-adapter/proxy-aware client; a name captured at module import would permanently ignore that replacement and connect on the pristine local backend instead of the HA-selected adapter/ESPHome proxy. Keep the type-only imports under `TYPE_CHECKING`. `connect()` logs a per-stage `stage=...` debug line (`establish_connection`/`start_notify`/`is_connected`/`keepalive`) on transport failure so the underlying connect exception is diagnosable; tests patch the canonical `bleak.BleakScanner`/`bleak.BleakClient` (not a module-level name). Note: this binding is a latent correctness bug independent of any specific incident; ESPHome-proxy connection-slot exhaustion is a separate operational cause that this change does not address. - **BLE domain-routing gotcha**: `Domain` (`tesla_protocol.command.universal_message_pb2`) has more values (`DOMAIN_BROADCAST`, `DOMAIN_AUTHD`, ...) than `VehicleBluetooth._queues` has keys (only `DOMAIN_VEHICLE_SECURITY`/`DOMAIN_INFOTAINMENT`). `_on_message` (`tesla_fleet_api/tesla/vehicle/bluetooth.py`) must look up `_queues` with `.get()` and drop unrecognized domains rather than indexing directly — indexing raises `KeyError` inside the `ReassemblingBuffer` callback, aborting reassembly of any further already-buffered messages in that notification. - **BLE infotainment boot-delay gotcha**: `wake_up()` (VCSEC) returns as soon as the vehicle-security computer acks it, well before the infotainment computer is ready to complete a signed-command handshake. An INFO-domain read/command issued immediately after `wake_up()` can raise `BluetoothTimeout` on the handshake through no fault of the command itself. Live-verified: waiting ~10s after `wake_up()` before the first INFO read is sufficient on the test rig; callers doing INFO work right after waking should retry-with-backoff rather than treat one timeout as failure. - **BLE `vehicle_data()` response-size cap**: the vehicle's signed-command implementation enforces its own response-size limit independent of the BLE transport's packet reassembly. Live-verified: a single-endpoint `vehicle_data()` call (or any of the dedicated per-substate readers like `charge_state()`) succeeds, but requesting as few as two `BluetoothVehicleData` endpoints together reliably raises `TeslaFleetMessageFaultResponseSizeExceedsMTU` (`exceptions.py`). This is why `vehicle_data()`'s `endpoints` arg has no all-endpoints default (unlike the cloud method) - prefer the per-substate readers, or a single-endpoint `vehicle_data()` call, over a multi-endpoint composite. Auto-chunking (split under the cap, merge replies) would fix this properly but is not implemented. diff --git a/tesla_fleet_api/tesla/bluetooth.py b/tesla_fleet_api/tesla/bluetooth.py index ca5cac0..bf99cb3 100644 --- a/tesla_fleet_api/tesla/bluetooth.py +++ b/tesla_fleet_api/tesla/bluetooth.py @@ -4,7 +4,7 @@ import hashlib import re from typing import Any -from bleak import BleakClient +import bleak from bleak.backends.device import BLEDevice from bleak_retry_connector import establish_connection from google.protobuf.json_format import MessageToJson, MessageToDict @@ -40,8 +40,13 @@ async def query_display_name( self, device: BLEDevice, max_attempts: int = 5 ) -> str | None: """Queries the name of a bluetooth vehicle.""" + # Resolve BleakClient dynamically so habluetooth's late-installed + # multi-adapter/proxy client is honored regardless of import order. client = await establish_connection( - BleakClient, device, device.name or "Unknown", max_attempts=max_attempts + bleak.BleakClient, + device, + device.name or "Unknown", + max_attempts=max_attempts, ) name: str | None = None for i in range(max_attempts): diff --git a/tesla_fleet_api/tesla/vehicle/bluetooth.py b/tesla_fleet_api/tesla/vehicle/bluetooth.py index f48f8c5..1b01601 100644 --- a/tesla_fleet_api/tesla/vehicle/bluetooth.py +++ b/tesla_fleet_api/tesla/vehicle/bluetooth.py @@ -9,7 +9,7 @@ from random import randbytes from typing import TYPE_CHECKING, Any, Callable, Generic, TypeVar -from bleak import BleakClient, BleakScanner +import bleak from bleak.backends.characteristic import BleakGATTCharacteristic from bleak.backends.device import BLEDevice from bleak.exc import BleakCharacteristicNotFoundError, BleakError @@ -116,6 +116,11 @@ DEFAULT_KEEPALIVE_INTERVAL = 20.0 if TYPE_CHECKING: + # Resolved dynamically as ``bleak.BleakClient``/``bleak.BleakScanner`` at + # call time so habluetooth's late-installed multi-adapter wrappers win + # regardless of import order; imported here only for type annotations. + from bleak import BleakClient, BleakScanner + from tesla_fleet_api.tesla.tesla import Tesla BluetoothParentT = TypeVar("BluetoothParentT", bound="Tesla") @@ -588,7 +593,7 @@ async def find_vehicle( # No service_uuids filter: the vehicle advertises no service UUID # (SERVICE_UUID is GATT-only, post-connect); active scan is needed # since the name lives in the scan response, not the advertisement. - scanner = BleakScanner(scanning_mode="active") + scanner = bleak.BleakScanner(scanning_mode="active") if address is not None: device = await scanner.find_device_by_address(address) @@ -613,9 +618,13 @@ async def connect(self, max_attempts: int = MAX_CONNECT_ATTEMPTS) -> None: """Connect to the Tesla BLE device.""" if not self.device: raise ValueError(f"BLEDevice {self.ble_name} has not been found or set") + # Resolve BleakClient dynamically: habluetooth replaces + # ``bleak.BleakClient`` with its proxy/multi-adapter client at runtime, + # and an import-time binding would ignore that replacement. + stage = "establish_connection" try: self.client = await establish_connection( - BleakClient, + bleak.BleakClient, self.device, self.vin, disconnected_callback=self._on_ble_disconnected, @@ -623,13 +632,25 @@ async def connect(self, max_attempts: int = MAX_CONNECT_ATTEMPTS) -> None: # ble_device_callback=self.get_device, services=[SERVICE_UUID], ) + stage = "start_notify" await self.client.start_notify(READ_UUID, self._on_notify) + stage = "is_connected" + if not self.client.is_connected: + raise BleakError("client not connected after establish_connection") + stage = "keepalive" await self._start_keepalive() self._set_connected(True) # bleak-esphome converts an aioesphomeapi transport timeout into a # builtin TimeoutError, not a BleakError, so catch both to keep every # connect transport failure within TeslaFleetError. except (BleakError, TimeoutError) as e: + LOGGER.debug( + "BLE connect failed vin=%s stage=%s %s: %s", + self.vin, + stage, + type(e).__name__, + e, + ) client = self.client self.client = None if client: @@ -917,6 +938,12 @@ async def _send( # the connected GATT server, strictly before any backend I/O - # the write never reached the transport, so a fallback router # can safely retry it on another backend. + LOGGER.debug( + "BLE send failed vin=%s stage=characteristic_resolution %s: %s", + self.vin, + type(e).__name__, + e, + ) self._disarm_broadcast_confirmation(domain, broadcast_watcher) raise BluetoothTransportError from e except (BleakError, TimeoutError) as e: diff --git a/tests/test_ble_client_binding.py b/tests/test_ble_client_binding.py new file mode 100644 index 0000000..086ba53 --- /dev/null +++ b/tests/test_ble_client_binding.py @@ -0,0 +1,251 @@ +"""Regression tests for dynamic ``bleak`` client/scanner resolution. + +Home Assistant's habluetooth stack replaces ``bleak.BleakClient`` (and +``bleak.BleakScanner``) at runtime with a multi-adapter/proxy-aware client +*after* this library may already have been imported. A class captured by an +``from bleak import BleakClient`` at module load never sees that replacement, +so the library would connect on the pristine local backend instead of the +Home Assistant-selected adapter or ESPHome proxy. These tests import the +modules first, install a late sentinel onto the ``bleak`` module, and prove the +sentinel - not a stale import-time class - reaches ``establish_connection`` and +``BleakScanner`` construction. + +They also lock in the stage-specific connect diagnostics and confirm the fix +leaves ``Router`` failover semantics untouched. +""" + +from __future__ import annotations + +from typing import Any +from unittest import IsolatedAsyncioTestCase +from unittest.mock import AsyncMock, MagicMock, patch + +from bleak.exc import BleakError +from cryptography.hazmat.primitives.asymmetric import ec + +# Import the modules under test up front, exactly as a consumer that loads this +# library before habluetooth installs its wrappers would. +import tesla_fleet_api.tesla.bluetooth as tesla_bluetooth +import tesla_fleet_api.tesla.vehicle.bluetooth as vehicle_bluetooth +from tesla_fleet_api.exceptions import ( + BluetoothTransportError, + BluetoothUnconfirmedCommand, +) +from tesla_fleet_api.router import VehicleRouter +from tesla_fleet_api.tesla.bluetooth import TeslaBluetooth +from tesla_fleet_api.tesla.vehicle.bluetooth import VehicleBluetooth + +VIN = "5YJXCAE43LF123456" + + +class _SentinelClient: + """Stand-in for habluetooth's late-installed multi-adapter BleakClient.""" + + +def _make_vehicle() -> VehicleBluetooth[Any]: + parent = MagicMock() + parent.private_key = ec.generate_private_key(ec.SECP256R1()) + vehicle = VehicleBluetooth(parent, VIN) + vehicle.device = MagicMock() + vehicle._start_keepalive = AsyncMock() # type: ignore[method-assign] + return vehicle + + +def _make_connected_client() -> MagicMock: + client = MagicMock() + client.start_notify = AsyncMock() + client.disconnect = AsyncMock() + client.is_connected = True + return client + + +class LateBleakClientBindingTests(IsolatedAsyncioTestCase): + """The vehicle transport must honor a bleak.BleakClient swapped in late.""" + + async def test_connect_uses_late_installed_bleak_client(self) -> None: + vehicle = _make_vehicle() + captured: dict[str, Any] = {} + + async def fake_establish(client_cls: Any, *args: Any, **kwargs: Any) -> Any: + captured["client_cls"] = client_cls + return _make_connected_client() + + # Install the sentinel onto the live bleak module AFTER import, then + # dispatch a connect. A dynamic lookup passes the sentinel through; the + # stale import-time class would still be handed to establish_connection. + with ( + patch("bleak.BleakClient", _SentinelClient), + patch.object( + vehicle_bluetooth, "establish_connection", side_effect=fake_establish + ), + ): + await vehicle.connect() + + self.assertIs(captured["client_cls"], _SentinelClient) + + +class LateBleakClientBindingTopLevelTests(IsolatedAsyncioTestCase): + """The top-level Bluetooth helper must honor the same late swap.""" + + async def test_query_display_name_uses_late_installed_bleak_client(self) -> None: + helper = TeslaBluetooth() + device = MagicMock() + device.name = "S8831fdab22879fd7C" + captured: dict[str, Any] = {} + + client = MagicMock() + client.read_gatt_char = AsyncMock(return_value="🔑 MyCar".encode("utf-8")) + client.disconnect = AsyncMock() + + async def fake_establish(client_cls: Any, *args: Any, **kwargs: Any) -> Any: + captured["client_cls"] = client_cls + return client + + with ( + patch("bleak.BleakClient", _SentinelClient), + patch.object( + tesla_bluetooth, "establish_connection", side_effect=fake_establish + ), + ): + name = await helper.query_display_name(device) + + self.assertIs(captured["client_cls"], _SentinelClient) + self.assertEqual(name, "MyCar") + + +class LateBleakScannerBindingTests(IsolatedAsyncioTestCase): + """find_vehicle must construct a late-installed scanner, unfiltered/active.""" + + async def test_find_vehicle_uses_late_installed_scanner(self) -> None: + vehicle = _make_vehicle() + + scanner_instance = MagicMock() + scanner_instance.find_device_by_name = AsyncMock(return_value=MagicMock()) + scanner_factory = MagicMock(return_value=scanner_instance) + + with patch("bleak.BleakScanner", scanner_factory): + await vehicle.find_vehicle() + + scanner_factory.assert_called_once() + _, kwargs = scanner_factory.call_args + # Active scanning is required (the name lives in the scan response), and + # the vehicle advertises no service UUID pre-connect, so the scan must + # stay unfiltered. + self.assertEqual(kwargs.get("scanning_mode"), "active") + self.assertNotIn("service_uuids", kwargs) + + +class ConnectStageDiagnosticsTests(IsolatedAsyncioTestCase): + """A failed connect must name the stage and chain the original cause.""" + + async def _assert_stage_logged( + self, + *, + establish_side_effect: BaseException | None, + start_notify_side_effect: BaseException | None, + expected_stage: str, + underlying: BaseException, + ) -> None: + vehicle = _make_vehicle() + + if establish_side_effect is not None: + establish = AsyncMock(side_effect=establish_side_effect) + else: + client = _make_connected_client() + if start_notify_side_effect is not None: + client.start_notify = AsyncMock(side_effect=start_notify_side_effect) + establish = AsyncMock(return_value=client) + + with patch.object(vehicle_bluetooth, "establish_connection", establish): + with self.assertLogs("tesla_fleet_api", level="DEBUG") as logs: + with self.assertRaises(BluetoothTransportError) as ctx: + await vehicle.connect() + + self.assertIs(ctx.exception.__cause__, underlying) + record = "\n".join(logs.output) + self.assertIn(f"stage={expected_stage}", record) + self.assertIn(type(underlying).__name__, record) + self.assertIn(str(underlying), record) + + async def test_establish_connection_bleak_error_names_stage(self) -> None: + err = BleakError("no adapter") + await self._assert_stage_logged( + establish_side_effect=err, + start_notify_side_effect=None, + expected_stage="establish_connection", + underlying=err, + ) + + async def test_establish_connection_timeout_names_stage(self) -> None: + err = TimeoutError("connect timed out") + await self._assert_stage_logged( + establish_side_effect=err, + start_notify_side_effect=None, + expected_stage="establish_connection", + underlying=err, + ) + + async def test_start_notify_bleak_error_names_stage(self) -> None: + err = BleakError("notify failed") + await self._assert_stage_logged( + establish_side_effect=None, + start_notify_side_effect=err, + expected_stage="start_notify", + underlying=err, + ) + + async def test_start_notify_timeout_names_stage(self) -> None: + err = TimeoutError("start_notify timed out") + await self._assert_stage_logged( + establish_side_effect=None, + start_notify_side_effect=err, + expected_stage="start_notify", + underlying=err, + ) + + +class _FakeCloudFallback: + """A cloud secondary tracking whether the router fell over to it.""" + + def __init__(self) -> None: + self.vin = VIN + self.wake_up_calls = 0 + + async def wake_up(self) -> dict[str, Any]: + self.wake_up_calls += 1 + return {"response": {"result": True, "reason": ""}} + + +class RouterFailoverUnchangedTests(IsolatedAsyncioTestCase): + """The fix must not alter Router failover: a pre-command connect + BluetoothTransportError still fails over to cloud exactly once, and an + ambiguous BluetoothUnconfirmedCommand still never replays.""" + + async def test_pre_command_connect_error_fails_over_once(self) -> None: + primary = _make_vehicle() + primary.connect = AsyncMock( # type: ignore[method-assign] + side_effect=BluetoothTransportError() + ) + primary.connect_if_needed = AsyncMock( # type: ignore[method-assign] + side_effect=BluetoothTransportError() + ) + fallback = _FakeCloudFallback() + router = VehicleRouter(primary, fallback) + + result = await router.wake_up() + + self.assertEqual(result, {"response": {"result": True, "reason": ""}}) + self.assertEqual(fallback.wake_up_calls, 1) + + async def test_unconfirmed_command_does_not_replay(self) -> None: + primary = _make_vehicle() + primary.wake_up = AsyncMock( # type: ignore[method-assign] + side_effect=BluetoothUnconfirmedCommand() + ) + fallback = _FakeCloudFallback() + router = VehicleRouter(primary, fallback) + + with self.assertRaises(BluetoothUnconfirmedCommand): + await router.wake_up() + + self.assertEqual(fallback.wake_up_calls, 0) diff --git a/tests/test_find_vehicle_scan_filter.py b/tests/test_find_vehicle_scan_filter.py index 88225e8..c13b188 100644 --- a/tests/test_find_vehicle_scan_filter.py +++ b/tests/test_find_vehicle_scan_filter.py @@ -40,7 +40,7 @@ def __init__(self, *args, **kwargs): async def find_device_by_name(self, name): return fake_device - with patch("tesla_fleet_api.tesla.vehicle.bluetooth.BleakScanner", FakeScanner): + with patch("bleak.BleakScanner", FakeScanner): device = await vehicle.find_vehicle() self.assertIs(device, fake_device)