From 4ac9a2e9debc11532b9b1472916ce4e12d4b7032 Mon Sep 17 00:00:00 2001 From: Jc2k Date: Thu, 18 Apr 2019 16:55:34 +0100 Subject: [PATCH] Add storage for cacheable homekit entity maps. (#23191) --- .../components/homekit_controller/__init__.py | 28 +++-- .../homekit_controller/config_flow.py | 2 +- .../homekit_controller/connection.py | 100 +++++++++++++--- .../components/homekit_controller/const.py | 1 + .../components/homekit_controller/storage.py | 80 +++++++++++++ tests/components/homekit_controller/common.py | 1 + .../specific_devices/test_ecobee3.py | 76 +++++++++++- .../homekit_controller/test_config_flow.py | 2 +- .../homekit_controller/test_storage.py | 112 ++++++++++++++++++ 9 files changed, 373 insertions(+), 29 deletions(-) create mode 100644 homeassistant/components/homekit_controller/storage.py create mode 100644 tests/components/homekit_controller/test_storage.py diff --git a/homeassistant/components/homekit_controller/__init__.py b/homeassistant/components/homekit_controller/__init__.py index 11026d7e9ac..3fa4ade519e 100644 --- a/homeassistant/components/homekit_controller/__init__.py +++ b/homeassistant/components/homekit_controller/__init__.py @@ -8,9 +8,10 @@ from homeassistant.helpers.entity import Entity from .config_flow import load_old_pairings from .connection import get_accessory_information, HKDevice from .const import ( - CONTROLLER, KNOWN_DEVICES + CONTROLLER, ENTITY_MAP, KNOWN_DEVICES ) from .const import DOMAIN # noqa: pylint: disable=unused-import +from .storage import EntityMapStorage HOMEKIT_IGNORE = [ 'BSB002', @@ -44,7 +45,7 @@ class HomeKitEntity(Entity): # pylint: disable=import-error from homekit.model.characteristics import CharacteristicsTypes - pairing_data = self._accessory.pairing.pairing_data + accessories = self._accessory.accessories get_uuid = CharacteristicsTypes.get_uuid characteristic_types = [ @@ -55,7 +56,7 @@ class HomeKitEntity(Entity): self._chars = {} self._char_names = {} - for accessory in pairing_data.get('accessories', []): + for accessory in accessories: if accessory['aid'] != self._aid: continue self._accessory_info = get_accessory_information(accessory) @@ -149,12 +150,15 @@ class HomeKitEntity(Entity): raise NotImplementedError -def setup(hass, config): +async def async_setup(hass, config): """Set up for Homekit devices.""" # pylint: disable=import-error import homekit from homekit.controller.ip_implementation import IpPairing + map_storage = hass.data[ENTITY_MAP] = EntityMapStorage(hass) + await map_storage.async_initialize() + hass.data[CONTROLLER] = controller = homekit.Controller() for hkid, pairing_data in load_old_pairings(hass).items(): @@ -185,12 +189,22 @@ def setup(hass, config): device = hass.data[KNOWN_DEVICES][hkid] if config_num > device.config_num and \ device.pairing is not None: - device.accessory_setup() + device.refresh_entity_map(config_num) return _LOGGER.debug('Discovered unique device %s', hkid) - HKDevice(hass, host, port, model, hkid, config_num, config) + device = HKDevice(hass, host, port, model, hkid, config_num, config) + device.setup() hass.data[KNOWN_DEVICES] = {} - discovery.listen(hass, SERVICE_HOMEKIT, discovery_dispatch) + + await hass.async_add_executor_job( + discovery.listen, hass, SERVICE_HOMEKIT, discovery_dispatch) + return True + + +async def async_remove_entry(hass, entry): + """Cleanup caches before removing config entry.""" + hkid = entry.data['AccessoryPairingID'] + hass.data[ENTITY_MAP].async_delete_map(hkid) diff --git a/homeassistant/components/homekit_controller/config_flow.py b/homeassistant/components/homekit_controller/config_flow.py index 1cd66896fe2..a6c5ac8b36d 100644 --- a/homeassistant/components/homekit_controller/config_flow.py +++ b/homeassistant/components/homekit_controller/config_flow.py @@ -152,7 +152,7 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow): "HomeKit info %s: c# incremented, refreshing entities", hkid) self.hass.async_create_task( - conn.async_config_num_changed(config_num)) + conn.async_refresh_entity_map(config_num)) return self.async_abort(reason='already_configured') old_pairings = await self.hass.async_add_executor_job( diff --git a/homeassistant/components/homekit_controller/connection.py b/homeassistant/components/homekit_controller/connection.py index 2ca568b547f..2b82370d187 100644 --- a/homeassistant/components/homekit_controller/connection.py +++ b/homeassistant/components/homekit_controller/connection.py @@ -4,11 +4,10 @@ import logging import os from homeassistant.helpers import discovery -from homeassistant.helpers.event import call_later from .const import ( CONTROLLER, DOMAIN, HOMEKIT_ACCESSORY_DISPATCH, KNOWN_DEVICES, - PAIRING_FILE, HOMEKIT_DIR + PAIRING_FILE, HOMEKIT_DIR, ENTITY_MAP ) @@ -67,7 +66,7 @@ class HKDevice(): self.config_num = config_num self.config = config self.configurator = hass.components.configurator - self._connection_warning_logged = False + self.accessories = {} # This just tracks aid/iid pairs so we know if a HK service has been # mapped to a HA entity. @@ -79,27 +78,77 @@ class HKDevice(): hass.data[KNOWN_DEVICES][hkid] = self - if self.pairing is not None: - self.accessory_setup() - else: + def setup(self): + """Prepare to use a paired HomeKit device in homeassistant.""" + if self.pairing is None: self.configure() - - def accessory_setup(self): - """Handle setup of a HomeKit accessory.""" - # pylint: disable=import-error - from homekit.model.services import ServicesTypes - from homekit.exceptions import AccessoryDisconnectedError + return self.pairing.pairing_data['AccessoryIP'] = self.host self.pairing.pairing_data['AccessoryPort'] = self.port + cache = self.hass.data[ENTITY_MAP].get_map(self.unique_id) + if not cache or cache['config_num'] < self.config_num: + return self.refresh_entity_map(self.config_num) + + self.accessories = cache['accessories'] + + # Ensure the Pairing object has access to the latest version of the + # entity map. + self.pairing.pairing_data['accessories'] = self.accessories + + self.add_entities() + + return True + + async def async_refresh_entity_map(self, config_num): + """ + Handle setup of a HomeKit accessory. + + The sync version will be removed when homekit_controller migrates to + config flow. + """ + return await self.hass.async_add_executor_job( + self.refresh_entity_map, + config_num, + ) + + def refresh_entity_map(self, config_num): + """Handle setup of a HomeKit accessory.""" + # pylint: disable=import-error + from homekit.exceptions import AccessoryDisconnectedError + try: - data = self.pairing.list_accessories_and_characteristics() + accessories = self.pairing.list_accessories_and_characteristics() except AccessoryDisconnectedError: - call_later( - self.hass, RETRY_INTERVAL, lambda _: self.accessory_setup()) + # If we fail to refresh this data then we will naturally retry + # later when Bonjour spots c# is still not up to date. return - for accessory in data: + + self.hass.data[ENTITY_MAP].async_create_or_update_map( + self.unique_id, + config_num, + accessories, + ) + + self.accessories = accessories + self.config_num = config_num + + # For BLE, 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'] = accessories + + # Register add new entities that are available + self.add_entities() + + return True + + def add_entities(self): + """Process the entity map and create HA entities.""" + # pylint: disable=import-error + from homekit.model.services import ServicesTypes + + for accessory in self.accessories: aid = accessory['aid'] for service in accessory['services']: iid = service['iid'] @@ -118,6 +167,7 @@ class HKDevice(): if component is not None: discovery.load_platform(self.hass, component, DOMAIN, service_info, self.config) + self.entities.append((aid, iid)) def device_config_callback(self, callback_data): """Handle initial pairing.""" @@ -145,15 +195,20 @@ class HKDevice(): self.pairing = self.controller.pairings.get(self.hkid) if self.pairing is not None: - pairing_file = os.path.join( + pairing_dir = os.path.join( self.hass.config.path(), HOMEKIT_DIR, + ) + if not os.path.exists(pairing_dir): + os.makedirs(pairing_dir) + pairing_file = os.path.join( + pairing_dir, PAIRING_FILE, ) self.controller.save_data(pairing_file) _configurator = self.hass.data[DOMAIN+self.hkid] self.configurator.request_done(_configurator) - self.accessory_setup() + self.setup() else: error_msg = "Unable to pair, please try again" _configurator = self.hass.data[DOMAIN+self.hkid] @@ -197,3 +252,12 @@ class HKDevice(): self.pairing.put_characteristics, chars ) + + @property + def unique_id(self): + """ + Return a unique id for this accessory or bridge. + + This id is random and will change if a device undergoes a hard reset. + """ + return self.hkid diff --git a/homeassistant/components/homekit_controller/const.py b/homeassistant/components/homekit_controller/const.py index de9663f1202..f112737ca24 100644 --- a/homeassistant/components/homekit_controller/const.py +++ b/homeassistant/components/homekit_controller/const.py @@ -3,6 +3,7 @@ DOMAIN = 'homekit_controller' KNOWN_DEVICES = "{}-devices".format(DOMAIN) CONTROLLER = "{}-controller".format(DOMAIN) +ENTITY_MAP = '{}-entity-map'.format(DOMAIN) HOMEKIT_DIR = '.homekit' PAIRING_FILE = 'pairing.json' diff --git a/homeassistant/components/homekit_controller/storage.py b/homeassistant/components/homekit_controller/storage.py new file mode 100644 index 00000000000..4a7c0a8057b --- /dev/null +++ b/homeassistant/components/homekit_controller/storage.py @@ -0,0 +1,80 @@ +"""Helpers for HomeKit data stored in HA storage.""" + +from homeassistant.helpers.storage import Store +from homeassistant.core import callback + +from .const import DOMAIN + +ENTITY_MAP_STORAGE_KEY = '{}-entity-map'.format(DOMAIN) +ENTITY_MAP_STORAGE_VERSION = 1 +ENTITY_MAP_SAVE_DELAY = 10 + + +class EntityMapStorage: + """ + Holds a cache of entity structure data from a paired HomeKit device. + + HomeKit has a cacheable entity map that describes how an IP or BLE + endpoint is structured. This object holds the latest copy of that data. + + An endpoint is made of accessories, services and characteristics. It is + safe to cache this data until the c# discovery data changes. + + Caching this data means we can add HomeKit devices to HA immediately at + start even if discovery hasn't seen them yet or they are out of range. It + is also important for BLE devices - accessing the entity structure is + very slow for these devices. + """ + + def __init__(self, hass): + """Create a new entity map store.""" + self.hass = hass + self.store = Store( + hass, + ENTITY_MAP_STORAGE_VERSION, + ENTITY_MAP_STORAGE_KEY + ) + self.storage_data = {} + + async def async_initialize(self): + """Get the pairing cache data.""" + raw_storage = await self.store.async_load() + if not raw_storage: + # There is no cached data about HomeKit devices yet + return + + self.storage_data = raw_storage.get('pairings', {}) + + def get_map(self, homekit_id): + """Get a pairing cache item.""" + return self.storage_data.get(homekit_id) + + def async_create_or_update_map(self, homekit_id, config_num, accessories): + """Create a new pairing cache.""" + data = { + 'config_num': config_num, + 'accessories': accessories, + } + self.storage_data[homekit_id] = data + self._async_schedule_save() + return data + + def async_delete_map(self, homekit_id): + """Delete pairing cache.""" + if homekit_id not in self.storage_data: + return + + self.storage_data.pop(homekit_id) + self._async_schedule_save() + + @callback + def _async_schedule_save(self): + """Schedule saving the entity map cache.""" + self.store.async_delay_save(self._data_to_save, ENTITY_MAP_SAVE_DELAY) + + @callback + def _data_to_save(self): + """Return data of entity map to store in a file.""" + return { + 'pairings': self.storage_data, + } diff --git a/tests/components/homekit_controller/common.py b/tests/components/homekit_controller/common.py index 5ad197f8294..5d85fba6ae3 100644 --- a/tests/components/homekit_controller/common.py +++ b/tests/components/homekit_controller/common.py @@ -249,6 +249,7 @@ async def device_config_changed(hass, accessories): # Wait for services to reconfigure await hass.async_block_till_done() + await hass.async_block_till_done() async def setup_test_component(hass, services, capitalize=False, suffix=None): diff --git a/tests/components/homekit_controller/specific_devices/test_ecobee3.py b/tests/components/homekit_controller/specific_devices/test_ecobee3.py index 0831cd5b780..a7e449ddbe4 100644 --- a/tests/components/homekit_controller/specific_devices/test_ecobee3.py +++ b/tests/components/homekit_controller/specific_devices/test_ecobee3.py @@ -4,12 +4,16 @@ Regression tests for Ecobee 3. https://github.com/home-assistant/home-assistant/issues/15336 """ +from unittest import mock + +from homekit import AccessoryDisconnectedError + from homeassistant.components.climate.const import ( SUPPORT_TARGET_TEMPERATURE, SUPPORT_TARGET_HUMIDITY, SUPPORT_OPERATION_MODE) from tests.components.homekit_controller.common import ( - device_config_changed, setup_accessories_from_file, setup_test_accessories, - Helper + FakePairing, device_config_changed, setup_accessories_from_file, + setup_test_accessories, Helper ) @@ -46,6 +50,74 @@ async def test_ecobee3_setup(hass): assert occ3.unique_id == 'homekit-AB3C-56' +async def test_ecobee3_setup_from_cache(hass, hass_storage): + """Test that Ecbobee can be correctly setup from its cached entity map.""" + accessories = await setup_accessories_from_file(hass, 'ecobee3.json') + + hass_storage['homekit_controller-entity-map'] = { + 'version': 1, + 'data': { + 'pairings': { + '00:00:00:00:00:00': { + 'config_num': 1, + 'accessories': [ + a.to_accessory_and_service_list() for a in accessories + ], + } + } + } + } + + await setup_test_accessories(hass, accessories) + + entity_registry = await hass.helpers.entity_registry.async_get_registry() + + climate = entity_registry.async_get('climate.homew') + assert climate.unique_id == 'homekit-123456789012-16' + + occ1 = entity_registry.async_get('binary_sensor.kitchen') + assert occ1.unique_id == 'homekit-AB1C-56' + + occ2 = entity_registry.async_get('binary_sensor.porch') + assert occ2.unique_id == 'homekit-AB2C-56' + + occ3 = entity_registry.async_get('binary_sensor.basement') + assert occ3.unique_id == 'homekit-AB3C-56' + + +async def test_ecobee3_setup_connection_failure(hass): + """Test that Ecbobee can be correctly setup from its cached entity map.""" + accessories = await setup_accessories_from_file(hass, 'ecobee3.json') + + entity_registry = await hass.helpers.entity_registry.async_get_registry() + + # Test that the connection fails during initial setup. + # No entities should be created. + list_accessories = 'list_accessories_and_characteristics' + with mock.patch.object(FakePairing, list_accessories) as laac: + laac.side_effect = AccessoryDisconnectedError('Connection failed') + await setup_test_accessories(hass, accessories) + + climate = entity_registry.async_get('climate.homew') + assert climate is None + + # When a regular discovery event happens it should trigger another scan + # which should cause our entities to be added. + await device_config_changed(hass, accessories) + + climate = entity_registry.async_get('climate.homew') + assert climate.unique_id == 'homekit-123456789012-16' + + occ1 = entity_registry.async_get('binary_sensor.kitchen') + assert occ1.unique_id == 'homekit-AB1C-56' + + occ2 = entity_registry.async_get('binary_sensor.porch') + assert occ2.unique_id == 'homekit-AB2C-56' + + occ3 = entity_registry.async_get('binary_sensor.basement') + assert occ3.unique_id == 'homekit-AB3C-56' + + async def test_ecobee3_add_sensors_at_runtime(hass): """Test that new sensors are automatically added.""" entity_registry = await hass.helpers.entity_registry.async_get_registry() diff --git a/tests/components/homekit_controller/test_config_flow.py b/tests/components/homekit_controller/test_config_flow.py index 62c741b4eaa..da4176e1edc 100644 --- a/tests/components/homekit_controller/test_config_flow.py +++ b/tests/components/homekit_controller/test_config_flow.py @@ -295,7 +295,7 @@ async def test_discovery_already_configured_config_change(hass): assert result['type'] == 'abort' assert result['reason'] == 'already_configured' - assert conn.async_config_num_changed.call_args == mock.call(2) + assert conn.async_refresh_entity_map.call_args == mock.call(2) async def test_pair_unable_to_pair(hass): diff --git a/tests/components/homekit_controller/test_storage.py b/tests/components/homekit_controller/test_storage.py new file mode 100644 index 00000000000..43b8cba885a --- /dev/null +++ b/tests/components/homekit_controller/test_storage.py @@ -0,0 +1,112 @@ +"""Basic checks for entity map storage.""" +from tests.common import flush_store +from tests.components.homekit_controller.common import ( + FakeService, setup_test_component, setup_platform) + +from homeassistant import config_entries +from homeassistant.components.homekit_controller import async_remove_entry +from homeassistant.components.homekit_controller.const import ENTITY_MAP + + +async def test_load_from_storage(hass, hass_storage): + """Test that entity map can be correctly loaded from cache.""" + hkid = '00:00:00:00:00:00' + + hass_storage['homekit_controller-entity-map'] = { + 'version': 1, + 'data': { + 'pairings': { + hkid: { + 'c#': 1, + 'accessories': [], + } + } + } + } + + await setup_platform(hass) + assert hkid in hass.data[ENTITY_MAP].storage_data + + +async def test_storage_is_removed(hass, hass_storage): + """Test entity map storage removal is idempotent.""" + await setup_platform(hass) + + entity_map = hass.data[ENTITY_MAP] + hkid = '00:00:00:00:00:01' + + entity_map.async_create_or_update_map( + hkid, + 1, + [], + ) + assert hkid in entity_map.storage_data + await flush_store(entity_map.store) + assert hkid in hass_storage[ENTITY_MAP]['data']['pairings'] + + entity_map.async_delete_map(hkid) + assert hkid not in hass.data[ENTITY_MAP].storage_data + await flush_store(entity_map.store) + + assert hass_storage[ENTITY_MAP]['data']['pairings'] == {} + + +async def test_storage_is_removed_idempotent(hass): + """Test entity map storage removal is idempotent.""" + await setup_platform(hass) + + entity_map = hass.data[ENTITY_MAP] + hkid = '00:00:00:00:00:01' + + assert hkid not in entity_map.storage_data + + entity_map.async_delete_map(hkid) + + assert hkid not in entity_map.storage_data + + +def create_lightbulb_service(): + """Define lightbulb characteristics.""" + service = FakeService('public.hap.service.lightbulb') + on_char = service.add_characteristic('on') + on_char.value = 0 + return service + + +async def test_storage_is_updated_on_add(hass, hass_storage, utcnow): + """Test entity map storage is cleaned up on adding an accessory.""" + bulb = create_lightbulb_service() + await setup_test_component(hass, [bulb]) + + entity_map = hass.data[ENTITY_MAP] + hkid = '00:00:00:00:00:00' + + # Is in memory store updated? + assert hkid in entity_map.storage_data + + # Is saved out to store? + await flush_store(entity_map.store) + assert hkid in hass_storage[ENTITY_MAP]['data']['pairings'] + + +async def test_storage_is_removed_on_config_entry_removal(hass, utcnow): + """Test entity map storage is cleaned up on config entry removal.""" + bulb = create_lightbulb_service() + await setup_test_component(hass, [bulb]) + + hkid = '00:00:00:00:00:00' + + pairing_data = { + 'AccessoryPairingID': hkid, + } + + entry = config_entries.ConfigEntry( + 1, 'homekit_controller', 'TestData', pairing_data, + 'test', config_entries.CONN_CLASS_LOCAL_PUSH + ) + + assert hkid in hass.data[ENTITY_MAP].storage_data + + await async_remove_entry(hass, entry) + + assert hkid not in hass.data[ENTITY_MAP].storage_data