Fix bluetooth remote connections not picking the best path (#82957)

homeassistant/components/bluetooth/wrappers.py

@@ -12,7 +12,7 @@ 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_retry_connector import NO_RSSI_VALUE, freshen_ble_device
+from bleak_retry_connector import NO_RSSI_VALUE
 
 from homeassistant.core import CALLBACK_TYPE, callback as hass_callback
 from homeassistant.helpers.frame import report
@@ -182,19 +182,11 @@ class HaBleakClientWrapper(BleakClient):
 
     async def connect(self, **kwargs: Any) -> bool:
         """Connect to the specified GATT server."""
-        if (
-            not self._backend
-            or not self.__ble_device
-            or not self._async_get_backend_for_ble_device(self.__ble_device)
-        ):
         assert models.MANAGER is not None
-            wrapped_backend = (
+        (
-                self._async_get_backend() or self._async_get_fallback_backend()
+            wrapped_backend,
-            )
+            self.__ble_device,
-            self.__ble_device = (
+        ) = self._async_get_best_available_backend_and_device()
-                await freshen_ble_device(wrapped_backend.device)
-                or wrapped_backend.device
-            )
         self._backend = wrapped_backend.client(
             self.__ble_device,
             disconnected_callback=self.__disconnected_callback,
@@ -224,29 +216,14 @@ class HaBleakClientWrapper(BleakClient):
         return _HaWrappedBleakBackend(ble_device, connector.client)
 
     @hass_callback
-    def _async_get_backend(self) -> _HaWrappedBleakBackend | None:
+    def _async_get_best_available_backend_and_device(
-        """Get the bleak backend for the given address."""
+        self,
-        assert models.MANAGER is not None
+    ) -> tuple[_HaWrappedBleakBackend, BLEDevice]:
-        address = self.__address
+        """Get a best available backend and device for the given address.
-        ble_device = models.MANAGER.async_ble_device_from_address(address, True)
-        if ble_device is None:
-            raise BleakError(f"No device found for address {address}")
 
-        if backend := self._async_get_backend_for_ble_device(ble_device):
+        This method will return the backend with the best rssi
-            return backend
+        that has a free connection slot.
-
+        """
-        return None
-
-    @hass_callback
-    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.
-        #
-        # We need to try all backends to find one that can
-        # connect to the device.
-        #
         assert models.MANAGER is not None
         address = self.__address
         device_advertisement_datas = models.MANAGER.async_get_discovered_devices_and_advertisement_data_by_address(
@@ -258,10 +235,9 @@ class HaBleakClientWrapper(BleakClient):
             or NO_RSSI_VALUE,
             reverse=True,
         ):
-            if backend := self._async_get_backend_for_ble_device(
+            ble_device = device_advertisement_data[0]
-                device_advertisement_data[0]
+            if backend := self._async_get_backend_for_ble_device(ble_device):
-            ):
+                return backend, ble_device
-                return backend
 
         raise BleakError(
             f"No backend with an available connection slot that can reach address {address} was found"

tests/components/bluetooth/test_models.py

@@ -60,22 +60,73 @@ async def test_wrapped_bleak_client_set_disconnected_callback_after_connected(
     hass, enable_bluetooth, one_adapter
 ):
     """Test wrapped bleak client can set a disconnected callback after connected."""
+    manager = _get_manager()
+
+    switchbot_proxy_device_has_connection_slot = BLEDevice(
+        "44:44:33:11:23:45",
+        "wohand",
+        {
+            "connector": HaBluetoothConnector(
+                MockBleakClient, "mock_bleak_client", lambda: True
+            ),
+            "path": "/org/bluez/hci0/dev_44_44_33_11_23_45",
+        },
+        rssi=-40,
+    )
+    switchbot_proxy_device_adv_has_connection_slot = generate_advertisement_data(
+        local_name="wohand",
+        service_uuids=[],
+        manufacturer_data={1: b"\x01"},
+        rssi=-40,
+    )
     switchbot_device = BLEDevice(
-        "44:44:33:11:23:45", "wohand", {"path": "/org/bluez/hci0/dev_44_44_33_11_23_45"}
+        "44:44:33:11:23:45",
+        "wohand",
+        {"path": "/org/bluez/hci0/dev_44_44_33_11_23_45"},
     )
     switchbot_adv = generate_advertisement_data(
-        local_name="wohand", service_uuids=[], manufacturer_data={1: b"\x01"}
+        local_name="wohand", service_uuids=[], manufacturer_data={1: b"\x01"}, rssi=-100
     )
-    inject_advertisement(hass, switchbot_device, switchbot_adv)
+
-    client = HaBleakClientWrapper(switchbot_device)
+    inject_advertisement_with_source(
-    with patch(
+        hass, switchbot_device, switchbot_adv, "00:00:00:00:00:01"
-        "bleak.backends.bluezdbus.client.BleakClientBlueZDBus.connect"
+    )
-    ) as connect:
+    inject_advertisement_with_source(
+        hass,
+        switchbot_proxy_device_has_connection_slot,
+        switchbot_proxy_device_adv_has_connection_slot,
+        "esp32_has_connection_slot",
+    )
+
+    class FakeScanner(BaseHaScanner):
+        @property
+        def discovered_devices_and_advertisement_data(
+            self,
+        ) -> dict[str, tuple[BLEDevice, AdvertisementData]]:
+            """Return a list of discovered devices."""
+            return {
+                switchbot_proxy_device_has_connection_slot.address: (
+                    switchbot_proxy_device_has_connection_slot,
+                    switchbot_proxy_device_adv_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(hass, "esp32", "esp32")
+    cancel = manager.async_register_scanner(scanner, True)
+
+    client = HaBleakClientWrapper(switchbot_proxy_device_has_connection_slot)
+    with patch("bleak.backends.bluezdbus.client.BleakClientBlueZDBus.connect"):
         await client.connect()
-    assert len(connect.mock_calls) == 1
+    assert client.is_connected is True
-    assert client._backend is not None
     client.set_disconnected_callback(lambda client: None)
     await client.disconnect()
+    cancel()
 
 
 async def test_ble_device_with_proxy_client_out_of_connections(

tests/components/keymitt_ble/test_config_flow.py

@@ -1,6 +1,6 @@
 """Test the MicroBot config flow."""
 
-from unittest.mock import ANY, patch
+from unittest.mock import ANY, AsyncMock, patch
 
 from homeassistant.config_entries import SOURCE_BLUETOOTH, SOURCE_USER
 from homeassistant.const import CONF_ACCESS_TOKEN, CONF_ADDRESS
@@ -9,7 +9,6 @@ from homeassistant.data_entry_flow import FlowResultType
 from . import (
     SERVICE_INFO,
     USER_INPUT,
-    MockMicroBotApiClient,
     MockMicroBotApiClientFail,
     patch_async_setup_entry,
 )
@@ -19,6 +18,13 @@ from tests.common import MockConfigEntry
 DOMAIN = "keymitt_ble"
 
 
+def patch_microbot_api():
+    """Patch MicroBot API."""
+    return patch(
+        "homeassistant.components.keymitt_ble.config_flow.MicroBotApiClient", AsyncMock
+    )
+
+
 async def test_bluetooth_discovery(hass):
     """Test discovery via bluetooth with a valid device."""
     result = await hass.config_entries.flow.async_init(
@@ -29,7 +35,7 @@ async def test_bluetooth_discovery(hass):
     assert result["type"] == FlowResultType.FORM
     assert result["step_id"] == "init"
 
-    with patch_async_setup_entry() as mock_setup_entry:
+    with patch_async_setup_entry() as mock_setup_entry, patch_microbot_api():
         result = await hass.config_entries.flow.async_configure(
             result["flow_id"],
             USER_INPUT,
@@ -51,6 +57,7 @@ async def test_bluetooth_discovery_already_setup(hass):
         unique_id="aa:bb:cc:dd:ee:ff",
     )
     entry.add_to_hass(hass)
+    with patch_microbot_api():
         result = await hass.config_entries.flow.async_init(
             DOMAIN,
             context={"source": SOURCE_BLUETOOTH},
@@ -74,19 +81,18 @@ async def test_user_setup(hass):
     assert result["step_id"] == "init"
     assert result["errors"] == {}
 
+    with patch_microbot_api():
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
             USER_INPUT,
         )
+        await hass.async_block_till_done()
 
     assert result2["type"] == FlowResultType.FORM
     assert result2["step_id"] == "link"
     assert result2["errors"] is None
 
-    with patch(
+    with patch_microbot_api(), patch_async_setup_entry() as mock_setup_entry:
-        "homeassistant.components.keymitt_ble.config_flow.MicroBotApiClient",
-        MockMicroBotApiClient,
-    ), patch_async_setup_entry() as mock_setup_entry:
         result3 = await hass.config_entries.flow.async_configure(
             result2["flow_id"],
             USER_INPUT,
@@ -124,7 +130,7 @@ async def test_user_setup_already_configured(hass):
 
 async def test_user_no_devices(hass):
     """Test the user initiated form with valid mac."""
-    with patch(
+    with patch_microbot_api(), patch(
         "homeassistant.components.keymitt_ble.config_flow.async_discovered_service_info",
         return_value=[],
     ):
@@ -138,7 +144,7 @@ async def test_user_no_devices(hass):
 async def test_no_link(hass):
     """Test the user initiated form with invalid response."""
 
-    with patch(
+    with patch_microbot_api(), patch(
         "homeassistant.components.keymitt_ble.config_flow.async_discovered_service_info",
         return_value=[SERVICE_INFO],
     ):
@@ -149,6 +155,7 @@ async def test_no_link(hass):
     assert result["step_id"] == "init"
     assert result["errors"] == {}
 
+    with patch_microbot_api():
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
             USER_INPUT,