From 75f6f9b5e200ad44f1564786b40d9382dda21e08 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 26 Sep 2022 03:12:08 -1000 Subject: [PATCH] Improve performance of Bluetooth device fallback (#79078) --- homeassistant/components/bluetooth/manager.py | 17 ++++ .../components/bluetooth/manifest.json | 6 +- homeassistant/components/bluetooth/models.py | 27 +++--- homeassistant/components/bluetooth/scanner.py | 11 +++ homeassistant/components/bluetooth/usage.py | 2 +- homeassistant/components/esphome/bluetooth.py | 5 + homeassistant/package_constraints.txt | 6 +- requirements_all.txt | 6 +- requirements_test_all.txt | 6 +- tests/components/bluetooth/test_models.py | 93 +++++++++++++++++++ 10 files changed, 155 insertions(+), 24 deletions(-) diff --git a/homeassistant/components/bluetooth/manager.py b/homeassistant/components/bluetooth/manager.py index a7d1e141b3d..4229b285939 100644 --- a/homeassistant/components/bluetooth/manager.py +++ b/homeassistant/components/bluetooth/manager.py @@ -235,6 +235,23 @@ class BluetoothManager: self._cancel_unavailable_tracking.clear() uninstall_multiple_bleak_catcher() + async def async_get_devices_by_address( + self, address: str, connectable: bool + ) -> list[BLEDevice]: + """Get devices by address.""" + types_ = (True,) if connectable else (True, False) + return [ + device + for device in await asyncio.gather( + *( + scanner.async_get_device_by_address(address) + for type_ in types_ + for scanner in self._get_scanners_by_type(type_) + ) + ) + if device is not None + ] + @hass_callback def async_all_discovered_devices(self, connectable: bool) -> Iterable[BLEDevice]: """Return all of discovered devices from all the scanners including duplicates.""" diff --git a/homeassistant/components/bluetooth/manifest.json b/homeassistant/components/bluetooth/manifest.json index 636d5925651..91b15583086 100644 --- a/homeassistant/components/bluetooth/manifest.json +++ b/homeassistant/components/bluetooth/manifest.json @@ -6,9 +6,9 @@ "after_dependencies": ["hassio"], "quality_scale": "internal", "requirements": [ - "bleak==0.18.0", - "bleak-retry-connector==2.0.2", - "bluetooth-adapters==0.5.1", + "bleak==0.18.1", + "bleak-retry-connector==2.1.0", + "bluetooth-adapters==0.5.2", "bluetooth-auto-recovery==0.3.3", "dbus-fast==1.14.0" ], diff --git a/homeassistant/components/bluetooth/models.py b/homeassistant/components/bluetooth/models.py index 100d9f69c03..d93f8efc1e2 100644 --- a/homeassistant/components/bluetooth/models.py +++ b/homeassistant/components/bluetooth/models.py @@ -91,6 +91,10 @@ class BaseHaScanner: def discovered_devices(self) -> list[BLEDevice]: """Return a list of discovered devices.""" + @abstractmethod + async def async_get_device_by_address(self, address: str) -> BLEDevice | None: + """Get a device by address.""" + async def async_diagnostics(self) -> dict[str, Any]: """Return diagnostic information about the scanner.""" return { @@ -256,7 +260,9 @@ class HaBleakClientWrapper(BleakClient): async def connect(self, **kwargs: Any) -> bool: """Connect to the specified GATT server.""" if not self._backend: - wrapped_backend = self._async_get_backend() + wrapped_backend = ( + self._async_get_backend() or await self._async_get_fallback_backend() + ) self._backend = wrapped_backend.client( await freshen_ble_device(wrapped_backend.device) or wrapped_backend.device, @@ -286,7 +292,7 @@ class HaBleakClientWrapper(BleakClient): return _HaWrappedBleakBackend(ble_device, connector.client) @hass_callback - def _async_get_backend(self) -> _HaWrappedBleakBackend: + def _async_get_backend(self) -> _HaWrappedBleakBackend | None: """Get the bleak backend for the given address.""" assert MANAGER is not None address = self.__address @@ -297,6 +303,10 @@ class HaBleakClientWrapper(BleakClient): if backend := self._async_get_backend_for_ble_device(ble_device): return backend + return None + + async def _async_get_fallback_backend(self) -> _HaWrappedBleakBackend: + """Get a fallback backend for the given address.""" # # The preferred backend cannot currently connect the device # because it is likely out of connection slots. @@ -304,16 +314,11 @@ class HaBleakClientWrapper(BleakClient): # We need to try all backends to find one that can # connect to the device. # - # Currently we have to search all the discovered devices - # because the bleak API does not allow us to get the - # details for a specific device. - # + assert MANAGER is not None + address = self.__address + devices = await MANAGER.async_get_devices_by_address(address, True) for ble_device in sorted( - ( - ble_device - for ble_device in MANAGER.async_all_discovered_devices(True) - if ble_device.address == address - ), + devices, key=lambda ble_device: ble_device.rssi or NO_RSSI_VALUE, reverse=True, ): diff --git a/homeassistant/components/bluetooth/scanner.py b/homeassistant/components/bluetooth/scanner.py index 3f089326eb2..857a0e4c01c 100644 --- a/homeassistant/components/bluetooth/scanner.py +++ b/homeassistant/components/bluetooth/scanner.py @@ -17,6 +17,7 @@ from bleak.backends.bluezdbus.advertisement_monitor import OrPattern from bleak.backends.bluezdbus.scanner import BlueZScannerArgs from bleak.backends.device import BLEDevice from bleak.backends.scanner import AdvertisementData +from bleak_retry_connector import get_device_by_adapter from dbus_fast import InvalidMessageError from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback as hass_callback @@ -140,6 +141,16 @@ class HaScanner(BaseHaScanner): """Return a list of discovered devices.""" return self.scanner.discovered_devices + async def async_get_device_by_address(self, address: str) -> BLEDevice | None: + """Get a device by address.""" + if platform.system() == "Linux": + return await get_device_by_adapter(address, self.adapter) + # We don't have a fast version of this for MacOS yet + return next( + (device for device in self.discovered_devices if device.address == address), + None, + ) + async def async_diagnostics(self) -> dict[str, Any]: """Return diagnostic information about the scanner.""" base_diag = await super().async_diagnostics() diff --git a/homeassistant/components/bluetooth/usage.py b/homeassistant/components/bluetooth/usage.py index 23388c84302..d282ca7415b 100644 --- a/homeassistant/components/bluetooth/usage.py +++ b/homeassistant/components/bluetooth/usage.py @@ -13,7 +13,7 @@ ORIGINAL_BLEAK_CLIENT = bleak.BleakClient def install_multiple_bleak_catcher() -> None: """Wrap the bleak classes to return the shared instance if multiple instances are detected.""" bleak.BleakScanner = HaBleakScannerWrapper # type: ignore[misc, assignment] - bleak.BleakClient = HaBleakClientWrapper # type: ignore[misc, assignment] + bleak.BleakClient = HaBleakClientWrapper # type: ignore[misc] def uninstall_multiple_bleak_catcher() -> None: diff --git a/homeassistant/components/esphome/bluetooth.py b/homeassistant/components/esphome/bluetooth.py index 94351293c7b..71dfc798ffd 100644 --- a/homeassistant/components/esphome/bluetooth.py +++ b/homeassistant/components/esphome/bluetooth.py @@ -1,4 +1,5 @@ """Bluetooth scanner for esphome.""" +from __future__ import annotations from collections.abc import Callable import datetime @@ -77,6 +78,10 @@ class ESPHomeScannner(BaseHaScanner): """Return a list of discovered devices.""" return list(self._discovered_devices.values()) + async def async_get_device_by_address(self, address: str) -> BLEDevice | None: + """Get a device by address.""" + return self._discovered_devices.get(address) + @callback def async_on_advertisement(self, adv: BluetoothLEAdvertisement) -> None: """Call the registered callback.""" diff --git a/homeassistant/package_constraints.txt b/homeassistant/package_constraints.txt index c2e38e09345..d0dc41935db 100644 --- a/homeassistant/package_constraints.txt +++ b/homeassistant/package_constraints.txt @@ -10,9 +10,9 @@ atomicwrites-homeassistant==1.4.1 attrs==21.2.0 awesomeversion==22.9.0 bcrypt==3.1.7 -bleak-retry-connector==2.0.2 -bleak==0.18.0 -bluetooth-adapters==0.5.1 +bleak-retry-connector==2.1.0 +bleak==0.18.1 +bluetooth-adapters==0.5.2 bluetooth-auto-recovery==0.3.3 certifi>=2021.5.30 ciso8601==2.2.0 diff --git a/requirements_all.txt b/requirements_all.txt index 3c24488e1b2..92c69e28ad3 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -410,10 +410,10 @@ bimmer_connected==0.10.4 bizkaibus==0.1.1 # homeassistant.components.bluetooth -bleak-retry-connector==2.0.2 +bleak-retry-connector==2.1.0 # homeassistant.components.bluetooth -bleak==0.18.0 +bleak==0.18.1 # homeassistant.components.blebox blebox_uniapi==2.0.2 @@ -435,7 +435,7 @@ bluemaestro-ble==0.2.0 # bluepy==1.3.0 # homeassistant.components.bluetooth -bluetooth-adapters==0.5.1 +bluetooth-adapters==0.5.2 # homeassistant.components.bluetooth bluetooth-auto-recovery==0.3.3 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 48c064c744e..84bf3a4451e 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -331,10 +331,10 @@ bellows==0.33.1 bimmer_connected==0.10.4 # homeassistant.components.bluetooth -bleak-retry-connector==2.0.2 +bleak-retry-connector==2.1.0 # homeassistant.components.bluetooth -bleak==0.18.0 +bleak==0.18.1 # homeassistant.components.blebox blebox_uniapi==2.0.2 @@ -346,7 +346,7 @@ blinkpy==0.19.2 bluemaestro-ble==0.2.0 # homeassistant.components.bluetooth -bluetooth-adapters==0.5.1 +bluetooth-adapters==0.5.2 # homeassistant.components.bluetooth bluetooth-auto-recovery==0.3.3 diff --git a/tests/components/bluetooth/test_models.py b/tests/components/bluetooth/test_models.py index 2321a64e1e3..d126dcac301 100644 --- a/tests/components/bluetooth/test_models.py +++ b/tests/components/bluetooth/test_models.py @@ -1,4 +1,5 @@ """Tests for the Bluetooth integration models.""" +from __future__ import annotations from unittest.mock import patch @@ -197,6 +198,12 @@ async def test_ble_device_with_proxy_client_out_of_connections_uses_best_availab """Return a list of discovered devices.""" return [switchbot_proxy_device_has_connection_slot] + async def async_get_device_by_address(self, address: str) -> BLEDevice | None: + """Return a list of discovered devices.""" + if address == switchbot_proxy_device_has_connection_slot.address: + return switchbot_proxy_device_has_connection_slot + return None + scanner = FakeScanner() cancel = manager.async_register_scanner(scanner, True) assert manager.async_discovered_devices(True) == [ @@ -210,3 +217,89 @@ async def test_ble_device_with_proxy_client_out_of_connections_uses_best_availab client.set_disconnected_callback(lambda client: None) await client.disconnect() cancel() + + +async def test_ble_device_with_proxy_client_out_of_connections_uses_best_available_macos( + hass, enable_bluetooth, macos_adapter +): + """Test we switch to the next available proxy when one runs out of connections on MacOS.""" + manager = _get_manager() + + switchbot_proxy_device_no_connection_slot = BLEDevice( + "44:44:33:11:23:45", + "wohand_no_connection_slot", + { + "connector": HaBluetoothConnector( + MockBleakClient, "mock_bleak_client", lambda: False + ), + "path": "/org/bluez/hci0/dev_44_44_33_11_23_45", + }, + rssi=-30, + ) + switchbot_proxy_device_no_connection_slot.metadata["delegate"] = 0 + + switchbot_proxy_device_has_connection_slot = BLEDevice( + "44:44:33:11:23:45", + "wohand_has_connection_slot", + { + "connector": HaBluetoothConnector( + MockBleakClient, "mock_bleak_client", lambda: True + ), + "path": "/org/bluez/hci0/dev_44_44_33_11_23_45", + }, + rssi=-40, + ) + switchbot_proxy_device_has_connection_slot.metadata["delegate"] = 0 + + switchbot_device = BLEDevice( + "44:44:33:11:23:45", + "wohand", + {}, + rssi=-100, + ) + switchbot_device.metadata["delegate"] = 0 + switchbot_adv = AdvertisementData( + local_name="wohand", service_uuids=[], manufacturer_data={1: b"\x01"} + ) + + inject_advertisement_with_source( + hass, switchbot_device, switchbot_adv, "00:00:00:00:00:01" + ) + inject_advertisement_with_source( + hass, + switchbot_proxy_device_has_connection_slot, + switchbot_adv, + "esp32_has_connection_slot", + ) + inject_advertisement_with_source( + hass, + switchbot_proxy_device_no_connection_slot, + switchbot_adv, + "esp32_no_connection_slot", + ) + + class FakeScanner(BaseHaScanner): + @property + def discovered_devices(self) -> list[BLEDevice]: + """Return a list of discovered devices.""" + return [switchbot_proxy_device_has_connection_slot] + + async def async_get_device_by_address(self, address: str) -> BLEDevice | None: + """Return a list of discovered devices.""" + if address == switchbot_proxy_device_has_connection_slot.address: + return switchbot_proxy_device_has_connection_slot + return None + + scanner = FakeScanner() + cancel = manager.async_register_scanner(scanner, True) + assert manager.async_discovered_devices(True) == [ + switchbot_proxy_device_no_connection_slot + ] + + client = HaBleakClientWrapper(switchbot_proxy_device_no_connection_slot) + with patch("bleak.get_platform_client_backend_type"): + await client.connect() + assert client.is_connected is True + client.set_disconnected_callback(lambda client: None) + await client.disconnect() + cancel()