Try the next best adapter after a BLE connection fails (#84512)

* Try the next best adapter after a BLE connection fails

* add cover

* tweak

* tweak

* Update homeassistant/components/bluetooth/wrappers.py

* bump

* small tweak

* tweak logic

homeassistant/components/bluetooth/manager.py

@@ -217,20 +217,19 @@ class BluetoothManager:
         uninstall_multiple_bleak_catcher()
 
     @hass_callback
-    def async_get_discovered_devices_and_advertisement_data_by_address(
+    def async_get_scanner_discovered_devices_and_advertisement_data_by_address(
         self, address: str, connectable: bool
-    ) -> list[tuple[BLEDevice, AdvertisementData]]:
+    ) -> list[tuple[BaseHaScanner, BLEDevice, AdvertisementData]]:
-        """Get devices and advertisement_data by address."""
+        """Get scanner, devices, and advertisement_data by address."""
         types_ = (True,) if connectable else (True, False)
-        return [
+        results: list[tuple[BaseHaScanner, BLEDevice, AdvertisementData]] = []
-            device_advertisement_data
+        for type_ in types_:
-            for device_advertisement_data in (
+            for scanner in self._get_scanners_by_type(type_):
-                scanner.discovered_devices_and_advertisement_data.get(address)
+                if device_advertisement_data := scanner.discovered_devices_and_advertisement_data.get(
-                for type_ in types_
+                    address
-                for scanner in self._get_scanners_by_type(type_)
+                ):
-            )
+                    results.append((scanner, *device_advertisement_data))
-            if device_advertisement_data is not None
+        return results
-        ]
 
     @hass_callback
     def _async_all_discovered_addresses(self, connectable: bool) -> Iterable[str]:

homeassistant/components/bluetooth/manifest.json

@@ -7,7 +7,7 @@
   "quality_scale": "internal",
   "requirements": [
     "bleak==0.19.2",
-    "bleak-retry-connector==2.12.1",
+    "bleak-retry-connector==2.13.0",
     "bluetooth-adapters==0.15.2",
     "bluetooth-auto-recovery==1.0.3",
     "bluetooth-data-tools==0.3.1",

homeassistant/components/bluetooth/wrappers.py

@@ -5,13 +5,18 @@ import asyncio
 from collections.abc import Callable
 import contextlib
 from dataclasses import dataclass
+from functools import partial
 import logging
 from typing import TYPE_CHECKING, Any, Final
 
 from bleak import BleakClient, BleakError
 from bleak.backends.client import BaseBleakClient, get_platform_client_backend_type
 from bleak.backends.device import BLEDevice
-from bleak.backends.scanner import AdvertisementDataCallback, BaseBleakScanner
+from bleak.backends.scanner import (
+    AdvertisementData,
+    AdvertisementDataCallback,
+    BaseBleakScanner,
+)
 from bleak_retry_connector import (
     NO_RSSI_VALUE,
     ble_device_description,
@@ -23,6 +28,7 @@ from homeassistant.core import CALLBACK_TYPE, callback as hass_callback
 from homeassistant.helpers.frame import report
 
 from . import models
+from .base_scanner import BaseHaScanner
 
 FILTER_UUIDS: Final = "UUIDs"
 _LOGGER = logging.getLogger(__name__)
@@ -37,6 +43,7 @@ class _HaWrappedBleakBackend:
     """Wrap bleak backend to make it usable by Home Assistant."""
 
     device: BLEDevice
+    scanner: BaseHaScanner
     client: type[BaseBleakClient]
     source: str | None
 
@@ -140,6 +147,33 @@ class HaBleakScannerWrapper(BaseBleakScanner):
                 asyncio.get_running_loop().call_soon_threadsafe(self._detection_cancel)
 
 
+def _rssi_sorter_with_connection_failure_penalty(
+    scanner_device_advertisement_data: tuple[
+        BaseHaScanner, BLEDevice, AdvertisementData
+    ],
+    connection_failure_count: dict[BaseHaScanner, int],
+    rssi_diff: int,
+) -> float:
+    """Get a sorted list of scanner, device, advertisement data adjusting for previous connection failures.
+
+    When a connection fails, we want to try the next best adapter so we
+    apply a penalty to the RSSI value to make it less likely to be chosen
+    for every previous connection failure.
+
+    We use the 51% of the RSSI difference between the first and second
+    best adapter as the penalty. This ensures we will always try the
+    best adapter twice before moving on to the next best adapter since
+    the first failure may be a transient service resolution issue.
+    """
+    scanner, _, advertisement_data = scanner_device_advertisement_data
+    base_rssi = advertisement_data.rssi or NO_RSSI_VALUE
+    if connect_failures := connection_failure_count.get(scanner):
+        if connect_failures > 1 and not rssi_diff:
+            rssi_diff = 1
+        return base_rssi - (rssi_diff * connect_failures * 0.51)
+    return base_rssi
+
+
 class HaBleakClientWrapper(BleakClient):
     """Wrap the BleakClient to ensure it does not shutdown our scanner.
 
@@ -171,6 +205,7 @@ class HaBleakClientWrapper(BleakClient):
             self.__address = address_or_ble_device
         self.__disconnected_callback = disconnected_callback
         self.__timeout = timeout
+        self.__connect_failures: dict[BaseHaScanner, int] = {}
         self._backend: BaseBleakClient | None = None  # type: ignore[assignment]
 
     @property
@@ -197,12 +232,13 @@ class HaBleakClientWrapper(BleakClient):
     async def connect(self, **kwargs: Any) -> bool:
         """Connect to the specified GATT server."""
         assert models.MANAGER is not None
-        wrapped_backend = self._async_get_best_available_backend_and_device()
+        manager = models.MANAGER
+        wrapped_backend = self._async_get_best_available_backend_and_device(manager)
         self._backend = wrapped_backend.client(
             wrapped_backend.device,
             disconnected_callback=self.__disconnected_callback,
             timeout=self.__timeout,
-            hass=models.MANAGER.hass,
+            hass=manager.hass,
         )
         if debug_logging := _LOGGER.isEnabledFor(logging.DEBUG):
             # Only lookup the description if we are going to log it
@@ -215,8 +251,12 @@ class HaBleakClientWrapper(BleakClient):
         finally:
             # If we failed to connect and its a local adapter (no source)
             # we release the connection slot
-            if not connected and not wrapped_backend.source:
+            if not connected:
-                models.MANAGER.async_release_connection_slot(wrapped_backend.device)
+                self.__connect_failures[wrapped_backend.scanner] = (
+                    self.__connect_failures.get(wrapped_backend.scanner, 0) + 1
+                )
+                if not wrapped_backend.source:
+                    manager.async_release_connection_slot(wrapped_backend.device)
 
         if debug_logging:
             _LOGGER.debug("%s: Connected (last rssi: %s)", description, rssi)
@@ -224,7 +264,7 @@ class HaBleakClientWrapper(BleakClient):
 
     @hass_callback
     def _async_get_backend_for_ble_device(
-        self, manager: BluetoothManager, ble_device: BLEDevice
+        self, manager: BluetoothManager, scanner: BaseHaScanner, ble_device: BLEDevice
     ) -> _HaWrappedBleakBackend | None:
         """Get the backend for a BLEDevice."""
         if not (source := device_source(ble_device)):
@@ -233,41 +273,68 @@ class HaBleakClientWrapper(BleakClient):
             if not manager.async_allocate_connection_slot(ble_device):
                 return None
             cls = get_platform_client_backend_type()
-            return _HaWrappedBleakBackend(ble_device, cls, source)
+            return _HaWrappedBleakBackend(ble_device, scanner, cls, source)
 
         # Make sure the backend can connect to the device
         # as some backends have connection limits
-        if (
+        if not scanner.connector or not scanner.connector.can_connect():
-            not (scanner := manager.async_scanner_by_source(source))
-            or not scanner.connector
-            or not scanner.connector.can_connect()
-        ):
             return None
 
-        return _HaWrappedBleakBackend(ble_device, scanner.connector.client, source)
+        return _HaWrappedBleakBackend(
+            ble_device, scanner, scanner.connector.client, source
+        )
 
     @hass_callback
     def _async_get_best_available_backend_and_device(
-        self,
+        self, manager: BluetoothManager
     ) -> _HaWrappedBleakBackend:
         """Get a best available backend and device for the given address.
 
         This method will return the backend with the best rssi
         that has a free connection slot.
         """
-        assert models.MANAGER is not None
         address = self.__address
-        device_advertisement_datas = models.MANAGER.async_get_discovered_devices_and_advertisement_data_by_address(
+        scanner_device_advertisement_datas = manager.async_get_scanner_discovered_devices_and_advertisement_data_by_address(
             address, True
         )
-        for device_advertisement_data in sorted(
+        sorted_scanner_device_advertisement_datas = sorted(
-            device_advertisement_datas,
+            scanner_device_advertisement_datas,
-            key=lambda device_advertisement_data: device_advertisement_data[1].rssi
+            key=lambda scanner_device_advertisement_data: scanner_device_advertisement_data[
+                2
+            ].rssi
             or NO_RSSI_VALUE,
             reverse=True,
+        )
+
+        # If we have connection failures we adjust the rssi sorting
+        # to prefer the adapter/scanner with the less failures so
+        # we don't keep trying to connect with an adapter
+        # that is failing
+        if (
+            self.__connect_failures
+            and len(sorted_scanner_device_advertisement_datas) > 1
         ):
+            # We use the rssi diff between to the top two
+            # to adjust the rssi sorter so that each failure
+            # will reduce the rssi sorter by the diff amount
+            rssi_diff = (
+                sorted_scanner_device_advertisement_datas[0][2].rssi
+                - sorted_scanner_device_advertisement_datas[1][2].rssi
+            )
+            adjusted_rssi_sorter = partial(
+                _rssi_sorter_with_connection_failure_penalty,
+                connection_failure_count=self.__connect_failures,
+                rssi_diff=rssi_diff,
+            )
+            sorted_scanner_device_advertisement_datas = sorted(
+                scanner_device_advertisement_datas,
+                key=adjusted_rssi_sorter,
+                reverse=True,
+            )
+
+        for (scanner, ble_device, _) in sorted_scanner_device_advertisement_datas:
             if backend := self._async_get_backend_for_ble_device(
-                models.MANAGER, device_advertisement_data[0]
+                manager, scanner, ble_device
             ):
                 return backend
 

homeassistant/package_constraints.txt

@@ -10,7 +10,7 @@ atomicwrites-homeassistant==1.4.1
 attrs==22.1.0
 awesomeversion==22.9.0
 bcrypt==3.1.7
-bleak-retry-connector==2.12.1
+bleak-retry-connector==2.13.0
 bleak==0.19.2
 bluetooth-adapters==0.15.2
 bluetooth-auto-recovery==1.0.3

requirements_all.txt

@@ -428,7 +428,7 @@ bimmer_connected==0.10.4
 bizkaibus==0.1.1
 
 # homeassistant.components.bluetooth
-bleak-retry-connector==2.12.1
+bleak-retry-connector==2.13.0
 
 # homeassistant.components.bluetooth
 bleak==0.19.2

requirements_test_all.txt

@@ -352,7 +352,7 @@ bellows==0.34.5
 bimmer_connected==0.10.4
 
 # homeassistant.components.bluetooth
-bleak-retry-connector==2.12.1
+bleak-retry-connector==2.13.0
 
 # homeassistant.components.bluetooth
 bleak==0.19.2

tests/components/bluetooth/test_wrappers.py

@@ -43,6 +43,10 @@ class FakeScanner(BaseHaRemoteScanner):
         )
         self._details: dict[str, str | HaBluetoothConnector] = {}
 
+    def __repr__(self) -> str:
+        """Return the representation."""
+        return f"FakeScanner({self.name})"
+
     def inject_advertisement(
         self, device: BLEDevice, advertisement_data: AdvertisementData
     ) -> None:
@@ -65,6 +69,7 @@ class BaseFakeBleakClient:
     def __init__(self, address_or_ble_device: Union[BLEDevice, str], **kwargs):
         """Initialize the fake bleak client."""
         self._device_path = "/dev/test"
+        self._device = address_or_ble_device
         self._address = address_or_ble_device.address
 
     async def disconnect(self, *args, **kwargs):
@@ -100,7 +105,7 @@ class FakeBleakClientRaisesOnConnect(BaseFakeBleakClient):
 
 
 def _generate_ble_device_and_adv_data(
-    interface: str, mac: str
+    interface: str, mac: str, rssi: int
 ) -> tuple[BLEDevice, AdvertisementData]:
     """Generate a BLE device with adv data."""
     return (
@@ -110,7 +115,7 @@ def _generate_ble_device_and_adv_data(
             delegate="",
             details={"path": f"/org/bluez/{interface}/dev_{mac}"},
         ),
-        generate_advertisement_data(),
+        generate_advertisement_data(rssi=rssi),
     )
 
 
@@ -158,13 +163,13 @@ def _generate_scanners_with_fake_devices(hass):
     hci0_device_advs = {}
     for i in range(10):
         device, adv_data = _generate_ble_device_and_adv_data(
-            "hci0", f"00:00:00:00:00:{i:02x}"
+            "hci0", f"00:00:00:00:00:{i:02x}", rssi=-60
         )
         hci0_device_advs[device.address] = (device, adv_data)
     hci1_device_advs = {}
     for i in range(10):
         device, adv_data = _generate_ble_device_and_adv_data(
-            "hci1", f"00:00:00:00:00:{i:02x}"
+            "hci1", f"00:00:00:00:00:{i:02x}", rssi=-80
         )
         hci1_device_advs[device.address] = (device, adv_data)
 
@@ -296,3 +301,63 @@ async def test_release_slot_on_connect_exception(
 
     cancel_hci0()
     cancel_hci1()
+
+
+async def test_we_switch_adapters_on_failure(
+    hass,
+    two_adapters,
+    enable_bluetooth,
+    install_bleak_catcher,
+):
+    """Ensure we try the next best adapter after a failure."""
+    hci0_device_advs, cancel_hci0, cancel_hci1 = _generate_scanners_with_fake_devices(
+        hass
+    )
+    ble_device = hci0_device_advs["00:00:00:00:00:01"][0]
+    client = bleak.BleakClient(ble_device)
+
+    class FakeBleakClientFailsHCI0Only(BaseFakeBleakClient):
+        """Fake bleak client that fails to connect."""
+
+        async def connect(self, *args, **kwargs):
+            """Connect."""
+            if "/hci0/" in self._device.details["path"]:
+                return False
+            return True
+
+    with patch(
+        "homeassistant.components.bluetooth.wrappers.get_platform_client_backend_type",
+        return_value=FakeBleakClientFailsHCI0Only,
+    ):
+        assert await client.connect() is False
+
+    with patch(
+        "homeassistant.components.bluetooth.wrappers.get_platform_client_backend_type",
+        return_value=FakeBleakClientFailsHCI0Only,
+    ):
+        assert await client.connect() is False
+
+    # After two tries we should switch to hci1
+    with patch(
+        "homeassistant.components.bluetooth.wrappers.get_platform_client_backend_type",
+        return_value=FakeBleakClientFailsHCI0Only,
+    ):
+        assert await client.connect() is True
+
+    # ..and we remember that hci1 works as long as the client doesn't change
+    with patch(
+        "homeassistant.components.bluetooth.wrappers.get_platform_client_backend_type",
+        return_value=FakeBleakClientFailsHCI0Only,
+    ):
+        assert await client.connect() is True
+
+    # If we replace the client, we should try hci0 again
+    client = bleak.BleakClient(ble_device)
+
+    with patch(
+        "homeassistant.components.bluetooth.wrappers.get_platform_client_backend_type",
+        return_value=FakeBleakClientFailsHCI0Only,
+    ):
+        assert await client.connect() is False
+    cancel_hci0()
+    cancel_hci1()