diff --git a/custom_components/philips_shaver/config_flow.py b/custom_components/philips_shaver/config_flow.py index 9923877..1116ed1 100644 --- a/custom_components/philips_shaver/config_flow.py +++ b/custom_components/philips_shaver/config_flow.py @@ -38,6 +38,11 @@ ) from homeassistant.data_entry_flow import AbortFlow from bleak import BleakClient + +try: + from habluetooth.usage import ORIGINAL_BLEAK_CLIENT +except ImportError: # pragma: no cover - compatibility with the pinned old test stack + ORIGINAL_BLEAK_CLIENT = BleakClient from bleak.exc import BleakError from bleak_retry_connector import ( BleakAbortedError, @@ -102,7 +107,7 @@ async_unpair_bridge_slot, describe_available_paths, describe_connection_path, - is_local_bluez_connection, + local_bluez_device_from_address, slot_changed_at, ) from .exceptions import ( @@ -646,9 +651,24 @@ async def _async_fetch_capabilities( ) raise DeviceAsleepException - device = async_ble_device_from_address(self.hass, address) + # "Direct Bluetooth" means the Home Assistant host adapter. + # Do not let HA's normal RSSI-based connection ranking escape to a + # stronger stock ESPHome proxy, whose bond is unrelated to BlueZ. + device = local_bluez_device_from_address(self.hass, address) if not device: - raise DeviceNotFoundException("BLE device not found") + paths = describe_available_paths(self.hass, address) + proxy = next((p for p in paths if not p["is_local"]), None) + if proxy is not None: + self._probe_via_proxy = True + self._probe_proxy_name = self._short_scanner(proxy) + raise NotPairedException( + "Device is visible only through a standard Bluetooth proxy" + ) + raise DeviceNotFoundException( + "BLE device not found on a local Bluetooth adapter" + ) + self._probe_via_proxy = False + self._probe_proxy_name = None client: BleakClient | None = None try: @@ -668,7 +688,7 @@ async def _async_fetch_capabilities( ) try: client = await establish_connection( - BleakClient, device, "philips_shaver", + ORIGINAL_BLEAK_CLIENT, device, "philips_shaver", use_services_cache=True, timeout=30.0, ) finally: @@ -682,17 +702,9 @@ async def _async_fetch_capabilities( capabilities["connection_path"] = describe_connection_path( self.hass, client, device ) - # Remember which transport carried this probe: a later - # NotPairedException must route to the matching pairing - # dialog (host instructions vs. proxy guidance) and decide - # whether the D-Bus pairing machinery applies at all. - self._probe_via_proxy = not is_local_bluez_connection(client) - # Scanner names carry the adapter MAC in parentheses — strip it - # for the dialog (same as _short_scanner in the preview). - self._probe_proxy_name = ( - capabilities["connection_path"].split(" (")[0] - if self._probe_via_proxy else None - ) + # This connection was deliberately opened on local BlueZ. + self._probe_via_proxy = False + self._probe_proxy_name = None _LOGGER.info( "%s: capabilities probe connected via %s", address, @@ -1028,7 +1040,8 @@ async def async_step_bluetooth_confirm( # scanner is the likely carrier — the BlueZ bond state says # nothing about a proxy-carried connection. paths = describe_available_paths(self.hass, address) - likely_proxy = bool(paths) and not paths[0]["is_local"] + has_local = any(bool(p["is_local"]) for p in paths) + likely_proxy = bool(paths) and not has_local if not likely_proxy: from .dbus_pairing import is_dbus_available, async_is_device_paired @@ -1100,9 +1113,8 @@ def _transport_lines(self) -> tuple[str, str, dict[str, str]]: a standard Bluetooth proxy cannot complete — pairing over one fails outright, so the warning is unconditional and hard. - habluetooth routes by signal strength, so the strongest scanner - is only the *likely* carrier; recomputed each render so the - ranking stays current. + Direct Bluetooth deliberately prefers a local HA Host adapter. + A stock proxy is shown only when no usable local path is available. """ address = self.discovery_info.address if self.discovery_info else "" paths = describe_available_paths(self.hass, address) @@ -1114,22 +1126,22 @@ def _transport_lines(self) -> tuple[str, str, dict[str, str]]: def _rssi(p: dict) -> str: return f" ({p['rssi']} dBm)" if p["rssi"] is not None else "" + local = next((p for p in paths if p["is_local"]), None) + if local is not None: + local_name = self._short_scanner(local) + local_rssi = ( + f", {local['rssi']} dBm" if local["rssi"] is not None else "" + ) + return ( + f" via **Direct Bluetooth** ({local_name}{local_rssi})", + "", + empty, + ) + best = paths[0] best_name = self._short_scanner(best) best_rssi = f", {best['rssi']} dBm" if best["rssi"] is not None else "" - - if best["is_local"]: - via = f" via **Direct Bluetooth** ({best_name}{best_rssi})" - return via, "", empty - via = f" via **Bluetooth proxy** ({best_name}{best_rssi})" - - # Markdown is not parsed inside an HTML block, so the warning uses - # /
for emphasis and paragraph breaks. - # Only names, signal strengths and the markup ha-markdown needs - # inside an HTML block travel as values; the wording itself lives - # in the translations. - local = next((p for p in paths if p["is_local"]), None) values = { "proxy_name": f"{best_name}", "proxy_rssi": _rssi(best), @@ -1137,11 +1149,7 @@ def _rssi(p: dict) -> str: "local_rssi": "", "nl": "

", } - if local is None: - return via, "proxy", values - values["local_name"] = f"{self._short_scanner(local)}" - values["local_rssi"] = _rssi(local) - return via, "proxy_local", values + return via, "proxy", values # ------------------------------------------------------------------ # Direct BLE probe as a progress task (discovery + manual + pair) @@ -1432,13 +1440,16 @@ async def async_step_user_bleak( self._manual_address_entry = True else: address = raw.upper() - await self.async_set_unique_id(address) + await self.async_set_unique_id( + address, raise_on_progress=False + ) self._abort_if_already_configured() # Quick D-Bus pre-check (same as bluetooth_confirm path); # skipped when a remote scanner is the likely carrier. paths = describe_available_paths(self.hass, address) - likely_proxy = bool(paths) and not paths[0]["is_local"] + has_local = any(bool(p["is_local"]) for p in paths) + likely_proxy = bool(paths) and not has_local if not likely_proxy: from .dbus_pairing import is_dbus_available, async_is_device_paired @@ -1466,10 +1477,8 @@ async def async_step_user_bleak( ) # Build the discovered-device picker. Each option label carries the - # advertisement age, RSSI and the scanner that would likely carry - # the connect — the step is titled "Direct Bluetooth", but - # habluetooth routes by signal strength and may pick a - # bluetooth_proxy (which cannot pair a shaver). + # advertisement age, RSSI and the route Direct Bluetooth will use. + # Prefer a local HA Host adapter whenever one sees the shaver. now_mono = time.monotonic() scored: list[tuple[int, SelectOptionDict]] = [] for info in async_discovered_service_info(self.hass): @@ -1487,10 +1496,12 @@ async def async_step_user_bleak( # strip it to keep the label compact ("hci0" / "atom-lite (proxy)"). paths = describe_available_paths(self.hass, info.address) if paths: - best = paths[0] - via = str(best["name"]).split(" (")[0] + local = next((p for p in paths if p["is_local"]), None) + selected = local or paths[0] + via = str(selected["name"]).split(" (")[0] label_parts.append( - f"via {via}" + ("" if best["is_local"] else " (proxy)") + f"via {via}" + + ("" if selected["is_local"] else " (proxy)") ) label = label_parts[0] + ( " — " + ", ".join(label_parts[1:]) if len(label_parts) > 1 else "" diff --git a/custom_components/philips_shaver/transport.py b/custom_components/philips_shaver/transport.py index 4d42a22..34f5241 100644 --- a/custom_components/philips_shaver/transport.py +++ b/custom_components/philips_shaver/transport.py @@ -16,6 +16,11 @@ from bleak import BleakClient from bleak_retry_connector import establish_connection as bleak_establish +try: + from habluetooth.usage import ORIGINAL_BLEAK_CLIENT +except ImportError: # pragma: no cover - compatibility with the pinned old test stack + ORIGINAL_BLEAK_CLIENT = BleakClient + from homeassistant.components.bluetooth import ( HaScanner, async_last_service_info, @@ -74,6 +79,41 @@ def _host_scanner_name_by_adapter( return None +def local_bluez_scanner_device_from_address( + hass: HomeAssistant, address: str +): + """Return the strongest usable local HA Host scanner entry for address. + + Philips shaver bonds are controller-local. A stock ESPHome Bluetooth + proxy may advertise the same device into Home Assistant, but it cannot + complete the LE Secure Connections pairing required by the shaver. + """ + candidates = [] + try: + for scanner_device in async_scanner_devices_by_address( + hass, address, connectable=True + ): + if not isinstance(scanner_device.scanner, HaScanner): + continue + rssi = getattr(scanner_device.advertisement, "rssi", None) + if rssi is not None and rssi <= -127: + continue + rank = rssi if isinstance(rssi, (int, float)) else -999 + candidates.append((rank, scanner_device)) + except Exception: # noqa: BLE001 + return None + + if not candidates: + return None + return max(candidates, key=lambda item: item[0])[1] + + +def local_bluez_device_from_address(hass: HomeAssistant, address: str): + """Return a BLEDevice seen by a local HA Host/BlueZ scanner.""" + scanner_device = local_bluez_scanner_device_from_address(hass, address) + return scanner_device.ble_device if scanner_device is not None else None + + def describe_connection_path( hass: HomeAssistant, client: BleakClient, device ) -> str: @@ -426,9 +466,15 @@ def connection_rssi(self) -> int | None: return int(rssi) async def connect(self) -> None: - service_info = async_last_service_info(self._hass, self._address) - if not service_info: - raise TransportError(f"Device {self._address} not in range") + scanner_device = local_bluez_scanner_device_from_address( + self._hass, self._address + ) + if not scanner_device: + raise TransportError( + f"Device {self._address} is not reachable via a local " + "Bluetooth adapter" + ) + device = scanner_device.ble_device def _on_disconnect(_client): _LOGGER.info("%s: connection lost", self._address) @@ -439,15 +485,17 @@ def _on_disconnect(_client): self._disconnect_cb() self._client = await bleak_establish( - BleakClient, - service_info.device, + ORIGINAL_BLEAK_CLIENT, + device, "philips_shaver", disconnected_callback=_on_disconnect, timeout=15.0, ) - self._connected_scanner = getattr(self._client, "_connected_scanner", None) + # ORIGINAL_BLEAK_CLIENT intentionally bypasses HA's scanner re-routing, + # so keep the selected Host scanner ourselves for RSSI reporting. + self._connected_scanner = scanner_device.scanner self._connection_path = describe_connection_path( - self._hass, self._client, service_info.device + self._hass, self._client, device ) _LOGGER.info("%s: connected via %s", self._address, self._connection_path) @@ -477,15 +525,18 @@ async def read_char(self, char_uuid: str) -> bytes | None: async def read_chars(self, char_uuids: list[str]) -> dict[str, bytes | None]: """Connect-read-disconnect pattern for polling.""" results: dict[str, bytes | None] = {u: None for u in char_uuids} - service_info = async_last_service_info(self._hass, self._address) - if not service_info: - _LOGGER.warning("Device %s not in range", self._address) + device = local_bluez_device_from_address(self._hass, self._address) + if not device: + _LOGGER.warning( + "Device %s not reachable via a local Bluetooth adapter", + self._address, + ) return results client: BleakClient | None = None try: client = await bleak_establish( - BleakClient, service_info.device, "philips_shaver", timeout=15.0 + ORIGINAL_BLEAK_CLIENT, device, "philips_shaver", timeout=15.0 ) if not client or not client.is_connected: return results diff --git a/tests/test_direct_bluez_routing.py b/tests/test_direct_bluez_routing.py new file mode 100644 index 0000000..76e2aa2 --- /dev/null +++ b/tests/test_direct_bluez_routing.py @@ -0,0 +1,119 @@ +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock + +from homeassistant.data_entry_flow import FlowResultType + +from custom_components.philips_shaver import transport as tr +from custom_components.philips_shaver.config_flow import PhilipsShaverConfigFlow + +ADDRESS = "F4:B3:B1:AA:BB:CC" + + +def test_local_selector_ignores_stronger_proxy(monkeypatch) -> None: + class LocalScanner: + pass + + local_device = object() + local = SimpleNamespace( + scanner=LocalScanner(), + advertisement=SimpleNamespace(rssi=-82), + ble_device=local_device, + ) + proxy = SimpleNamespace( + scanner=SimpleNamespace(), + advertisement=SimpleNamespace(rssi=-42), + ble_device=object(), + ) + monkeypatch.setattr(tr, "HaScanner", LocalScanner) + monkeypatch.setattr( + tr, + "async_scanner_devices_by_address", + lambda hass, address, connectable=True: [proxy, local], + ) + + selected = tr.local_bluez_scanner_device_from_address( + SimpleNamespace(), ADDRESS + ) + + assert selected is local + assert tr.local_bluez_device_from_address(SimpleNamespace(), ADDRESS) is local_device + + +def test_direct_preview_prefers_local_over_stronger_proxy(monkeypatch) -> None: + flow = PhilipsShaverConfigFlow() + flow.flow_id = "test-flow" + flow.handler = "philips_shaver" + flow.discovery_info = SimpleNamespace(address=ADDRESS, name="Philips Shaver") + flow.hass = SimpleNamespace() + monkeypatch.setattr( + "custom_components.philips_shaver.config_flow.describe_available_paths", + MagicMock(return_value=[ + {"name": "aquarium-multisensor", "rssi": -42, "is_local": False}, + {"name": "hci0", "rssi": -82, "is_local": True}, + ]), + ) + + via, warning, _values = flow._transport_lines() + + assert via == " via **Direct Bluetooth** (hci0, -82 dBm)" + assert warning == "" + + +async def test_manual_selection_does_not_abort_for_discovery_flow(monkeypatch) -> None: + class FakeTask: + def done(self) -> bool: + return False + + flow = PhilipsShaverConfigFlow() + flow.flow_id = "test-flow" + flow.handler = "philips_shaver" + flow.discovery_info = None + + def create_task(coro, *args, **kwargs): + coro.close() + return FakeTask() + + flow.hass = SimpleNamespace(async_create_task=MagicMock(side_effect=create_task)) + flow.async_set_unique_id = AsyncMock() + flow._abort_if_already_configured = MagicMock() + monkeypatch.setattr( + "custom_components.philips_shaver.config_flow.describe_available_paths", + MagicMock(return_value=[{"name": "hci0", "rssi": -60, "is_local": True}]), + ) + monkeypatch.setattr( + "custom_components.philips_shaver.dbus_pairing.is_dbus_available", + lambda: False, + ) + + result = await flow.async_step_user_bleak({"address": ADDRESS}) + + assert result["type"] == FlowResultType.SHOW_PROGRESS + flow.async_set_unique_id.assert_awaited_once_with( + ADDRESS, raise_on_progress=False + ) + flow._abort_if_already_configured.assert_called_once_with() + + +async def test_runtime_connect_keeps_local_scanner_for_rssi(monkeypatch) -> None: + scanner = SimpleNamespace() + device = SimpleNamespace(address=ADDRESS, name="Philips QP4530") + scanner_device = SimpleNamespace(scanner=scanner, ble_device=device) + client = SimpleNamespace(is_connected=True) + establish = AsyncMock(return_value=client) + + monkeypatch.setattr( + tr, "local_bluez_scanner_device_from_address", + lambda hass, address: scanner_device, + ) + monkeypatch.setattr(tr, "bleak_establish", establish) + monkeypatch.setattr(tr, "describe_connection_path", lambda *args: "hci0") + + transport = tr.BleakTransport(SimpleNamespace(), ADDRESS) + await transport.connect() + + assert transport._connected_scanner is scanner + assert transport.connection_path == "hci0" + assert establish.await_args.args[0] is tr.ORIGINAL_BLEAK_CLIENT + assert establish.await_args.args[1] is device diff --git a/tests/test_transport_preview.py b/tests/test_transport_preview.py index 89f2ec3..cbe6c68 100644 --- a/tests/test_transport_preview.py +++ b/tests/test_transport_preview.py @@ -82,16 +82,16 @@ def test_proxy_via_and_hard_warning(monkeypatch) -> None: assert "ESP32 bridge" in warning -def test_proxy_preferred_with_local_fallback_hint(monkeypatch) -> None: +def test_local_direct_route_wins_over_stronger_proxy(monkeypatch) -> None: _patch_paths(monkeypatch, [ {"name": "atom-lite", "rssi": -64, "is_local": False}, {"name": "hci0 (00:0A:CD:46:B2:2D)", "rssi": -82, "is_local": True}, ]) via, variant, values = _flow()._transport_lines() warning = _warning(variant, values) - assert via == " via **Bluetooth proxy** (atom-lite, -64 dBm)" - assert "hci0" in warning - assert "strongest signal" in warning + assert via == " via **Direct Bluetooth** (hci0, -82 dBm)" + assert variant == "" + assert warning == "" def test_local_strongest_wins_over_weaker_proxy(monkeypatch) -> None: