From a8bb00f30581968eda231feda740790c5e64503a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" <nick@koston.org> Date: Sun, 17 Jul 2022 17:45:04 -0500 Subject: [PATCH] Fix availability in HKC for sleeping bluetooth devices (#75357) --- .../homekit_controller/connection.py | 79 +++++++++-------- .../homekit_controller/manifest.json | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- .../homekit_controller/test_init.py | 87 ++++++++++++++++--- 5 files changed, 125 insertions(+), 47 deletions(-) diff --git a/homeassistant/components/homekit_controller/connection.py b/homeassistant/components/homekit_controller/connection.py index 7e4dcc59be0..b2f3b3ae8e0 100644 --- a/homeassistant/components/homekit_controller/connection.py +++ b/homeassistant/components/homekit_controller/connection.py @@ -3,7 +3,7 @@ from __future__ import annotations import asyncio from collections.abc import Callable -import datetime +from datetime import timedelta import logging from types import MappingProxyType from typing import Any @@ -14,8 +14,8 @@ from aiohomekit.exceptions import ( AccessoryNotFoundError, EncryptionError, ) -from aiohomekit.model import Accessories, Accessory -from aiohomekit.model.characteristics import Characteristic, CharacteristicsTypes +from aiohomekit.model import Accessories, Accessory, Transport +from aiohomekit.model.characteristics import Characteristic from aiohomekit.model.services import Service from homeassistant.config_entries import ConfigEntry @@ -40,9 +40,9 @@ from .const import ( from .device_trigger import async_fire_triggers, async_setup_triggers_for_entry from .storage import EntityMapStorage -DEFAULT_SCAN_INTERVAL = datetime.timedelta(seconds=60) RETRY_INTERVAL = 60 # seconds MAX_POLL_FAILURES_TO_DECLARE_UNAVAILABLE = 3 +BLE_AVAILABILITY_CHECK_INTERVAL = 1800 # seconds _LOGGER = logging.getLogger(__name__) @@ -122,6 +122,7 @@ class HKDevice: # If this is set polling is active and can be disabled by calling # this method. self._polling_interval_remover: CALLBACK_TYPE | None = None + self._ble_available_interval_remover: CALLBACK_TYPE | None = None # Never allow concurrent polling of the same accessory or bridge self._polling_lock = asyncio.Lock() @@ -133,9 +134,6 @@ class HKDevice: self.watchable_characteristics: list[tuple[int, int]] = [] - self.pairing.dispatcher_connect(self.process_new_events) - self.pairing.dispatcher_connect_config_changed(self.process_config_changed) - @property def entity_map(self) -> Accessories: """Return the accessories from the pairing.""" @@ -182,32 +180,15 @@ class HKDevice: self.available = available async_dispatcher_send(self.hass, self.signal_state_updated) - async def async_ensure_available(self) -> None: - """Verify the accessory is available after processing the entity map.""" - if self.available: - return - if self.watchable_characteristics and self.pollable_characteristics: - # We already tried, no need to try again - return - # We there are no watchable and not pollable characteristics, - # we need to force a connection to the device to verify its alive. - # - # This is similar to iOS's behavior for keeping alive connections - # to cameras. - # - primary = self.entity_map.accessories[0] - aid = primary.aid - iid = primary.accessory_information[CharacteristicsTypes.SERIAL_NUMBER].iid - await self.pairing.get_characteristics([(aid, iid)]) - self.async_set_available_state(True) - async def async_setup(self) -> None: """Prepare to use a paired HomeKit device in Home Assistant.""" entity_storage: EntityMapStorage = self.hass.data[ENTITY_MAP] + pairing = self.pairing + transport = pairing.transport + entry = self.config_entry + if cache := entity_storage.get_map(self.unique_id): - self.pairing.restore_accessories_state( - cache["accessories"], cache["config_num"] - ) + pairing.restore_accessories_state(cache["accessories"], cache["config_num"]) # We need to force an update here to make sure we have # the latest values since the async_update we do in @@ -219,22 +200,47 @@ class HKDevice: # Ideally we would know which entities we are about to add # so we only poll those chars but that is not possible # yet. - await self.pairing.async_populate_accessories_state(force_update=True) + try: + await self.pairing.async_populate_accessories_state(force_update=True) + except AccessoryNotFoundError: + if transport != Transport.BLE or not cache: + # BLE devices may sleep and we can't force a connection + raise + + entry.async_on_unload(pairing.dispatcher_connect(self.process_new_events)) + entry.async_on_unload( + pairing.dispatcher_connect_config_changed(self.process_config_changed) + ) + entry.async_on_unload( + pairing.dispatcher_availability_changed(self.async_set_available_state) + ) await self.async_process_entity_map() - await self.async_ensure_available() - if not cache: # If its missing from the cache, make sure we save it self.async_save_entity_map() # If everything is up to date, we can create the entities # since we know the data is not stale. await self.async_add_new_entities() + + self.async_set_available_state(self.pairing.is_available) + self._polling_interval_remover = async_track_time_interval( - self.hass, self.async_update, DEFAULT_SCAN_INTERVAL + self.hass, self.async_update, self.pairing.poll_interval ) + if transport == Transport.BLE: + # If we are using BLE, we need to periodically check of the + # BLE device is available since we won't get callbacks + # when it goes away since we HomeKit supports disconnected + # notifications and we cannot treat a disconnect as unavailability. + self._ble_available_interval_remover = async_track_time_interval( + self.hass, + self.async_update_available_state, + timedelta(seconds=BLE_AVAILABILITY_CHECK_INTERVAL), + ) + async def async_add_new_entities(self) -> None: """Add new entities to Home Assistant.""" await self.async_load_platforms() @@ -546,10 +552,15 @@ class HKDevice: if tasks: await asyncio.gather(*tasks) + @callback + def async_update_available_state(self, *_: Any) -> None: + """Update the available state of the device.""" + self.async_set_available_state(self.pairing.is_available) + async def async_update(self, now=None): """Poll state of all entities attached to this bridge/accessory.""" if not self.pollable_characteristics: - self.async_set_available_state(self.pairing.is_connected) + self.async_update_available_state() _LOGGER.debug( "HomeKit connection not polling any characteristics: %s", self.unique_id ) diff --git a/homeassistant/components/homekit_controller/manifest.json b/homeassistant/components/homekit_controller/manifest.json index 17692476834..29268f3b6e9 100644 --- a/homeassistant/components/homekit_controller/manifest.json +++ b/homeassistant/components/homekit_controller/manifest.json @@ -3,7 +3,7 @@ "name": "HomeKit Controller", "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/homekit_controller", - "requirements": ["aiohomekit==1.1.4"], + "requirements": ["aiohomekit==1.1.5"], "zeroconf": ["_hap._tcp.local.", "_hap._udp.local."], "bluetooth": [{ "manufacturer_id": 76, "manufacturer_data_start": [6] }], "after_dependencies": ["zeroconf"], diff --git a/requirements_all.txt b/requirements_all.txt index 47ff7005c66..a8d87c8a557 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -168,7 +168,7 @@ aioguardian==2022.03.2 aioharmony==0.2.9 # homeassistant.components.homekit_controller -aiohomekit==1.1.4 +aiohomekit==1.1.5 # homeassistant.components.emulated_hue # homeassistant.components.http diff --git a/requirements_test_all.txt b/requirements_test_all.txt index a9d015738f5..ebfaabd16c3 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -152,7 +152,7 @@ aioguardian==2022.03.2 aioharmony==0.2.9 # homeassistant.components.homekit_controller -aiohomekit==1.1.4 +aiohomekit==1.1.5 # homeassistant.components.emulated_hue # homeassistant.components.http diff --git a/tests/components/homekit_controller/test_init.py b/tests/components/homekit_controller/test_init.py index df14ad325b6..37d41fcf372 100644 --- a/tests/components/homekit_controller/test_init.py +++ b/tests/components/homekit_controller/test_init.py @@ -3,15 +3,15 @@ from datetime import timedelta from unittest.mock import patch -from aiohomekit import AccessoryDisconnectedError -from aiohomekit.model import Accessory +from aiohomekit import AccessoryNotFoundError +from aiohomekit.model import Accessory, Transport from aiohomekit.model.characteristics import CharacteristicsTypes from aiohomekit.model.services import ServicesTypes from aiohomekit.testing import FakePairing from homeassistant.components.homekit_controller.const import DOMAIN, ENTITY_MAP from homeassistant.config_entries import ConfigEntryState -from homeassistant.const import EVENT_HOMEASSISTANT_STOP +from homeassistant.const import EVENT_HOMEASSISTANT_STOP, STATE_OFF, STATE_UNAVAILABLE from homeassistant.core import HomeAssistant from homeassistant.helpers import device_registry as dr, entity_registry as er from homeassistant.helpers.entity_registry import EntityRegistry @@ -104,31 +104,89 @@ async def test_offline_device_raises(hass, controller): is_connected = False class OfflineFakePairing(FakePairing): - """Fake pairing that always returns False for is_connected.""" + """Fake pairing that can flip is_connected.""" @property def is_connected(self): nonlocal is_connected return is_connected + @property + def is_available(self): + return self.is_connected + async def async_populate_accessories_state(self, *args, **kwargs): nonlocal is_connected if not is_connected: - raise AccessoryDisconnectedError("any") + raise AccessoryNotFoundError("any") async def get_characteristics(self, chars, *args, **kwargs): nonlocal is_connected if not is_connected: - raise AccessoryDisconnectedError("any") + raise AccessoryNotFoundError("any") return {} + accessory = Accessory.create_with_info( + "TestDevice", "example.com", "Test", "0001", "0.1" + ) + create_alive_service(accessory) + with patch("aiohomekit.testing.FakePairing", OfflineFakePairing): await async_setup_component(hass, DOMAIN, {}) - accessory = Accessory.create_with_info( - "TestDevice", "example.com", "Test", "0001", "0.1" + config_entry, _ = await setup_test_accessories_with_controller( + hass, [accessory], controller ) - create_alive_service(accessory) + await hass.async_block_till_done() + + assert config_entry.state == ConfigEntryState.SETUP_RETRY + + is_connected = True + + async_fire_time_changed(hass, utcnow() + timedelta(seconds=10)) + await hass.async_block_till_done() + assert config_entry.state == ConfigEntryState.LOADED + assert hass.states.get("light.testdevice").state == STATE_OFF + + +async def test_ble_device_only_checks_is_available(hass, controller): + """Test a BLE device only checks is_available.""" + + is_available = False + + class FakeBLEPairing(FakePairing): + """Fake BLE pairing that can flip is_available.""" + @property + def transport(self): + return Transport.BLE + + @property + def is_connected(self): + return False + + @property + def is_available(self): + nonlocal is_available + return is_available + + async def async_populate_accessories_state(self, *args, **kwargs): + nonlocal is_available + if not is_available: + raise AccessoryNotFoundError("any") + + async def get_characteristics(self, chars, *args, **kwargs): + nonlocal is_available + if not is_available: + raise AccessoryNotFoundError("any") + return {} + + accessory = Accessory.create_with_info( + "TestDevice", "example.com", "Test", "0001", "0.1" + ) + create_alive_service(accessory) + + with patch("aiohomekit.testing.FakePairing", FakeBLEPairing): + await async_setup_component(hass, DOMAIN, {}) config_entry, _ = await setup_test_accessories_with_controller( hass, [accessory], controller ) @@ -136,8 +194,17 @@ async def test_offline_device_raises(hass, controller): assert config_entry.state == ConfigEntryState.SETUP_RETRY - is_connected = True + is_available = True async_fire_time_changed(hass, utcnow() + timedelta(seconds=10)) await hass.async_block_till_done() assert config_entry.state == ConfigEntryState.LOADED + assert hass.states.get("light.testdevice").state == STATE_OFF + + is_available = False + async_fire_time_changed(hass, utcnow() + timedelta(hours=1)) + assert hass.states.get("light.testdevice").state == STATE_UNAVAILABLE + + is_available = True + async_fire_time_changed(hass, utcnow() + timedelta(hours=1)) + assert hass.states.get("light.testdevice").state == STATE_OFF -- GitLab