Skip to content

Commit

Permalink
feat: remove freshen fallback logic since Home Assistant always provi…
Browse files Browse the repository at this point in the history 
…des us the best path to the device now (#83)
  • Loading branch information
@bdraco
bdraco authored Dec 23, 2022
1 parent 8d49ed1 commit 0954d2d
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 214 deletions.
64 changes: 9 additions & 55 deletions src/bleak_retry_connector/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
_get_properties,
clear_cache,
device_source,
get_bluez_device,
get_connected_devices,
get_device,
get_device_by_adapter,
Expand Down Expand Up @@ -217,24 +216,6 @@ def ble_device_description(device: BLEDevice) -> str:
return base_name


async def freshen_ble_device(device: BLEDevice) -> BLEDevice | None:
"""Freshen the device.
There may be a better path to the device on another adapter
that was seen after the code that provided the BLEDevice to
the establish_connection function was run.
"""
if (
not IS_LINUX
or not isinstance(device.details, dict)
or "path" not in device.details
):
return None
return await get_bluez_device(
device.name or device.address, device.details["path"], _get_rssi(device)
)


def calculate_backoff_time(exc: Exception) -> float:
"""Calculate the backoff time based on the exception."""

Expand Down Expand Up @@ -266,7 +247,8 @@ def calculate_backoff_time(exc: Exception) -> float:

async def _disconnect_devices(devices: list[BLEDevice]) -> None:
"""Disconnect the devices."""
await disconnect_devices(devices)
if IS_LINUX:
await disconnect_devices(devices)


async def close_stale_connections(
Expand Down Expand Up @@ -296,13 +278,6 @@ async def close_stale_connections(
await _disconnect_devices(to_disconnect)


def _get_rssi(device: BLEDevice) -> int:
"""Get the RSSI for the device."""
if not isinstance(device.details, dict) or "props" not in device.details:
return device.rssi
return device.details["props"].get("RSSI") or device.rssi or NO_RSSI_VALUE


async def establish_connection(
client_class: type[BleakClient],
device: BLEDevice,
Expand Down Expand Up @@ -342,26 +317,18 @@ def _raise_if_needed(name: str, description: str, exc: Exception) -> None:
raise BleakNotFoundError(f"{msg}: {DEVICE_MISSING_ADVICE}") from exc
raise BleakConnectionError(msg) from exc

create_client = True
debug_enabled = _LOGGER.isEnabledFor(logging.DEBUG)
rssi: int | None = None
if IS_LINUX and (devices := await get_connected_devices(device)):
# Bleak 0.17 will handle already connected devices for us so
# if we are already connected we swap the device to the connected
# device.
device = devices[0]

client = client_class(device, disconnected_callback=disconnected_callback, **kwargs)

while True:
attempt += 1
original_device = device

# Its possible the BLEDevice can change between
# between connection attempts so we do not want
# to keep trying to connect to the old one if it has changed.
if ble_device_callback is not None:
device = ble_device_callback()

if fresh_device := await freshen_ble_device(device):
device = fresh_device

if not create_client:
create_client = ble_device_has_changed(original_device, device)

if debug_enabled:
_LOGGER.debug(
"%s - %s: Connection attempt: %s",
Expand All @@ -370,19 +337,6 @@ def _raise_if_needed(name: str, description: str, exc: Exception) -> None:
attempt,
)

if create_client:
client = client_class(
device, disconnected_callback=disconnected_callback, **kwargs
)
create_client = False

if IS_LINUX:
# Bleak 0.17 will handle already connected devices for us, but
# we still need to disconnect if its unexpectedly connected to another
# adapter.

await close_stale_connections(device, only_other_adapters=True)

try:
async with async_timeout.timeout(BLEAK_SAFETY_TIMEOUT):
await client.connect(
Expand Down
2 changes: 1 addition & 1 deletion tests/conftest.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def configure_test_logging(caplog):
def mock_linux():
with patch.object(bleak_retry_connector, "IS_LINUX", True), patch.object(
bleak_retry_connector.bluez, "IS_LINUX", True
):
), patch("bleak.backends.scanner.platform.system", return_value="Linux"):
yield


Expand Down
160 changes: 2 additions & 158 deletions tests/test_init.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -607,154 +607,6 @@ def test_ble_device_has_changed():
)


@pytest.mark.asyncio
async def test_establish_connection_ble_device_changed():
"""Test we switch BLEDevice when the underlying device has changed."""

attempts = 0

ble_device_1 = BLEDevice("aa:bb:cc:dd:ee:ff", "name", {"path": "/dev/1"})
ble_device_2 = BLEDevice("aa:bb:cc:dd:ee:ff", "name", {"path": "/dev/2"})
ble_device_3 = BLEDevice("aa:bb:cc:dd:ee:ff", "name", {"path": "/dev/3"})

def _get_ble_device():
nonlocal attempts

if attempts == 0:
return ble_device_1
if attempts == 1:
return ble_device_2
return ble_device_3

class FakeBleakClient(BleakClient):
def __init__(self, ble_device_or_address, *args, **kwargs):
self.ble_device_or_address = ble_device_or_address
pass

async def connect(self, *args, **kwargs):
nonlocal attempts
attempts += 1
if self.ble_device_or_address != ble_device_3:
raise BleakError("le-connection-abort-by-local")
pass

async def disconnect(self, *args, **kwargs):
pass

with patch("bleak_retry_connector.calculate_backoff_time", return_value=0):
client = await establish_connection(
FakeBleakClient, ble_device_1, "test", ble_device_callback=_get_ble_device
)
assert isinstance(client, FakeBleakClient)
assert attempts == 3


@pytest.mark.asyncio
async def test_establish_connection_better_rssi_available(mock_linux):

device: BLEDevice | None = None

class FakeBleakClient(BleakClient):
def __init__(self, ble_device_or_address, *args, **kwargs):
ble_device_or_address.metadata["delegate"] = 0
super().__init__(ble_device_or_address, *args, **kwargs)
nonlocal device
device = ble_device_or_address
self._device_path = "/org/bluez/hci2/dev_FA_23_9D_AA_45_46"

async def connect(self, *args, **kwargs):
return True

async def disconnect(self, *args, **kwargs):
pass

async def get_services(self, *args, **kwargs):
return []

class FakeBleakClientWithServiceCache(BleakClientWithServiceCache, FakeBleakClient):
"""Fake BleakClientWithServiceCache."""

async def get_services(self, *args, **kwargs):
return []

collection = BleakGATTServiceCollection()

class FakeBluezManager:
def __init__(self):
self._properties = {
"/org/bluez/hci0/dev_FA_23_9D_AA_45_46": {
"UUID": "service",
"Primary": True,
"Characteristics": [],
defs.DEVICE_INTERFACE: {
"Address": "FA:23:9D:AA:45:46",
"Alias": "FA:23:9D:AA:45:46",
"RSSI": -30,
},
defs.GATT_SERVICE_INTERFACE: True,
},
"/org/bluez/hci1/dev_FA_23_9D_AA_45_46": {
"UUID": "service",
"Primary": True,
"Characteristics": [],
defs.DEVICE_INTERFACE: {
"Address": "FA:23:9D:AA:45:46",
"Alias": "FA:23:9D:AA:45:46",
"RSSI": -79,
},
defs.GATT_SERVICE_INTERFACE: True,
},
"/org/bluez/hci2/dev_FA_23_9D_AA_45_46": {
"UUID": "service",
"Primary": True,
"Characteristics": [],
defs.DEVICE_INTERFACE: {
"Address": "FA:23:9D:AA:45:46",
"Alias": "FA:23:9D:AA:45:46",
"RSSI": -80,
},
defs.GATT_SERVICE_INTERFACE: True,
},
"/org/bluez/hci3/dev_FA_23_9D_AA_45_46": {
"UUID": "service",
"Primary": True,
"Characteristics": [],
defs.DEVICE_INTERFACE: {
"Address": "FA:23:9D:AA:45:46",
"Alias": "FA:23:9D:AA:45:46",
"RSSI": -31,
},
defs.GATT_SERVICE_INTERFACE: True,
},
}

bleak_retry_connector.bluez.get_global_bluez_manager = AsyncMock(
return_value=FakeBluezManager()
)
bleak_retry_connector.bluez.defs = defs

with patch("bleak.get_platform_client_backend_type"):

client = await establish_connection(
FakeBleakClientWithServiceCache,
BLEDevice(
"aa:bb:cc:dd:ee:ff",
"name",
{"path": "/org/bluez/hci2/dev_FA_23_9D_AA_45_46"},
-80,
delegate=False,
),
"test",
disconnected_callback=MagicMock(),
cached_services=collection,
)

assert isinstance(client, FakeBleakClientWithServiceCache)
await client.get_services() is collection
assert device is not None
assert device.details["path"] == "/org/bluez/hci0/dev_FA_23_9D_AA_45_46"


@pytest.mark.asyncio
async def test_establish_connection_other_adapter_already_connected(mock_linux):

Expand Down Expand Up @@ -1349,11 +1201,7 @@ def __init__(self):
await client.get_services() is collection
assert device is not None
assert device.details["path"] == "/org/bluez/hci1/dev_FA_23_9D_AA_45_46"
assert len(mock_disconnect_device.mock_calls) == 1
assert (
mock_disconnect_device.mock_calls[0][1][0][0].details["path"]
== "/org/bluez/hci2/dev_FA_23_9D_AA_45_46"
)
assert not mock_disconnect_device.mock_calls


@pytest.mark.asyncio
Expand Down Expand Up @@ -1479,11 +1327,7 @@ def __init__(self):
await client.get_services() is collection
assert device is not None
assert device.details["path"] == "/org/bluez/hci1/dev_FA_23_9D_AA_45_46"
assert len(mock_disconnect_device.mock_calls) == 1
assert (
mock_disconnect_device.mock_calls[0][1][0][0].details["path"]
== "/org/bluez/hci2/dev_FA_23_9D_AA_45_46"
)
assert not mock_disconnect_device.mock_calls


@pytest.mark.asyncio
Expand Down

0 comments on commit 0954d2d

Please sign in to comment.