diff --git a/homeassistant/components/homekit_controller/__init__.py b/homeassistant/components/homekit_controller/__init__.py index 9175dca50bc..37dd648dedb 100644 --- a/homeassistant/components/homekit_controller/__init__.py +++ b/homeassistant/components/homekit_controller/__init__.py @@ -6,6 +6,11 @@ import logging from typing import Any import aiohomekit +from aiohomekit.exceptions import ( + AccessoryDisconnectedError, + AccessoryNotFoundError, + EncryptionError, +) from aiohomekit.model import Accessory from aiohomekit.model.characteristics import ( Characteristic, @@ -26,7 +31,7 @@ from homeassistant.helpers.typing import ConfigType from .config_flow import normalize_hkid from .connection import HKDevice, valid_serial_number from .const import ENTITY_MAP, KNOWN_DEVICES, TRIGGERS -from .storage import EntityMapStorage +from .storage import async_get_entity_storage from .utils import async_get_controller, folded_name _LOGGER = logging.getLogger(__name__) @@ -227,23 +232,19 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: entry, unique_id=normalize_hkid(conn.unique_id) ) - if not await conn.async_setup(): + try: + await conn.async_setup() + except (AccessoryNotFoundError, EncryptionError, AccessoryDisconnectedError) as ex: del hass.data[KNOWN_DEVICES][conn.unique_id] - if (connection := getattr(conn.pairing, "connection", None)) and hasattr( - connection, "host" - ): - raise ConfigEntryNotReady( - f"Cannot connect to {connection.host}:{connection.port}" - ) - raise ConfigEntryNotReady + await conn.pairing.close() + raise ConfigEntryNotReady from ex return True async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: """Set up for Homekit devices.""" - map_storage = hass.data[ENTITY_MAP] = EntityMapStorage(hass) - await map_storage.async_initialize() + await async_get_entity_storage(hass) await async_get_controller(hass) diff --git a/homeassistant/components/homekit_controller/config_flow.py b/homeassistant/components/homekit_controller/config_flow.py index 41094a65e00..9b8b759f80e 100644 --- a/homeassistant/components/homekit_controller/config_flow.py +++ b/homeassistant/components/homekit_controller/config_flow.py @@ -8,7 +8,6 @@ from typing import Any import aiohomekit from aiohomekit.controller.abstract import AbstractPairing from aiohomekit.exceptions import AuthenticationError -from aiohomekit.model import Accessories, CharacteristicsTypes, ServicesTypes from aiohomekit.utils import domain_supported, domain_to_name import voluptuous as vol @@ -20,6 +19,7 @@ from homeassistant.helpers import device_registry as dr from .connection import HKDevice from .const import DOMAIN, KNOWN_DEVICES +from .storage import async_get_entity_storage from .utils import async_get_controller HOMEKIT_DIR = ".homekit" @@ -252,9 +252,7 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): _LOGGER.debug( "HomeKit info %s: c# incremented, refreshing entities", hkid ) - self.hass.async_create_task( - conn.async_refresh_entity_map_and_entities(config_num) - ) + conn.async_notify_config_changed(config_num) return self.async_abort(reason="already_configured") _LOGGER.debug("Discovered device %s (%s - %s)", name, model, hkid) @@ -481,17 +479,22 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): # available. Otherwise request a fresh copy from the API. # This removes the 'accessories' key from pairing_data at # the same time. - if not (accessories := pairing_data.pop("accessories", None)): - accessories = await pairing.list_accessories_and_characteristics() - - parsed = Accessories.from_list(accessories) - accessory_info = parsed.aid(1).services.first( - service_type=ServicesTypes.ACCESSORY_INFORMATION - ) - name = accessory_info.value(CharacteristicsTypes.NAME, "") + name = await pairing.get_primary_name() await pairing.close() + # Save the state of the accessories so we do not + # have to request them again when we setup the + # config entry. + accessories_state = pairing.accessories_state + entity_storage = await async_get_entity_storage(self.hass) + assert self.unique_id is not None + entity_storage.async_create_or_update_map( + self.unique_id, + accessories_state.config_num, + accessories_state.accessories.serialize(), + ) + return self.async_create_entry(title=name, data=pairing_data) diff --git a/homeassistant/components/homekit_controller/connection.py b/homeassistant/components/homekit_controller/connection.py index 85e8e3987bd..7e4dcc59be0 100644 --- a/homeassistant/components/homekit_controller/connection.py +++ b/homeassistant/components/homekit_controller/connection.py @@ -5,6 +5,7 @@ import asyncio from collections.abc import Callable import datetime import logging +from types import MappingProxyType from typing import Any from aiohomekit import Controller @@ -17,8 +18,9 @@ from aiohomekit.model import Accessories, Accessory from aiohomekit.model.characteristics import Characteristic, CharacteristicsTypes from aiohomekit.model.services import Service +from homeassistant.config_entries import ConfigEntry from homeassistant.const import ATTR_VIA_DEVICE -from homeassistant.core import CALLBACK_TYPE, callback +from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback from homeassistant.helpers import device_registry as dr from homeassistant.helpers.dispatcher import async_dispatcher_send from homeassistant.helpers.entity import DeviceInfo @@ -62,7 +64,12 @@ def valid_serial_number(serial: str) -> bool: class HKDevice: """HomeKit device.""" - def __init__(self, hass, config_entry, pairing_data) -> None: + def __init__( + self, + hass: HomeAssistant, + config_entry: ConfigEntry, + pairing_data: MappingProxyType[str, Any], + ) -> None: """Initialise a generic HomeKit device.""" self.hass = hass @@ -78,11 +85,6 @@ class HKDevice: self.pairing_data["AccessoryPairingID"], self.pairing_data ) - self.accessories: list[Any] | None = None - self.config_num = 0 - - self.entity_map = Accessories() - # A list of callbacks that turn HK accessories into entities self.accessory_factories: list[AddAccessoryCb] = [] @@ -132,6 +134,17 @@ 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.""" + return self.pairing.accessories_state.accessories + + @property + def config_num(self) -> int: + """Return the config num from the pairing.""" + return self.pairing.accessories_state.config_num def add_pollable_characteristics( self, characteristics: list[tuple[int, int]] @@ -169,13 +182,13 @@ class HKDevice: self.available = available async_dispatcher_send(self.hass, self.signal_state_updated) - async def async_ensure_available(self) -> bool: + async def async_ensure_available(self) -> None: """Verify the accessory is available after processing the entity map.""" if self.available: - return True + return if self.watchable_characteristics and self.pollable_characteristics: # We already tried, no need to try again - return False + return # We there are no watchable and not pollable characteristics, # we need to force a connection to the device to verify its alive. # @@ -185,34 +198,42 @@ class HKDevice: primary = self.entity_map.accessories[0] aid = primary.aid iid = primary.accessory_information[CharacteristicsTypes.SERIAL_NUMBER].iid - try: - await self.pairing.get_characteristics([(aid, iid)]) - except (AccessoryDisconnectedError, EncryptionError, AccessoryNotFoundError): - return False + await self.pairing.get_characteristics([(aid, iid)]) self.async_set_available_state(True) - return True - async def async_setup(self) -> bool: + async def async_setup(self) -> None: """Prepare to use a paired HomeKit device in Home Assistant.""" entity_storage: EntityMapStorage = self.hass.data[ENTITY_MAP] if cache := entity_storage.get_map(self.unique_id): - self.accessories = cache["accessories"] - self.config_num = cache["config_num"] - self.entity_map = Accessories.from_list(self.accessories) - elif not await self.async_refresh_entity_map(self.config_num): - return False + self.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 + # async_process_entity_map will no values to poll yet + # since entities are added via dispatching and then + # they add the chars they are concerned about in + # async_added_to_hass which is too late. + # + # 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) await self.async_process_entity_map() - if not await self.async_ensure_available(): - return False + 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._polling_interval_remover = async_track_time_interval( self.hass, self.async_update, DEFAULT_SCAN_INTERVAL ) - return True async def async_add_new_entities(self) -> None: """Add new entities to Home Assistant.""" @@ -390,9 +411,6 @@ class HKDevice: # Ensure the Pairing object has access to the latest version of the entity map. This # is especially important for BLE, as the Pairing instance relies on the entity map # to map aid/iid to GATT characteristics. So push it to there as well. - - self.pairing.pairing_data["accessories"] = self.accessories # type: ignore[attr-defined] - self.async_detect_workarounds() # Migrate to new device ids @@ -403,13 +421,6 @@ class HKDevice: # Load any triggers for this config entry await async_setup_triggers_for_entry(self.hass, self.config_entry) - if self.watchable_characteristics: - await self.pairing.subscribe(self.watchable_characteristics) - if not self.pairing.is_connected: - return - - await self.async_update() - async def async_unload(self) -> None: """Stop interacting with device and prepare for removal from hass.""" if self._polling_interval_remover: @@ -421,34 +432,31 @@ class HKDevice: self.config_entry, self.platforms ) - async def async_refresh_entity_map_and_entities(self, config_num: int) -> None: - """Refresh the entity map and entities for this pairing.""" - await self.async_refresh_entity_map(config_num) + def async_notify_config_changed(self, config_num: int) -> None: + """Notify the pairing of a config change.""" + self.pairing.notify_config_changed(config_num) + + def process_config_changed(self, config_num: int) -> None: + """Handle a config change notification from the pairing.""" + self.hass.async_create_task(self.async_update_new_accessories_state()) + + async def async_update_new_accessories_state(self) -> None: + """Process a change in the pairings accessories state.""" + self.async_save_entity_map() await self.async_process_entity_map() + if self.watchable_characteristics: + await self.pairing.subscribe(self.watchable_characteristics) + await self.async_update() await self.async_add_new_entities() - async def async_refresh_entity_map(self, config_num: int) -> bool: - """Handle setup of a HomeKit accessory.""" - try: - self.accessories = await self.pairing.list_accessories_and_characteristics() - except AccessoryDisconnectedError: - # If we fail to refresh this data then we will naturally retry - # later when Bonjour spots c# is still not up to date. - return False - - assert self.accessories is not None - - self.entity_map = Accessories.from_list(self.accessories) - + @callback + def async_save_entity_map(self) -> None: + """Save the entity map.""" entity_storage: EntityMapStorage = self.hass.data[ENTITY_MAP] - entity_storage.async_create_or_update_map( - self.unique_id, config_num, self.accessories + self.unique_id, self.config_num, self.entity_map.serialize() ) - self.config_num = config_num - return True - def add_accessory_factory(self, add_entities_cb) -> None: """Add a callback to run when discovering new entities for accessories.""" self.accessory_factories.append(add_entities_cb) diff --git a/homeassistant/components/homekit_controller/device_trigger.py b/homeassistant/components/homekit_controller/device_trigger.py index 49924b30c57..d7828753d98 100644 --- a/homeassistant/components/homekit_controller/device_trigger.py +++ b/homeassistant/components/homekit_controller/device_trigger.py @@ -15,6 +15,7 @@ from homeassistant.components.automation import ( AutomationTriggerInfo, ) from homeassistant.components.device_automation import DEVICE_TRIGGER_BASE_SCHEMA +from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_DEVICE_ID, CONF_DOMAIN, CONF_PLATFORM, CONF_TYPE from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback from homeassistant.helpers.typing import ConfigType @@ -195,7 +196,9 @@ TRIGGER_FINDERS = { } -async def async_setup_triggers_for_entry(hass: HomeAssistant, config_entry): +async def async_setup_triggers_for_entry( + hass: HomeAssistant, config_entry: ConfigEntry +) -> None: """Triggers aren't entities as they have no state, but we still need to set them up for a config entry.""" hkid = config_entry.data["AccessoryPairingID"] conn: HKDevice = hass.data[KNOWN_DEVICES][hkid] diff --git a/homeassistant/components/homekit_controller/diagnostics.py b/homeassistant/components/homekit_controller/diagnostics.py index f83ce7604cf..9b17c0c2fe7 100644 --- a/homeassistant/components/homekit_controller/diagnostics.py +++ b/homeassistant/components/homekit_controller/diagnostics.py @@ -107,6 +107,7 @@ def _async_get_diagnostics( # It is roughly equivalent to what is in .storage/homekit_controller-entity-map # But it also has the latest values seen by the polling or events data["entity-map"] = accessories = connection.entity_map.serialize() + data["config-num"] = connection.config_num # It contains serial numbers, which we should strip out for accessory in accessories: diff --git a/homeassistant/components/homekit_controller/manifest.json b/homeassistant/components/homekit_controller/manifest.json index 0275afa07fe..a5ca76fdc1e 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.0.0"], + "requirements": ["aiohomekit==1.1.1"], "zeroconf": ["_hap._tcp.local.", "_hap._udp.local."], "after_dependencies": ["zeroconf"], "codeowners": ["@Jc2k", "@bdraco"], diff --git a/homeassistant/components/homekit_controller/storage.py b/homeassistant/components/homekit_controller/storage.py index ff39c52627e..8c0628c97f6 100644 --- a/homeassistant/components/homekit_controller/storage.py +++ b/homeassistant/components/homekit_controller/storage.py @@ -7,7 +7,7 @@ from typing import Any, TypedDict from homeassistant.core import HomeAssistant, callback from homeassistant.helpers.storage import Store -from .const import DOMAIN +from .const import DOMAIN, ENTITY_MAP ENTITY_MAP_STORAGE_KEY = f"{DOMAIN}-entity-map" ENTITY_MAP_STORAGE_VERSION = 1 @@ -91,3 +91,13 @@ class EntityMapStorage: def _data_to_save(self) -> StorageLayout: """Return data of entity map to store in a file.""" return StorageLayout(pairings=self.storage_data) + + +async def async_get_entity_storage(hass: HomeAssistant) -> EntityMapStorage: + """Get entity storage.""" + if ENTITY_MAP in hass.data: + map_storage: EntityMapStorage = hass.data[ENTITY_MAP] + return map_storage + map_storage = hass.data[ENTITY_MAP] = EntityMapStorage(hass) + await map_storage.async_initialize() + return map_storage diff --git a/requirements_all.txt b/requirements_all.txt index 63fd2862faa..4fc5e180e95 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.0.0 +aiohomekit==1.1.1 # homeassistant.components.emulated_hue # homeassistant.components.http diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 034ead5c35e..ff34193cee8 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.0.0 +aiohomekit==1.1.1 # homeassistant.components.emulated_hue # homeassistant.components.http diff --git a/tests/components/homekit_controller/common.py b/tests/components/homekit_controller/common.py index e773b2ffc66..18367d28f63 100644 --- a/tests/components/homekit_controller/common.py +++ b/tests/components/homekit_controller/common.py @@ -9,7 +9,7 @@ import os from typing import Any, Final from unittest import mock -from aiohomekit.model import Accessories, Accessory +from aiohomekit.model import Accessories, AccessoriesState, Accessory from aiohomekit.testing import FakeController, FakePairing from homeassistant.components import zeroconf @@ -225,7 +225,9 @@ async def device_config_changed(hass, accessories): accessories_obj = Accessories() for accessory in accessories: accessories_obj.add_accessory(accessory) - pairing.accessories = accessories_obj + pairing._accessories_state = AccessoriesState( + accessories_obj, pairing.config_num + 1 + ) discovery_info = zeroconf.ZeroconfServiceInfo( host="127.0.0.1", diff --git a/tests/components/homekit_controller/test_config_flow.py b/tests/components/homekit_controller/test_config_flow.py index 9535b7d0cd5..73bd159fd73 100644 --- a/tests/components/homekit_controller/test_config_flow.py +++ b/tests/components/homekit_controller/test_config_flow.py @@ -2,7 +2,7 @@ import asyncio from unittest import mock import unittest.mock -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import aiohomekit from aiohomekit.exceptions import AuthenticationError @@ -492,7 +492,7 @@ async def test_discovery_already_configured_update_csharp(hass, controller): connection_mock = AsyncMock() connection_mock.pairing.connect.reconnect_soon = AsyncMock() - connection_mock.async_refresh_entity_map = AsyncMock() + connection_mock.async_notify_config_changed = MagicMock() hass.data[KNOWN_DEVICES] = {"AA:BB:CC:DD:EE:FF": connection_mock} device = setup_mock_accessory(controller) @@ -515,7 +515,7 @@ async def test_discovery_already_configured_update_csharp(hass, controller): assert entry.data["AccessoryIP"] == discovery_info.host assert entry.data["AccessoryPort"] == discovery_info.port - assert connection_mock.async_refresh_entity_map_and_entities.await_count == 1 + assert connection_mock.async_notify_config_changed.call_count == 1 @pytest.mark.parametrize("exception,expected", PAIRING_START_ABORT_ERRORS) diff --git a/tests/components/homekit_controller/test_diagnostics.py b/tests/components/homekit_controller/test_diagnostics.py index e770279752e..dd0e35b0d1e 100644 --- a/tests/components/homekit_controller/test_diagnostics.py +++ b/tests/components/homekit_controller/test_diagnostics.py @@ -28,6 +28,7 @@ async def test_config_entry(hass: HomeAssistant, hass_client: ClientSession, utc "version": 1, "data": {"AccessoryPairingID": "00:00:00:00:00:00"}, }, + "config-num": 0, "entity-map": [ { "aid": 1, @@ -299,6 +300,7 @@ async def test_device(hass: HomeAssistant, hass_client: ClientSession, utcnow): "version": 1, "data": {"AccessoryPairingID": "00:00:00:00:00:00"}, }, + "config-num": 0, "entity-map": [ { "aid": 1, diff --git a/tests/components/homekit_controller/test_init.py b/tests/components/homekit_controller/test_init.py index 57b39076aa6..df14ad325b6 100644 --- a/tests/components/homekit_controller/test_init.py +++ b/tests/components/homekit_controller/test_init.py @@ -111,8 +111,16 @@ async def test_offline_device_raises(hass, controller): nonlocal is_connected return is_connected - def get_characteristics(self, chars, *args, **kwargs): - raise AccessoryDisconnectedError("any") + async def async_populate_accessories_state(self, *args, **kwargs): + nonlocal is_connected + if not is_connected: + raise AccessoryDisconnectedError("any") + + async def get_characteristics(self, chars, *args, **kwargs): + nonlocal is_connected + if not is_connected: + raise AccessoryDisconnectedError("any") + return {} with patch("aiohomekit.testing.FakePairing", OfflineFakePairing): await async_setup_component(hass, DOMAIN, {}) diff --git a/tests/components/homekit_controller/test_storage.py b/tests/components/homekit_controller/test_storage.py index b4ed617f901..13d613e3916 100644 --- a/tests/components/homekit_controller/test_storage.py +++ b/tests/components/homekit_controller/test_storage.py @@ -3,6 +3,7 @@ from aiohomekit.model.characteristics import CharacteristicsTypes from aiohomekit.model.services import ServicesTypes from homeassistant.components.homekit_controller.const import ENTITY_MAP +from homeassistant.components.homekit_controller.storage import EntityMapStorage from tests.common import flush_store from tests.components.homekit_controller.common import ( @@ -68,7 +69,7 @@ async def test_storage_is_updated_on_add(hass, hass_storage, utcnow): """Test entity map storage is cleaned up on adding an accessory.""" await setup_test_component(hass, create_lightbulb_service) - entity_map = hass.data[ENTITY_MAP] + entity_map: EntityMapStorage = hass.data[ENTITY_MAP] hkid = "00:00:00:00:00:00" # Is in memory store updated?