diff --git a/bleak/backends/bluezdbus/adapter.py b/bleak/backends/bluezdbus/adapter.py index 001c9714c..db65a6eac 100644 --- a/bleak/backends/bluezdbus/adapter.py +++ b/bleak/backends/bluezdbus/adapter.py @@ -48,6 +48,14 @@ async def get_connected_devices( if props["Alias"] == props["Address"].replace(":", "-") else props["Alias"] ) - devices.append(BLEDevice(address, name, {"path": path, "props": props})) + # "from_connected_devices" marker: BleakClientBlueZDBus uses it + # to skip the BlueZ Disconnect call when this client is closed. + devices.append( + BLEDevice( + address, + name, + {"path": path, "props": props, "from_connected_devices": True}, + ) + ) return devices diff --git a/bleak/backends/bluezdbus/client.py b/bleak/backends/bluezdbus/client.py index c4a699d1a..5fe972459 100644 --- a/bleak/backends/bluezdbus/client.py +++ b/bleak/backends/bluezdbus/client.py @@ -79,9 +79,16 @@ def __init__( if isinstance(address_or_ble_device, BLEDevice): self._device_path = address_or_ble_device.details["path"] self._device_info = address_or_ble_device.details.get("props") + # Set by BleakAdapter.get_connected_devices(). When True, the + # user opted into an existing OS-level connection and + # disconnect() must not tear it down. + self._from_connected_devices = bool( + address_or_ble_device.details.get("from_connected_devices") + ) else: self._device_path = None self._device_info = None + self._from_connected_devices = False self._requested_services = services @@ -211,7 +218,7 @@ async def disconnect_device() -> None: # if connection was successful but _get_services() raises (e.g. # because task was cancelled), then we still need to disconnect # before passing on the exception. - if self._bus: + if self._bus and not self._from_connected_devices: # If disconnected callback already fired, this will be a no-op # since self._bus will be None and the _cleanup_all call will # have already disconnected. @@ -406,19 +413,31 @@ async def disconnect(self) -> None: self._disconnecting_event = asyncio.Event() try: if self.is_connected: - # Try to disconnect the actual device/peripheral - reply = await self._bus.call( - Message( - destination=defs.BLUEZ_SERVICE, - path=self._device_path, - interface=defs.DEVICE_INTERFACE, - member="Disconnect", + if not self._from_connected_devices: + # Try to disconnect the actual device/peripheral + reply = await self._bus.call( + Message( + destination=defs.BLUEZ_SERVICE, + path=self._device_path, + interface=defs.DEVICE_INTERFACE, + member="Disconnect", + ) ) - ) - assert_reply(reply) + assert_reply(reply) - async with async_timeout(10): - await self._disconnecting_event.wait() + async with async_timeout(10): + await self._disconnecting_event.wait() + else: + # Connection was opted into; leave it up. Mirror the + # cleanup that on_connected_changed would do, except + # for _disconnected_callback (the device is not + # actually disconnecting from BlueZ's perspective). + self._is_connected = False + if self._disconnect_monitor_event: + self._disconnect_monitor_event.set() + self._disconnect_monitor_event = None + self._cleanup_all() + self._disconnecting_event.set() finally: self._disconnecting_event = None diff --git a/docs/api/adapter.rst b/docs/api/adapter.rst index 20193a041..8948f0c14 100644 --- a/docs/api/adapter.rst +++ b/docs/api/adapter.rst @@ -49,3 +49,14 @@ system. The returned :class:`BLEDevice` objects can be passed directly to ... .. automethod:: bleak.BleakAdapter.get_connected_devices + +.. note:: + + On Linux/BlueZ, BLE connections are not ref-counted: if any client calls + ``Disconnect`` (or the original :class:`BleakClient` that established + the connection exits), the connection drops for *all* clients sharing + it, including those that obtained the device via + :meth:`get_connected_devices`. On Windows and macOS the OS BT stack + ref-counts the underlying handle, so an attached client stays + connected when the originator disconnects. See + https://github.com/bluez/bluez/issues/89. diff --git a/tests/integration/test_adapter.py b/tests/integration/test_adapter.py index aeaceee83..d19f29de8 100644 --- a/tests/integration/test_adapter.py +++ b/tests/integration/test_adapter.py @@ -1,3 +1,4 @@ +import asyncio import dataclasses from collections.abc import AsyncGenerator @@ -10,6 +11,7 @@ from bumble.transport.common import Transport from bleak import BleakAdapter, BleakClient +from bleak.backends import BleakBackend, get_default_backend from bleak.backends.device import BLEDevice from tests.integration.conftest import ( configure_and_power_on_bumble_peripheral, @@ -99,3 +101,58 @@ async def test_get_connected_devices_filters_by_service_uuid( not_connected = await adapter.get_connected_devices([OTHER_SERVICE_UUID]) assert not_connected == [] + + +@pytest.mark.asyncio(loop_scope="module") +async def test_disconnect_does_not_disconnect_connected_device( + connected_peripheral: ConnectedPeripheral, +) -> None: + """A BleakClient using a device from get_connected_devices() must not + disconnect the underlying connection on close. + """ + adapter = await BleakAdapter.get() + connected_devices = await adapter.get_connected_devices([TEST_SERVICE_UUID]) + target = next( + d for d in connected_devices if d.address == connected_peripheral.device.address + ) + + async with BleakClient(target) as client: + # If this is false, it is a bug in Bleak. Real usage never needs + # to check `is_connected` after connecting because it cannot be + # anything but true. If connecting fails, an exception is raised + # before we get here. + assert client.is_connected + + assert connected_peripheral.client.is_connected + + +@pytest.mark.asyncio(loop_scope="module") +async def test_disconnect_of_originating_client_disconnects_attached_client( + connected_peripheral: ConnectedPeripheral, +) -> None: + """When the BleakClient that originally established the connection + disconnects, the attached BleakClient (using a device from + get_connected_devices()) sees the disconnect on BlueZ but stays + connected on other backends. + """ + adapter = await BleakAdapter.get() + connected_devices = await adapter.get_connected_devices([TEST_SERVICE_UUID]) + target = next( + d for d in connected_devices if d.address == connected_peripheral.device.address + ) + + # NOTE: this test must be the last in the module since it leaves the + # fixture's BleakClient in a disconnected state. Reconnecting Bumble + # immediately after a cascading disconnect is unreliable. + async with BleakClient(target) as client: + await connected_peripheral.client.disconnect() + await asyncio.sleep(0.5) + if get_default_backend() == BleakBackend.BLUEZ_DBUS: + # BlueZ does not ref-count connections, so any client tearing + # down the connection takes everyone else with it. See + # https://github.com/bluez/bluez/issues/89. + assert not client.is_connected + else: + # Other backends ref-count and keep the attached client + # connected when the originating one disconnects. + assert client.is_connected