diff --git a/docs/usage.md b/docs/usage.md index c04e6b9..d9cea2e 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -322,15 +322,15 @@ checks these flags before calling proxy-side APIs and degrades gracefully when a flag is missing. The table below maps each public surface to the flag it requires and what happens when the proxy firmware does not advertise it. -| Public surface | Required flag | Behavior when flag is absent | -| ----------------------------------------------- | --------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `BleakClient.connect` / GATT operations | `ACTIVE_CONNECTIONS` | The scanner is registered as non-connectable. Discovery still works; connect attempts are rejected by `bleak` before reaching this library. | -| Decoded vs. raw advertisements | `RAW_ADVERTISEMENTS` | Falls back to `subscribe_bluetooth_le_advertisements` (proxy-side decoded) instead of `subscribe_bluetooth_le_raw_advertisements`. Both paths feed the same scanner. | -| Scanner state / mode + on-demand active windows | `FEATURE_STATE_AND_MODE` | `subscribe_bluetooth_scanner_state` is skipped and the `APIClient` is not bound to the scanner, so `current_mode` / `requested_mode` stay at their defaults and `habluetooth` cannot open on-demand active-scan windows (`async_request_active_window` returns `False`). Connection-slot tracking is unaffected. | -| `connect(dangerous_use_bleak_cache=…)` | `REMOTE_CACHING` | The cached-services hint sent to the proxy is forced off, so the proxy re-discovers services on every connect. `dangerous_use_bleak_cache` still hits the on-host LRU in `_get_services` when populated, and `start_notify` skips the CCCD write because the firmware handles it. | -| `BleakClient.pair` / `unpair` | `PAIRING` | Raises `NotImplementedError("Pairing is not available in this version ESPHome; Upgrade the ESPHome version on the … device.")`. | -| `ESPHomeClient.clear_cache` | `CACHE_CLEARING` | Returns `True` after clearing only the on-host LRU caches; logs `"On device cache clear is not available with this ESPHome version; … Only memory cache will be cleared"`. No proxy round-trip. | -| `ESPHomeClient.set_connection_params` | `CONNECTION_PARAMS_SETTING` | Silently returns after logging `"Setting connection parameters is not available with ESPHome version …; Upgrade the ESPHome version on the device"`. No exception is raised. | +| Public surface | Required flag | Behavior when flag is absent | +| ----------------------------------------------- | --------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `BleakClient.connect` / GATT operations | `ACTIVE_CONNECTIONS` | The scanner is registered as non-connectable. Discovery still works; connect attempts are rejected by `bleak` before reaching this library. | +| Decoded vs. raw advertisements | `RAW_ADVERTISEMENTS` | Falls back to `subscribe_bluetooth_le_advertisements` (proxy-side decoded) instead of `subscribe_bluetooth_le_raw_advertisements`. Both paths feed the same scanner. | +| Scanner state / mode + on-demand active windows | `FEATURE_STATE_AND_MODE` | `subscribe_bluetooth_scanner_state` is skipped and the `APIClient` is not bound to the scanner, so `current_mode` / `requested_mode` stay at their defaults and `habluetooth` cannot open on-demand active-scan windows (`async_request_active_window` returns `False`). Connection-slot tracking is unaffected. | +| `connect(dangerous_use_bleak_cache=…)` | `REMOTE_CACHING` | The cached-services hint sent to the proxy is forced off, so the proxy re-discovers services on every connect. `dangerous_use_bleak_cache` still hits the on-host LRU in `_get_services` when populated, and `start_notify` skips the CCCD write -- and `stop_notify` the matching CCCD clear -- because the firmware handles both. | +| `BleakClient.pair` / `unpair` | `PAIRING` | Raises `NotImplementedError("Pairing is not available in this version ESPHome; Upgrade the ESPHome version on the … device.")`. | +| `ESPHomeClient.clear_cache` | `CACHE_CLEARING` | Returns `True` after clearing only the on-host LRU caches; logs `"On device cache clear is not available with this ESPHome version; … Only memory cache will be cleared"`. No proxy round-trip. | +| `ESPHomeClient.set_connection_params` | `CONNECTION_PARAMS_SETTING` | Silently returns after logging `"Setting connection parameters is not available with ESPHome version …; Upgrade the ESPHome version on the device"`. No exception is raised. | If a method appears to silently do nothing, check the proxy's reported feature flags first — the warning is logged at WARNING level on the diff --git a/src/bleak_esphome/backend/client.py b/src/bleak_esphome/backend/client.py index 6668142..c73d199 100644 --- a/src/bleak_esphome/backend/client.py +++ b/src/bleak_esphome/backend/client.py @@ -8,7 +8,7 @@ import sys from dataclasses import dataclass, field from functools import partial, wraps -from typing import TYPE_CHECKING, Any, Concatenate, ParamSpec, TypeVar +from typing import TYPE_CHECKING, Any, Concatenate, ParamSpec, TypeAlias, TypeVar from aioesphomeapi import ( ESP_CONNECTION_ERROR_DESCRIPTION, @@ -43,6 +43,11 @@ from .device import ESPHomeBluetoothDevice from .scanner import ESPHomeScanner + # (release, abort) pair returned by ``bluetooth_gatt_start_notify``. + _NotifyCancel: TypeAlias = tuple[ + Callable[[], Coroutine[Any, Any, None]], Callable[[], None] + ] + if sys.version_info < (3, 12): from typing_extensions import Buffer else: @@ -60,6 +65,7 @@ CCCD_UUID = "00002902-0000-1000-8000-00805f9b34fb" CCCD_NOTIFY_BYTES = b"\x01\x00" CCCD_INDICATE_BYTES = b"\x02\x00" +CCCD_DISABLE_BYTES = b"\x00\x00" DEFAULT_MAX_WRITE_WITHOUT_RESPONSE = DEFAULT_MTU - GATT_HEADER_SIZE @@ -162,9 +168,10 @@ def __init__( self._pending_release = False self._mtu: int | None = None self._cancel_connection_state: Callable[[], None] | None = None - self._notify_cancels: dict[ - int, tuple[Callable[[], Coroutine[Any, Any, None]], Callable[[], None]] - ] = {} + self._notify_cancels: dict[int, _NotifyCancel] = {} + # Handles whose CCCD write may have landed while the local bookkeeping + # was unwound, so a later stop_notify retries the clear. + self._cccd_dirty: set[int] = set() self._device_info = client_data.device_info self._feature_flags = device_info.bluetooth_proxy_feature_flags_compat( client_data.api_version @@ -191,6 +198,7 @@ def _async_disconnected_cleanup(self) -> None: for _, notify_abort in self._notify_cancels.values(): notify_abort() self._notify_cancels.clear() + self._cccd_dirty.clear() self._disconnect_callbacks.discard(self._async_esp_disconnected) self._bluetooth_device.async_untrack_client( self._address_as_int, self._async_ble_device_disconnected @@ -876,6 +884,13 @@ async def start_notify( proxy (the subscribe, and the CCCD write that follows it on v3/REMOTE_CACHING connections). Defaults to 30.0. + Note: + ---- + On a ``REMOTE_CACHING`` proxy a failed CCCD write may still have + landed, leaving the peripheral notifying. Callers that keep the + connection alive should call ``stop_notify`` on the same + characteristic to clear it. + """ self._raise_if_not_connected() timeout = kwargs.get("timeout", GATT_NOTIFY_TIMEOUT) @@ -905,32 +920,18 @@ async def start_notify( ) ) - if not self._feature_flags & BluetoothProxyFeature.REMOTE_CACHING.value: - return - try: # For connection v3 we are responsible for enabling notifications # on the cccd (characteristic client config descriptor) handle since # the esp32 will not have resolved the characteristic descriptors to # save memory since doing so can exhaust the memory and cause a soft # reset - cccd_descriptor = characteristic.get_descriptor(CCCD_UUID) - if not cccd_descriptor: - raise BleakError( - f"{self._description}: Characteristic {characteristic.uuid} " - "does not have a characteristic client config descriptor." - ) - - _LOGGER.debug( - "%s: Writing to CCD descriptor %s for notifications with properties=%s", - self._description, - cccd_descriptor.handle, - characteristic.properties, - ) + if (cccd_descriptor := self._get_cccd(characteristic)) is None: + return supports_notify = "notify" in characteristic.properties - await self._client.bluetooth_gatt_write_descriptor( - self._address_as_int, - cccd_descriptor.handle, + await self._async_write_cccd( + ble_handle, + cccd_descriptor, CCCD_NOTIFY_BYTES if supports_notify else CCCD_INDICATE_BYTES, timeout, ) @@ -950,6 +951,15 @@ async def stop_notify(self, characteristic: BleakGATTCharacteristic) -> None: handle, matching the BlueZ backend's behavior. Callers do not need to track which characteristics they have subscribed to. + On connection v3 (``REMOTE_CACHING``) proxies the client config + descriptor is cleared so the peripheral stops notifying. That is a + round trip, so this method can block for up to the proxy GATT + timeout and can raise ``BleakError`` where it previously always + returned. Calling ``stop_notify`` again retries whatever failed: + the CCCD write, and the proxy-side release, which is put back + under its handle when it raises. A characteristic with no client + config descriptor raises on every call and has nothing to retry. + Args: ---- characteristic (BleakGATTCharacteristic): @@ -959,11 +969,138 @@ async def stop_notify(self, characteristic: BleakGATTCharacteristic) -> None: """ self._raise_if_not_connected() + handle = characteristic.handle # Do not raise KeyError if notifications are not enabled on this characteristic - # to be consistent with the behavior of the BlueZ backend - if notify_cancel := self._notify_cancels.pop(characteristic.handle, None): - notify_stop, _ = notify_cancel + # to be consistent with the behavior of the BlueZ backend. Popping up front + # keeps a concurrent second stop_notify the no-op it has always been. + if not (notify_cancel := self._notify_cancels.pop(handle, None)): + # The subscription is already gone, but an earlier CCCD write may + # have failed. Retry it instead of reporting success. + if handle in self._cccd_dirty: + await self._async_clear_cccd(characteristic) + return + try: + # Write the CCCD first so the peripheral is quiet before the + # proxy-side subscription goes away. + await self._async_clear_cccd(characteristic) + except BaseException: + # Use BaseException to handle CancelledError as well as Exception. + # Release the proxy-side subscription anyway, but let the CCCD + # failure reach the caller: it is the actionable root cause. The + # release keeps its own retry affordance, so nothing is lost. + try: + await self._async_release_notify(handle, notify_cancel) + except Exception: + _LOGGER.warning( + "%s: Failed to release the proxy notify subscription for " + "handle %s; the proxy may keep forwarding notifications", + self._description, + handle, + exc_info=True, + ) + raise + await self._async_release_notify(handle, notify_cancel) + + async def _async_release_notify( + self, handle: int, notify_cancel: _NotifyCancel + ) -> None: + """ + Release the proxy-side notify subscription for ``handle``. + + The handle is popped from ``_notify_cancels`` before the CCCD write, + so a failed release would strand the subscription: nothing left for + ``_async_disconnected_cleanup`` to abort and nothing for a later + ``stop_notify`` to retry. Put the pair back so both paths reach it + again -- unless the link went down, since the cleanup already ran and + an unguarded restore would survive into the next connection, or a + ``start_notify`` took the handle over in the meantime. + """ + notify_stop, _ = notify_cancel + try: await notify_stop() + except BaseException: + if self._is_connected: + self._notify_cancels.setdefault(handle, notify_cancel) + raise + + def _get_cccd( + self, characteristic: BleakGATTCharacteristic + ) -> BleakGATTDescriptor | None: + """ + Return the client config descriptor this host has to write itself. + + ``None`` without ``REMOTE_CACHING``: the esp32 resolved the + descriptors and drives the CCCD itself. On connection v3 it skipped + that resolution to save memory, so a missing CCCD is an error. + """ + if not self._feature_flags & BluetoothProxyFeature.REMOTE_CACHING.value: + return None + if cccd_descriptor := characteristic.get_descriptor(CCCD_UUID): + return cccd_descriptor + raise BleakError( + f"{self._description}: Characteristic {characteristic.uuid} " + "does not have a characteristic client config descriptor." + ) + + async def _async_write_cccd( + self, + char_handle: int, + cccd_descriptor: BleakGATTDescriptor, + value: bytes, + timeout: float, + ) -> None: + """ + Write ``value`` to the client config descriptor of ``char_handle``. + + A failed write may still have landed on the peripheral, so the + characteristic handle is recorded in ``_cccd_dirty`` and a later + ``stop_notify`` clears the descriptor instead of taking the + missing-handle no-op. Only while the link is up: a disconnect clears + the set, so an unguarded add would survive into the next connection, + which has nothing left to retry. + """ + _LOGGER.debug( + "%s: Writing %s to CCD descriptor %s for characteristic %s", + self._description, + value, + cccd_descriptor.handle, + char_handle, + ) + try: + await self._client.bluetooth_gatt_write_descriptor( + self._address_as_int, + cccd_descriptor.handle, + value, + timeout, + ) + except BaseException: + if self._is_connected: + self._cccd_dirty.add(char_handle) + raise + if char_handle in self._notify_cancels: + # A start_notify owns the handle and may have a write in flight + # against the same descriptor with no ordering guarantee, so leave + # it dirty for the next stop_notify rather than declaring it clean. + return + self._cccd_dirty.discard(char_handle) + + async def _async_clear_cccd(self, characteristic: BleakGATTCharacteristic) -> None: + """ + Write ``0x0000`` to the client config descriptor. + + Mirror of the connection v3 branch in ``start_notify``: the esp32 + never resolved the descriptors, so it cannot undo the CCCD write + either. Without this the proxy stops forwarding the notifications but + the peripheral keeps sending them for the life of the connection. + """ + if (cccd_descriptor := self._get_cccd(characteristic)) is None: + return + await self._async_write_cccd( + characteristic.handle, + cccd_descriptor, + CCCD_DISABLE_BYTES, + GATT_NOTIFY_TIMEOUT, + ) def _raise_if_not_connected(self) -> None: """Raise a BleakError if not connected.""" diff --git a/tests/backend/_helpers.py b/tests/backend/_helpers.py index 6dd7539..0dfcf54 100644 --- a/tests/backend/_helpers.py +++ b/tests/backend/_helpers.py @@ -23,6 +23,8 @@ ESP_MAC_ADDRESS = "AA:BB:CC:DD:EE:FF" ESP_NAME = "proxy" BLE_ADDRESS = "CC:BB:AA:DD:EE:FF" +# The indicate-only characteristic in the standard GATT services fixture. +INDICATE_CHAR_UUID = "00002a05-0000-1000-8000-00805f9b34fb" def make_ble_device() -> BLEDevice: diff --git a/tests/backend/test_client.py b/tests/backend/test_client.py index 2110363..dc26e04 100644 --- a/tests/backend/test_client.py +++ b/tests/backend/test_client.py @@ -30,6 +30,7 @@ from ._helpers import ( ESP_MAC_ADDRESS, ESP_NAME, + INDICATE_CHAR_UUID, fetch_services, make_bleak_client, patch_connect_rpcs, @@ -38,7 +39,6 @@ ) PRIMARY_CHAR_UUID = "090b7847-e12b-09a8-b04b-8e0922a9abab" -INDICATE_CHAR_UUID = "00002a05-0000-1000-8000-00805f9b34fb" CCCD_UUID = "00002902-0000-1000-8000-00805f9b34fb" BLE_ADDRESS_AS_INT = 225106397622015 diff --git a/tests/backend/test_client_branches.py b/tests/backend/test_client_branches.py index 692ea79..36005da 100644 --- a/tests/backend/test_client_branches.py +++ b/tests/backend/test_client_branches.py @@ -13,9 +13,10 @@ import asyncio import gc +import logging from collections.abc import Iterator from typing import Any -from unittest.mock import AsyncMock, Mock, patch +from unittest.mock import ANY, AsyncMock, Mock, patch import pytest from aioesphomeapi import ( @@ -35,9 +36,33 @@ from bleak.exc import BleakError from pytest_asyncio import fixture as aio_fixture -from bleak_esphome.backend.client import ESPHomeClient, ESPHomeClientData +from bleak_esphome.backend.client import ( + CCCD_DISABLE_BYTES, + CCCD_UUID, + GATT_NOTIFY_TIMEOUT, + ESPHomeClient, + ESPHomeClientData, +) -from ._helpers import ESP_MAC_ADDRESS, _make_client +from ._helpers import ( + ESP_MAC_ADDRESS, + INDICATE_CHAR_UUID, + _make_client, + fetch_services, +) + + +async def _connected_client_with_char( + client_data: ESPHomeClientData, + services_payload: ESPHomeBluetoothGATTServices, +) -> tuple[ESPHomeClient, BleakGATTCharacteristic]: + """Return a connected client and the fixture's indicate characteristic.""" + client = _make_client(client_data) + client._is_connected = True + services = await fetch_services(client, services_payload) + char = services.get_characteristic(INDICATE_CHAR_UUID) + assert char is not None + return client, char @pytest.mark.asyncio @@ -388,16 +413,9 @@ async def test_start_notify_already_enabled_raises( esphome_bluetooth_gatt_services: ESPHomeBluetoothGATTServices, ) -> None: """A second start_notify on the same handle raises BleakError.""" - client = _make_client(client_data) - client._is_connected = True - with patch.object( - client._client, - "bluetooth_gatt_get_services", - return_value=esphome_bluetooth_gatt_services, - ): - services = await client._get_services() - char = services.get_characteristic("00002a05-0000-1000-8000-00805f9b34fb") - assert char is not None + client, char = await _connected_client_with_char( + client_data, esphome_bluetooth_gatt_services + ) # Pre-populate the cancels dict; the implementation only inspects keys. client._notify_cancels[char.handle] = (AsyncMock(), Mock()) with pytest.raises(BleakError, match="already enabled"): @@ -433,16 +451,9 @@ async def test_start_notify_skips_cccd_without_remote_caching( esphome_bluetooth_gatt_services: ESPHomeBluetoothGATTServices, ) -> None: """Without REMOTE_CACHING the host does not write to the CCCD itself.""" - client = _make_client(client_data) - client._is_connected = True - with patch.object( - client._client, - "bluetooth_gatt_get_services", - return_value=esphome_bluetooth_gatt_services, - ): - services = await client._get_services() - char = services.get_characteristic("00002a05-0000-1000-8000-00805f9b34fb") - assert char is not None + client, char = await _connected_client_with_char( + client_data, esphome_bluetooth_gatt_services + ) # Disable REMOTE_CACHING so start_notify returns before touching the CCCD. client._feature_flags &= ~BluetoothProxyFeature.REMOTE_CACHING.value mock_stop_notify = AsyncMock() @@ -469,6 +480,7 @@ async def test_stop_notify_calls_stop_callback( """``stop_notify`` awaits and removes the stored stop callback.""" client = _make_client(client_data) client._is_connected = True + client._feature_flags &= ~BluetoothProxyFeature.REMOTE_CACHING.value stop = AsyncMock() abort = Mock() char = Mock() @@ -479,6 +491,312 @@ async def test_stop_notify_calls_stop_callback( assert 99 not in client._notify_cancels +@pytest.mark.asyncio +async def test_stop_notify_disables_cccd( + client_data: ESPHomeClientData, + esphome_bluetooth_gatt_services: ESPHomeBluetoothGATTServices, +) -> None: + """With REMOTE_CACHING the host clears the CCCD it wrote on start.""" + client, char = await _connected_client_with_char( + client_data, esphome_bluetooth_gatt_services + ) + cccd = char.get_descriptor(CCCD_UUID) + assert cccd is not None + stop = AsyncMock() + client._notify_cancels[char.handle] = (stop, Mock()) + with patch.object( + client._client, "bluetooth_gatt_write_descriptor" + ) as mock_write_desc: + await client.stop_notify(char) + mock_write_desc.assert_awaited_once_with( + client._address_as_int, cccd.handle, CCCD_DISABLE_BYTES, GATT_NOTIFY_TIMEOUT + ) + stop.assert_awaited_once() + assert char.handle not in client._notify_cancels + + +@pytest.mark.asyncio +async def test_stop_notify_skips_cccd_without_remote_caching( + client_data: ESPHomeClientData, + esphome_bluetooth_gatt_services: ESPHomeBluetoothGATTServices, +) -> None: + """Without REMOTE_CACHING the esp32 owns the CCCD, so the host leaves it.""" + client, char = await _connected_client_with_char( + client_data, esphome_bluetooth_gatt_services + ) + client._feature_flags &= ~BluetoothProxyFeature.REMOTE_CACHING.value + stop = AsyncMock() + client._notify_cancels[char.handle] = (stop, Mock()) + with patch.object( + client._client, "bluetooth_gatt_write_descriptor" + ) as mock_write_desc: + await client.stop_notify(char) + mock_write_desc.assert_not_called() + stop.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_stop_notify_releases_proxy_when_cccd_write_fails( + client_data: ESPHomeClientData, + esphome_bluetooth_gatt_services: ESPHomeBluetoothGATTServices, +) -> None: + """A failing CCCD write still releases the proxy-side subscription.""" + client, char = await _connected_client_with_char( + client_data, esphome_bluetooth_gatt_services + ) + stop = AsyncMock() + client._notify_cancels[char.handle] = (stop, Mock()) + with ( + patch.object( + client._client, + "bluetooth_gatt_write_descriptor", + side_effect=BluetoothGATTAPIError(BluetoothGATTError(address=1, handle=2)), + ), + pytest.raises(BleakError), + ): + await client.stop_notify(char) + stop.assert_awaited_once() + assert char.handle not in client._notify_cancels + + +@pytest.mark.asyncio +async def test_stop_notify_raises_when_cccd_missing( + client_data: ESPHomeClientData, +) -> None: + """A missing CCCD under REMOTE_CACHING raises, as it does on start.""" + client = _make_client(client_data) + client._is_connected = True + stop = AsyncMock() + char = Mock() + char.handle = 99 + char.uuid = INDICATE_CHAR_UUID + char.get_descriptor.return_value = None + client._notify_cancels[99] = (stop, Mock()) + with ( + patch.object( + client._client, "bluetooth_gatt_write_descriptor" + ) as mock_write_desc, + pytest.raises(BleakError, match="client config descriptor"), + ): + await client.stop_notify(char) + mock_write_desc.assert_not_called() + stop.assert_awaited_once() + assert 99 not in client._notify_cancels + # There is no descriptor to write, so nothing is retryable: the handle + # must not be latched as dirty or every later stop_notify would raise + # instead of returning the BlueZ-compatible no-op. + assert 99 not in client._cccd_dirty + with patch.object( + client._client, "bluetooth_gatt_write_descriptor" + ) as mock_write_desc: + await client.stop_notify(char) + mock_write_desc.assert_not_called() + + +@pytest.mark.asyncio +async def test_stop_notify_cccd_failure_survives_failing_release( + client_data: ESPHomeClientData, + esphome_bluetooth_gatt_services: ESPHomeBluetoothGATTServices, + caplog: pytest.LogCaptureFixture, +) -> None: + """A failing release is logged and does not mask the CCCD error.""" + client, char = await _connected_client_with_char( + client_data, esphome_bluetooth_gatt_services + ) + stop = AsyncMock(side_effect=RuntimeError("release failed")) + client._notify_cancels[char.handle] = (stop, Mock()) + with ( + caplog.at_level(logging.WARNING), + patch.object( + client._client, + "bluetooth_gatt_write_descriptor", + side_effect=BluetoothGATTAPIError(BluetoothGATTError(address=1, handle=2)), + ), + pytest.raises(BleakError), + ): + await client.stop_notify(char) + stop.assert_awaited_once() + # The failed release is put back so a retry and the disconnect cleanup + # can still reach it. + assert client._notify_cancels[char.handle] == (stop, ANY) + # The CCCD failure is the actionable root cause the caller gets; the + # secondary release failure is reported through the log. + assert "Failed to release the proxy notify subscription" in caplog.text + + +@pytest.mark.asyncio +async def test_stop_notify_release_failure_after_disconnect_is_not_restored( + client_data: ESPHomeClientData, + esphome_bluetooth_gatt_services: ESPHomeBluetoothGATTServices, +) -> None: + """A release failing after the link dropped must not outlive it.""" + client, char = await _connected_client_with_char( + client_data, esphome_bluetooth_gatt_services + ) + + async def _disconnect_and_raise() -> None: + client._is_connected = False + raise RuntimeError("release failed") + + stop = AsyncMock(side_effect=_disconnect_and_raise) + client._notify_cancels[char.handle] = (stop, Mock()) + with ( + patch.object(client._client, "bluetooth_gatt_write_descriptor"), + pytest.raises(RuntimeError), + ): + await client.stop_notify(char) + stop.assert_awaited_once() + assert char.handle not in client._notify_cancels + + +@pytest.mark.asyncio +async def test_stop_notify_forgets_cccd_when_disconnected_mid_write( + client_data: ESPHomeClientData, + esphome_bluetooth_gatt_services: ESPHomeBluetoothGATTServices, +) -> None: + """A timeout after a disconnect must not latch the handle as dirty.""" + client, char = await _connected_client_with_char( + client_data, esphome_bluetooth_gatt_services + ) + client._notify_cancels[char.handle] = (AsyncMock(), Mock()) + + async def _disconnect_and_time_out(*args: Any, **kwargs: Any) -> None: + client._async_disconnected_cleanup() + raise TimeoutAPIError("timed out") + + # A TimeoutAPIError does not trigger a second disconnected cleanup, so an + # unguarded add would survive the disconnect. + with ( + patch.object( + client._client, + "bluetooth_gatt_write_descriptor", + side_effect=_disconnect_and_time_out, + ), + pytest.raises(TimeoutError), + ): + await client.stop_notify(char) + assert not client._cccd_dirty + + +@pytest.mark.asyncio +async def test_stop_notify_cccd_failure_survives_cancelled_release( + client_data: ESPHomeClientData, + esphome_bluetooth_gatt_services: ESPHomeBluetoothGATTServices, +) -> None: + """A cancelled release propagates instead of being demoted to a log.""" + client, char = await _connected_client_with_char( + client_data, esphome_bluetooth_gatt_services + ) + stop = AsyncMock(side_effect=asyncio.CancelledError()) + client._notify_cancels[char.handle] = (stop, Mock()) + with ( + patch.object( + client._client, + "bluetooth_gatt_write_descriptor", + side_effect=BluetoothGATTAPIError(BluetoothGATTError(address=1, handle=2)), + ), + pytest.raises(asyncio.CancelledError), + ): + await client.stop_notify(char) + stop.assert_awaited_once() + # A cancelled release is retryable too, so the pair goes back. + assert client._notify_cancels[char.handle] == (stop, ANY) + + +@pytest.mark.asyncio +async def test_stop_notify_raises_when_release_fails( + client_data: ESPHomeClientData, +) -> None: + """A failing release on the success path reaches the caller.""" + client = _make_client(client_data) + client._is_connected = True + client._feature_flags &= ~BluetoothProxyFeature.REMOTE_CACHING.value + stop = AsyncMock(side_effect=RuntimeError("release failed")) + char = Mock() + char.handle = 99 + client._notify_cancels[99] = (stop, Mock()) + with pytest.raises(RuntimeError): + await client.stop_notify(char) + stop.assert_awaited_once() + assert client._notify_cancels[99] == (stop, ANY) + + +@pytest.mark.asyncio +async def test_stop_notify_survives_concurrent_cancel_clear( + client_data: ESPHomeClientData, + esphome_bluetooth_gatt_services: ESPHomeBluetoothGATTServices, +) -> None: + """A disconnect clearing the dict during the CCCD write is not a KeyError.""" + client, char = await _connected_client_with_char( + client_data, esphome_bluetooth_gatt_services + ) + stop = AsyncMock() + client._notify_cancels[char.handle] = (stop, Mock()) + + async def _clear(*args: Any, **kwargs: Any) -> None: + client._notify_cancels.clear() + + with patch.object( + client._client, "bluetooth_gatt_write_descriptor", side_effect=_clear + ): + await client.stop_notify(char) + stop.assert_awaited_once() + assert char.handle not in client._notify_cancels + + +@pytest.mark.asyncio +async def test_stop_notify_is_single_winner_when_reentered( + client_data: ESPHomeClientData, + esphome_bluetooth_gatt_services: ESPHomeBluetoothGATTServices, +) -> None: + """A second stop_notify during the CCCD write is a no-op, not a duplicate.""" + client, char = await _connected_client_with_char( + client_data, esphome_bluetooth_gatt_services + ) + stop = AsyncMock() + client._notify_cancels[char.handle] = (stop, Mock()) + + async def _reenter(*args: Any, **kwargs: Any) -> None: + await client.stop_notify(char) + + with patch.object( + client._client, "bluetooth_gatt_write_descriptor", side_effect=_reenter + ) as mock_write_desc: + await client.stop_notify(char) + assert mock_write_desc.await_count == 1 + stop.assert_awaited_once() + assert char.handle not in client._notify_cancels + + +@pytest.mark.asyncio +async def test_stop_notify_cccd_failure_survives_concurrent_cancel_clear( + client_data: ESPHomeClientData, + esphome_bluetooth_gatt_services: ESPHomeBluetoothGATTServices, +) -> None: + """The error path pops defensively when a disconnect cleared the dict.""" + client, char = await _connected_client_with_char( + client_data, esphome_bluetooth_gatt_services + ) + + async def _clear_and_raise(*args: Any, **kwargs: Any) -> None: + client._notify_cancels.clear() + raise BluetoothGATTAPIError(BluetoothGATTError(address=1, handle=2)) + + stop = AsyncMock() + client._notify_cancels[char.handle] = (stop, Mock()) + with ( + patch.object( + client._client, + "bluetooth_gatt_write_descriptor", + side_effect=_clear_and_raise, + ), + pytest.raises(BleakError), + ): + await client.stop_notify(char) + stop.assert_awaited_once() + assert char.handle not in client._notify_cancels + + @pytest.mark.asyncio async def test_stop_notify_missing_handle_is_noop( client_data: ESPHomeClientData, @@ -678,3 +996,119 @@ async def test_del_handles_loop_closed_race( side_effect=RuntimeError("loop closed"), ): client.__del__() # must not raise + + +@pytest.mark.asyncio +async def test_stop_notify_retries_cccd_after_a_failed_clear( + client_data: ESPHomeClientData, + esphome_bluetooth_gatt_services: ESPHomeBluetoothGATTServices, +) -> None: + """A later stop_notify re-attempts a CCCD clear that failed before.""" + client, char = await _connected_client_with_char( + client_data, esphome_bluetooth_gatt_services + ) + cccd = char.get_descriptor(CCCD_UUID) + assert cccd is not None + stop = AsyncMock() + client._notify_cancels[char.handle] = (stop, Mock()) + with ( + patch.object( + client._client, + "bluetooth_gatt_write_descriptor", + side_effect=BluetoothGATTAPIError(BluetoothGATTError(address=1, handle=2)), + ), + pytest.raises(BleakError), + ): + await client.stop_notify(char) + assert char.handle not in client._notify_cancels + assert char.handle in client._cccd_dirty + # The proxy-side subscription is gone, but the peripheral is still + # notifying, so the retry must write the CCCD instead of returning. + with patch.object( + client._client, "bluetooth_gatt_write_descriptor" + ) as mock_write_desc: + await client.stop_notify(char) + mock_write_desc.assert_awaited_once_with( + client._address_as_int, cccd.handle, CCCD_DISABLE_BYTES, GATT_NOTIFY_TIMEOUT + ) + assert char.handle not in client._cccd_dirty + stop.assert_awaited_once() + # Once the CCCD is clear the handle is a plain no-op again. + with patch.object( + client._client, "bluetooth_gatt_write_descriptor" + ) as mock_write_desc: + await client.stop_notify(char) + mock_write_desc.assert_not_called() + + +@pytest.mark.asyncio +async def test_start_notify_cccd_failure_is_cleared_by_stop_notify( + client_data: ESPHomeClientData, + esphome_bluetooth_gatt_services: ESPHomeBluetoothGATTServices, +) -> None: + """A failed enable write may have landed, so stop_notify clears it.""" + client, char = await _connected_client_with_char( + client_data, esphome_bluetooth_gatt_services + ) + cccd = char.get_descriptor(CCCD_UUID) + assert cccd is not None + with ( + patch.object( + client._client, + "bluetooth_gatt_start_notify", + return_value=(AsyncMock(), Mock()), + ), + patch.object( + client._client, + "bluetooth_gatt_write_descriptor", + side_effect=BluetoothGATTAPIError(BluetoothGATTError(address=1, handle=2)), + ), + patch.object(client._client, "bluetooth_gatt_stop_notify"), + pytest.raises(BleakError), + ): + await client.start_notify(char, lambda data: None) + assert char.handle not in client._notify_cancels + assert char.handle in client._cccd_dirty + with patch.object( + client._client, "bluetooth_gatt_write_descriptor" + ) as mock_write_desc: + await client.stop_notify(char) + mock_write_desc.assert_awaited_once_with( + client._address_as_int, cccd.handle, CCCD_DISABLE_BYTES, GATT_NOTIFY_TIMEOUT + ) + assert char.handle not in client._cccd_dirty + + +@pytest.mark.asyncio +async def test_stop_notify_retry_racing_start_notify_stays_dirty( + client_data: ESPHomeClientData, + esphome_bluetooth_gatt_services: ESPHomeBluetoothGATTServices, +) -> None: + """A resubscribe during the retry keeps the handle dirty.""" + client, char = await _connected_client_with_char( + client_data, esphome_bluetooth_gatt_services + ) + client._cccd_dirty.add(char.handle) + + async def _resubscribe(*args: Any, **kwargs: Any) -> None: + client._notify_cancels[char.handle] = (AsyncMock(), Mock()) + + # The concurrent start_notify writes the same descriptor with no ordering + # guarantee, so the clear cannot claim the CCCD is off. + with patch.object( + client._client, "bluetooth_gatt_write_descriptor", side_effect=_resubscribe + ): + await client.stop_notify(char) + assert char.handle in client._cccd_dirty + + +@pytest.mark.asyncio +async def test_disconnect_forgets_the_outstanding_cccd_clear( + client_data: ESPHomeClientData, +) -> None: + """A disconnect drops the pending CCCD write with the link.""" + client = _make_client(client_data) + client._is_connected = True + client._cccd_dirty.add(99) + client._async_disconnected_cleanup() + assert not client._cccd_dirty