diff --git a/homeassistant/components/homekit_controller/__init__.py b/homeassistant/components/homekit_controller/__init__.py index 9651e497ccc..f44243e2fc0 100644 --- a/homeassistant/components/homekit_controller/__init__.py +++ b/homeassistant/components/homekit_controller/__init__.py @@ -1,6 +1,7 @@ """Support for Homekit device discovery.""" import logging +from homeassistant.core import callback from homeassistant.helpers.entity import Entity from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers import device_registry as dr @@ -27,7 +28,6 @@ class HomeKitEntity(Entity): def __init__(self, accessory, devinfo): """Initialise a generic HomeKit device.""" - self._available = True self._accessory = accessory self._aid = devinfo['aid'] self._iid = devinfo['iid'] @@ -35,6 +35,39 @@ class HomeKitEntity(Entity): self._chars = {} self.setup() + self._signals = [] + + async def async_added_to_hass(self): + """Entity added to hass.""" + self._signals.append( + self.hass.helpers.dispatcher.async_dispatcher_connect( + self._accessory.signal_state_updated, + self.async_state_changed, + ) + ) + + self._accessory.add_pollable_characteristics( + self.pollable_characteristics, + ) + + async def async_will_remove_from_hass(self): + """Prepare to be removed from hass.""" + self._accessory.remove_pollable_characteristics( + self._aid, + ) + + for signal_remove in self._signals: + signal_remove() + self._signals.clear() + + @property + def should_poll(self) -> bool: + """Return False. + + Data update is triggered from HKDevice. + """ + return False + def setup(self): """Configure an entity baed on its HomeKit characterstics metadata.""" # pylint: disable=import-error @@ -47,7 +80,7 @@ class HomeKitEntity(Entity): get_uuid(c) for c in self.get_characteristic_types() ] - self._chars_to_poll = [] + self.pollable_characteristics = [] self._chars = {} self._char_names = {} @@ -75,7 +108,7 @@ class HomeKitEntity(Entity): from homekit.model.characteristics import CharacteristicsTypes # Build up a list of (aid, iid) tuples to poll on update() - self._chars_to_poll.append((self._aid, char['iid'])) + self.pollable_characteristics.append((self._aid, char['iid'])) # Build a map of ctype -> iid short_name = CharacteristicsTypes.get_short(char['type']) @@ -91,30 +124,11 @@ class HomeKitEntity(Entity): # pylint: disable=not-callable setup_fn(char) - async def async_update(self): - """Obtain a HomeKit device's state.""" - # pylint: disable=import-error - from homekit.exceptions import ( - AccessoryDisconnectedError, AccessoryNotFoundError, - EncryptionError) - - try: - new_values_dict = await self._accessory.get_characteristics( - self._chars_to_poll - ) - except AccessoryNotFoundError: - # Not only did the connection fail, but also the accessory is not - # visible on the network. - self._available = False - return - except (AccessoryDisconnectedError, EncryptionError): - # Temporary connection failure. Device is still available but our - # connection was dropped. - return - - self._available = True - - for (_, iid), result in new_values_dict.items(): + @callback + def async_state_changed(self): + """Collect new data from bridge and update the entity state in hass.""" + accessory_state = self._accessory.current_state.get(self._aid, {}) + for iid, result in accessory_state.items(): if 'value' not in result: continue # Callback to update the entity with this characteristic value @@ -125,6 +139,8 @@ class HomeKitEntity(Entity): # pylint: disable=not-callable update_fn(result['value']) + self.async_write_ha_state() + @property def unique_id(self): """Return the ID of this device.""" @@ -139,7 +155,7 @@ class HomeKitEntity(Entity): @property def available(self) -> bool: """Return True if entity is available.""" - return self._available + return self._accessory.available @property def device_info(self): @@ -211,6 +227,17 @@ async def async_setup(hass, config): return True +async def async_unload_entry(hass, entry): + """Disconnect from HomeKit devices before unloading entry.""" + hkid = entry.data['AccessoryPairingID'] + + if hkid in hass.data[KNOWN_DEVICES]: + connection = hass.data[KNOWN_DEVICES][hkid] + await connection.async_unload() + + return True + + async def async_remove_entry(hass, entry): """Cleanup caches before removing config entry.""" hkid = entry.data['AccessoryPairingID'] diff --git a/homeassistant/components/homekit_controller/connection.py b/homeassistant/components/homekit_controller/connection.py index d0fc99de0d7..d46699eb6b5 100644 --- a/homeassistant/components/homekit_controller/connection.py +++ b/homeassistant/components/homekit_controller/connection.py @@ -1,10 +1,14 @@ """Helpers for managing a pairing with a HomeKit accessory or bridge.""" import asyncio +import datetime import logging -from .const import HOMEKIT_ACCESSORY_DISPATCH, ENTITY_MAP +from homeassistant.helpers.event import async_track_time_interval + +from .const import DOMAIN, HOMEKIT_ACCESSORY_DISPATCH, ENTITY_MAP +DEFAULT_SCAN_INTERVAL = datetime.timedelta(seconds=60) RETRY_INTERVAL = 60 # seconds _LOGGER = logging.getLogger(__name__) @@ -81,11 +85,54 @@ class HKDevice(): # allow one entity to use pairing at once. self.pairing_lock = asyncio.Lock() + self.available = True + + self.signal_state_updated = '_'.join(( + DOMAIN, + self.unique_id, + 'state_updated', + )) + + # Current values of all characteristics homekit_controller is tracking. + # Key is a (accessory_id, characteristic_id) tuple. + self.current_state = {} + + self.pollable_characteristics = [] + + # If this is set polling is active and can be disabled by calling + # this method. + self._polling_interval_remover = None + + def add_pollable_characteristics(self, characteristics): + """Add (aid, iid) pairs that we need to poll.""" + self.pollable_characteristics.extend(characteristics) + + def remove_pollable_characteristics(self, accessory_id): + """Remove all pollable characteristics by accessory id.""" + self.pollable_characteristics = [ + char for char in self.pollable_characteristics + if char[0] != accessory_id + ] + + def async_set_unavailable(self): + """Mark state of all entities on this connection as unavailable.""" + self.available = False + self.hass.helpers.dispatcher.async_dispatcher_send( + self.signal_state_updated, + ) + async def async_setup(self): """Prepare to use a paired HomeKit device in homeassistant.""" cache = self.hass.data[ENTITY_MAP].get_map(self.unique_id) if not cache: - return await self.async_refresh_entity_map(self.config_num) + if await self.async_refresh_entity_map(self.config_num): + self._polling_interval_remover = async_track_time_interval( + self.hass, + self.async_update, + DEFAULT_SCAN_INTERVAL + ) + return True + return False self.accessories = cache['accessories'] self.config_num = cache['config_num'] @@ -98,8 +145,34 @@ class HKDevice(): self.add_entities() + await self.async_update() + + self._polling_interval_remover = async_track_time_interval( + self.hass, + self.async_update, + DEFAULT_SCAN_INTERVAL + ) + return True + async def async_unload(self): + """Stop interacting with device and prepare for removal from hass.""" + if self._polling_interval_remover: + self._polling_interval_remover() + + unloads = [] + for platform in self.platforms: + unloads.append( + self.hass.config_entries.async_forward_entry_unload( + self.config_entry, + platform + ) + ) + + results = await asyncio.gather(*unloads) + + return False not in results + async def async_refresh_entity_map(self, config_num): """Handle setup of a HomeKit accessory.""" # pylint: disable=import-error @@ -132,6 +205,8 @@ class HKDevice(): # Register and add new entities that are available self.add_entities() + await self.async_update() + return True def add_listener(self, add_entities_cb): @@ -184,6 +259,47 @@ class HKDevice(): ) self.platforms.add(platform) + async def async_update(self, now=None): + """Poll state of all entities attached to this bridge/accessory.""" + # pylint: disable=import-error + from homekit.exceptions import ( + AccessoryDisconnectedError, AccessoryNotFoundError, + EncryptionError) + + if not self.pollable_characteristics: + _LOGGER.debug( + "HomeKit connection not polling any characteristics." + ) + return + + _LOGGER.debug("Starting HomeKit controller update") + + try: + new_values_dict = await self.get_characteristics( + self.pollable_characteristics, + ) + except AccessoryNotFoundError: + # Not only did the connection fail, but also the accessory is not + # visible on the network. + self.async_set_unavailable() + return + except (AccessoryDisconnectedError, EncryptionError): + # Temporary connection failure. Device is still available but our + # connection was dropped. + return + + self.available = True + + for (aid, cid), value in new_values_dict.items(): + accessory = self.current_state.setdefault(aid, {}) + accessory[cid] = value + + self.hass.helpers.dispatcher.async_dispatcher_send( + self.signal_state_updated, + ) + + _LOGGER.debug("Finished HomeKit controller update") + async def get_characteristics(self, *args, **kwargs): """Read latest state from homekit accessory.""" async with self.pairing_lock: diff --git a/tests/components/homekit_controller/common.py b/tests/components/homekit_controller/common.py index 34b6474c6e9..a46c2190c8e 100644 --- a/tests/components/homekit_controller/common.py +++ b/tests/components/homekit_controller/common.py @@ -14,10 +14,12 @@ from homeassistant import config_entries from homeassistant.components.homekit_controller.const import ( CONTROLLER, DOMAIN, HOMEKIT_ACCESSORY_DISPATCH) from homeassistant.components.homekit_controller import ( - async_setup_entry, config_flow) + config_flow) from homeassistant.setup import async_setup_component import homeassistant.util.dt as dt_util -from tests.common import async_fire_time_changed, load_fixture +from tests.common import ( + async_fire_time_changed, load_fixture, MockConfigEntry +) class FakePairing: @@ -97,12 +99,13 @@ class FakeController: class Helper: """Helper methods for interacting with HomeKit fakes.""" - def __init__(self, hass, entity_id, pairing, accessory): + def __init__(self, hass, entity_id, pairing, accessory, config_entry): """Create a helper for a given accessory/entity.""" self.hass = hass self.entity_id = entity_id self.pairing = pairing self.accessory = accessory + self.config_entry = config_entry self.characteristics = {} for service in self.accessory.services: @@ -113,9 +116,7 @@ class Helper: async def poll_and_get_state(self): """Trigger a time based poll and return the current entity state.""" - next_update = dt_util.utcnow() + timedelta(seconds=60) - async_fire_time_changed(self.hass, next_update) - await self.hass.async_block_till_done() + await time_changed(self.hass, 60) state = self.hass.states.get(self.entity_id) assert state is not None @@ -161,6 +162,13 @@ class FakeService(AbstractService): return char +async def time_changed(hass, seconds): + """Trigger time changed.""" + next_update = dt_util.utcnow() + timedelta(seconds) + async_fire_time_changed(hass, next_update) + await hass.async_block_till_done() + + async def setup_accessories_from_file(hass, path): """Load an collection of accessory defs from JSON data.""" accessories_fixture = await hass.async_add_executor_job( @@ -239,18 +247,24 @@ async def setup_test_accessories(hass, accessories): 'AccessoryPairingID': discovery_info['properties']['id'], }) - config_entry = config_entries.ConfigEntry( - 1, 'homekit_controller', 'TestData', pairing.pairing_data, - 'test', config_entries.CONN_CLASS_LOCAL_PUSH + config_entry = MockConfigEntry( + version=1, + domain='homekit_controller', + entry_id='TestData', + data=pairing.pairing_data, + title='test', + connection_class=config_entries.CONN_CLASS_LOCAL_PUSH ) + config_entry.add_to_hass(hass) + pairing_cls_loc = 'homekit.controller.ip_implementation.IpPairing' with mock.patch(pairing_cls_loc) as pairing_cls: pairing_cls.return_value = pairing - await async_setup_entry(hass, config_entry) + await config_entry.async_setup(hass) await hass.async_block_till_done() - return pairing + return config_entry, pairing async def device_config_changed(hass, accessories): @@ -305,6 +319,8 @@ async def setup_test_component(hass, services, capitalize=False, suffix=None): accessory = Accessory('TestDevice', 'example.com', 'Test', '0001', '0.1') accessory.services.extend(services) - pairing = await setup_test_accessories(hass, [accessory]) + config_entry, pairing = await setup_test_accessories(hass, [accessory]) entity = 'testdevice' if suffix is None else 'testdevice_{}'.format(suffix) - return Helper(hass, '.'.join((domain, entity)), pairing, accessory) + return Helper( + hass, '.'.join((domain, entity)), pairing, accessory, config_entry + ) diff --git a/tests/components/homekit_controller/specific_devices/test_aqara_gateway.py b/tests/components/homekit_controller/specific_devices/test_aqara_gateway.py index 59b5be938d3..bee04cb0928 100644 --- a/tests/components/homekit_controller/specific_devices/test_aqara_gateway.py +++ b/tests/components/homekit_controller/specific_devices/test_aqara_gateway.py @@ -14,7 +14,7 @@ async def test_aqara_gateway_setup(hass): """Test that a Aqara Gateway can be correctly setup in HA.""" accessories = await setup_accessories_from_file( hass, 'aqara_gateway.json') - pairing = await setup_test_accessories(hass, accessories) + config_entry, pairing = await setup_test_accessories(hass, accessories) entity_registry = await hass.helpers.entity_registry.async_get_registry() @@ -24,7 +24,9 @@ async def test_aqara_gateway_setup(hass): assert alarm.unique_id == 'homekit-0000000123456789-66304' alarm_helper = Helper( - hass, 'alarm_control_panel.aqara_hub_1563', pairing, accessories[0]) + hass, 'alarm_control_panel.aqara_hub_1563', pairing, accessories[0], + config_entry + ) alarm_state = await alarm_helper.poll_and_get_state() assert alarm_state.attributes['friendly_name'] == 'Aqara Hub-1563' @@ -33,7 +35,7 @@ async def test_aqara_gateway_setup(hass): assert light.unique_id == 'homekit-0000000123456789-65792' light_helper = Helper( - hass, 'light.aqara_hub_1563', pairing, accessories[0]) + hass, 'light.aqara_hub_1563', pairing, accessories[0], config_entry) light_state = await light_helper.poll_and_get_state() assert light_state.attributes['friendly_name'] == 'Aqara Hub-1563' assert light_state.attributes['supported_features'] == ( diff --git a/tests/components/homekit_controller/specific_devices/test_ecobee3.py b/tests/components/homekit_controller/specific_devices/test_ecobee3.py index 8ff9219a1f8..c6b3142e7f3 100644 --- a/tests/components/homekit_controller/specific_devices/test_ecobee3.py +++ b/tests/components/homekit_controller/specific_devices/test_ecobee3.py @@ -7,30 +7,31 @@ https://github.com/home-assistant/home-assistant/issues/15336 from unittest import mock from homekit import AccessoryDisconnectedError -import pytest -from homeassistant.exceptions import ConfigEntryNotReady +from homeassistant.config_entries import ENTRY_STATE_SETUP_RETRY from homeassistant.components.climate.const import ( SUPPORT_TARGET_TEMPERATURE, SUPPORT_TARGET_HUMIDITY) from tests.components.homekit_controller.common import ( FakePairing, device_config_changed, setup_accessories_from_file, - setup_test_accessories, Helper + setup_test_accessories, Helper, time_changed ) async def test_ecobee3_setup(hass): """Test that a Ecbobee 3 can be correctly setup in HA.""" accessories = await setup_accessories_from_file(hass, 'ecobee3.json') - pairing = await setup_test_accessories(hass, accessories) + config_entry, pairing = 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' - climate_helper = Helper(hass, 'climate.homew', pairing, accessories[0]) + climate_helper = Helper( + hass, 'climate.homew', pairing, accessories[0], config_entry + ) climate_state = await climate_helper.poll_and_get_state() assert climate_state.attributes['friendly_name'] == 'HomeW' assert climate_state.attributes['supported_features'] == ( @@ -53,7 +54,7 @@ async def test_ecobee3_setup(hass): assert occ1.unique_id == 'homekit-AB1C-56' occ1_helper = Helper( - hass, 'binary_sensor.kitchen', pairing, accessories[0]) + hass, 'binary_sensor.kitchen', pairing, accessories[0], config_entry) occ1_state = await occ1_helper.poll_and_get_state() assert occ1_state.attributes['friendly_name'] == 'Kitchen' @@ -131,8 +132,8 @@ async def test_ecobee3_setup_connection_failure(hass): # If there is no cached entity map and the accessory connection is # failing then we have to fail the config entry setup. - with pytest.raises(ConfigEntryNotReady): - await setup_test_accessories(hass, accessories) + config_entry, pairing = await setup_test_accessories(hass, accessories) + assert config_entry.state == ENTRY_STATE_SETUP_RETRY climate = entity_registry.async_get('climate.homew') assert climate is None @@ -140,7 +141,16 @@ async def test_ecobee3_setup_connection_failure(hass): # When accessory raises ConfigEntryNoteReady HA will retry - lets make # sure there is no cruft causing conflicts left behind by now doing # a successful setup. - await setup_test_accessories(hass, accessories) + + # We just advance time by 5 minutes so that the retry happens, rather + # than manually invoking async_setup_entry - this means we need to + # make sure the IpPairing mock is in place or we'll try to connect to + # a real device. Normally this mocking is done by the helper in + # setup_test_accessories. + pairing_cls_loc = 'homekit.controller.ip_implementation.IpPairing' + with mock.patch(pairing_cls_loc) as pairing_cls: + pairing_cls.return_value = pairing + await time_changed(hass, 5 * 60) climate = entity_registry.async_get('climate.homew') assert climate.unique_id == 'homekit-123456789012-16' diff --git a/tests/components/homekit_controller/specific_devices/test_koogeek_ls1.py b/tests/components/homekit_controller/specific_devices/test_koogeek_ls1.py index 4f18392948b..70f1cd495dc 100644 --- a/tests/components/homekit_controller/specific_devices/test_koogeek_ls1.py +++ b/tests/components/homekit_controller/specific_devices/test_koogeek_ls1.py @@ -19,7 +19,7 @@ LIGHT_ON = ('lightbulb', 'on') async def test_koogeek_ls1_setup(hass): """Test that a Koogeek LS1 can be correctly setup in HA.""" accessories = await setup_accessories_from_file(hass, 'koogeek_ls1.json') - pairing = await setup_test_accessories(hass, accessories) + config_entry, pairing = await setup_test_accessories(hass, accessories) entity_registry = await hass.helpers.entity_registry.async_get_registry() @@ -27,7 +27,13 @@ async def test_koogeek_ls1_setup(hass): entry = entity_registry.async_get('light.koogeek_ls1_20833f') assert entry.unique_id == 'homekit-AAAA011111111111-7' - helper = Helper(hass, 'light.koogeek_ls1_20833f', pairing, accessories[0]) + helper = Helper( + hass, + 'light.koogeek_ls1_20833f', + pairing, + accessories[0], + config_entry + ) state = await helper.poll_and_get_state() # Assert that the friendly name is detected correctly @@ -58,9 +64,11 @@ async def test_recover_from_failure(hass, utcnow, failure_cls): See https://github.com/home-assistant/home-assistant/issues/18949 """ accessories = await setup_accessories_from_file(hass, 'koogeek_ls1.json') - pairing = await setup_test_accessories(hass, accessories) + config_entry, pairing = await setup_test_accessories(hass, accessories) - helper = Helper(hass, 'light.koogeek_ls1_20833f', pairing, accessories[0]) + helper = Helper( + hass, 'light.koogeek_ls1_20833f', pairing, accessories[0], config_entry + ) # Set light state on fake device to off helper.characteristics[LIGHT_ON].set_value(False) @@ -80,7 +88,8 @@ async def test_recover_from_failure(hass, utcnow, failure_cls): state = await helper.poll_and_get_state() assert state.state == 'off' - get_char.assert_called_with([(1, 8), (1, 9), (1, 10), (1, 11)]) + chars = get_char.call_args[0][0] + assert set(chars) == {(1, 8), (1, 9), (1, 10), (1, 11)} # Test that entity changes state when network error goes away next_update += timedelta(seconds=60) diff --git a/tests/components/homekit_controller/specific_devices/test_lennox_e30.py b/tests/components/homekit_controller/specific_devices/test_lennox_e30.py index 9d8bc84d501..b6cee307bed 100644 --- a/tests/components/homekit_controller/specific_devices/test_lennox_e30.py +++ b/tests/components/homekit_controller/specific_devices/test_lennox_e30.py @@ -14,14 +14,16 @@ from tests.components.homekit_controller.common import ( async def test_lennox_e30_setup(hass): """Test that a Lennox E30 can be correctly setup in HA.""" accessories = await setup_accessories_from_file(hass, 'lennox_e30.json') - pairing = await setup_test_accessories(hass, accessories) + config_entry, pairing = await setup_test_accessories(hass, accessories) entity_registry = await hass.helpers.entity_registry.async_get_registry() climate = entity_registry.async_get('climate.lennox') assert climate.unique_id == 'homekit-XXXXXXXX-100' - climate_helper = Helper(hass, 'climate.lennox', pairing, accessories[0]) + climate_helper = Helper( + hass, 'climate.lennox', pairing, accessories[0], config_entry + ) climate_state = await climate_helper.poll_and_get_state() assert climate_state.attributes['friendly_name'] == 'Lennox' assert climate_state.attributes['supported_features'] == ( diff --git a/tests/components/homekit_controller/test_light.py b/tests/components/homekit_controller/test_light.py index 59363f72146..6e2dc4891eb 100644 --- a/tests/components/homekit_controller/test_light.py +++ b/tests/components/homekit_controller/test_light.py @@ -1,4 +1,6 @@ """Basic checks for HomeKitSwitch.""" +from homeassistant.components.homekit_controller.const import KNOWN_DEVICES + from tests.components.homekit_controller.common import ( FakeService, setup_test_component) @@ -152,3 +154,23 @@ async def test_light_becomes_unavailable_but_recovers(hass, utcnow): assert state.state == 'on' assert state.attributes['brightness'] == 255 assert state.attributes['color_temp'] == 400 + + +async def test_light_unloaded(hass, utcnow): + """Test entity and HKDevice are correctly unloaded.""" + bulb = create_lightbulb_service_with_color_temp() + helper = await setup_test_component(hass, [bulb]) + + # Initial state is that the light is off + state = await helper.poll_and_get_state() + assert state.state == 'off' + + unload_result = await helper.config_entry.async_unload(hass) + assert unload_result is True + + # Make sure entity is unloaded + assert hass.states.get(helper.entity_id) is None + + # Make sure HKDevice is no longer set to poll this accessory + conn = hass.data[KNOWN_DEVICES]['00:00:00:00:00:00'] + assert not conn.pollable_characteristics