From de1e97a81f637853283e2263d6daa4ed9754a330 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 2 Dec 2022 14:45:49 -1000 Subject: [PATCH] Fix esphome ble client leaking notify on disconnect (#83106) * Fix esphome ble client leaking notify on disconnect needs: https://github.com/esphome/aioesphomeapi/pull/329 * leak * more cleanup * more cleanup * bump --- .../components/esphome/bluetooth/client.py | 32 ++++++++++++------- .../components/esphome/manifest.json | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/homeassistant/components/esphome/bluetooth/client.py b/homeassistant/components/esphome/bluetooth/client.py index da4d5fb3d26..1ec3b585fae 100644 --- a/homeassistant/components/esphome/bluetooth/client.py +++ b/homeassistant/components/esphome/bluetooth/client.py @@ -142,7 +142,9 @@ class ESPHomeClient(BaseBleakClient): self._is_connected = False self._mtu: int | None = None self._cancel_connection_state: CALLBACK_TYPE | None = None - self._notify_cancels: dict[int, Callable[[], Coroutine[Any, Any, None]]] = {} + self._notify_cancels: dict[ + int, tuple[Callable[[], Coroutine[Any, Any, None]], Callable[[], None]] + ] = {} self._disconnected_event: asyncio.Event | None = None device_info = self.entry_data.device_info assert device_info is not None @@ -169,15 +171,22 @@ class ESPHomeClient(BaseBleakClient): ) self._cancel_connection_state = None - def _async_ble_device_disconnected(self) -> None: - """Handle the BLE device disconnecting from the ESP.""" - was_connected = self._is_connected + def _async_disconnected_cleanup(self) -> None: + """Clean up on disconnect.""" self.services = BleakGATTServiceCollection() # type: ignore[no-untyped-call] self._is_connected = False + for _, notify_abort in self._notify_cancels.values(): + notify_abort() self._notify_cancels.clear() if self._disconnected_event: self._disconnected_event.set() self._disconnected_event = None + self._unsubscribe_connection_state() + + def _async_ble_device_disconnected(self) -> None: + """Handle the BLE device disconnecting from the ESP.""" + was_connected = self._is_connected + self._async_disconnected_cleanup() if was_connected: _LOGGER.debug( "%s: %s - %s: BLE device disconnected", @@ -186,7 +195,6 @@ class ESPHomeClient(BaseBleakClient): self._ble_device.address, ) self._async_call_bleak_disconnected_callback() - self._unsubscribe_connection_state() def _async_esp_disconnected(self) -> None: """Handle the esp32 client disconnecting from hass.""" @@ -316,7 +324,7 @@ class ESPHomeClient(BaseBleakClient): @api_error_as_bleak_error async def disconnect(self) -> bool: """Disconnect from the peripheral device.""" - self._unsubscribe_connection_state() + self._async_disconnected_cleanup() await self._client.bluetooth_device_disconnect(self._address_as_int) await self._wait_for_free_connection_slot(DISCONNECT_TIMEOUT) return True @@ -551,12 +559,13 @@ class ESPHomeClient(BaseBleakClient): f"Characteristic {characteristic.uuid} does not have notify or indicate property set." ) - cancel_coro = await self._client.bluetooth_gatt_start_notify( + self._notify_cancels[ + ble_handle + ] = await self._client.bluetooth_gatt_start_notify( self._address_as_int, ble_handle, lambda handle, data: callback(data), ) - self._notify_cancels[ble_handle] = cancel_coro if self._connection_version < MIN_BLUETOOTH_PROXY_VERSION_HAS_CACHE: return @@ -604,8 +613,9 @@ class ESPHomeClient(BaseBleakClient): characteristic = self._resolve_characteristic(char_specifier) # Do not raise KeyError if notifications are not enabled on this characteristic # to be consistent with the behavior of the BlueZ backend - if coro := self._notify_cancels.pop(characteristic.handle, None): - await coro() + if notify_cancel := self._notify_cancels.pop(characteristic.handle, None): + notify_stop, _ = notify_cancel + await notify_stop() def __del__(self) -> None: """Destructor to make sure the connection state is unsubscribed.""" @@ -617,4 +627,4 @@ class ESPHomeClient(BaseBleakClient): self._ble_device.address, ) if not self._hass.loop.is_closed(): - self._hass.loop.call_soon_threadsafe(self._unsubscribe_connection_state) + self._hass.loop.call_soon_threadsafe(self._async_disconnected_cleanup) diff --git a/homeassistant/components/esphome/manifest.json b/homeassistant/components/esphome/manifest.json index 42ebc471b7c..6d9796b8541 100644 --- a/homeassistant/components/esphome/manifest.json +++ b/homeassistant/components/esphome/manifest.json @@ -3,7 +3,7 @@ "name": "ESPHome", "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/esphome", - "requirements": ["aioesphomeapi==12.2.1"], + "requirements": ["aioesphomeapi==13.0.0"], "zeroconf": ["_esphomelib._tcp.local."], "dhcp": [{ "registered_devices": true }], "codeowners": ["@OttoWinter", "@jesserockz"], diff --git a/requirements_all.txt b/requirements_all.txt index c00615c7f4d..1434622c7bf 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -156,7 +156,7 @@ aioecowitt==2022.11.0 aioemonitor==1.0.5 # homeassistant.components.esphome -aioesphomeapi==12.2.1 +aioesphomeapi==13.0.0 # homeassistant.components.flo aioflo==2021.11.0 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index c4d38523559..21c332500d9 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -143,7 +143,7 @@ aioecowitt==2022.11.0 aioemonitor==1.0.5 # homeassistant.components.esphome -aioesphomeapi==12.2.1 +aioesphomeapi==13.0.0 # homeassistant.components.flo aioflo==2021.11.0