Only poll HomeKit connection once for all entities on a single bridge/pairing

homeassistant/components/homekit_controller/__init__.py

@@ -27,14 +27,46 @@ 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']
         self._features = 0
         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 +79,7 @@ def setup(self):
             get_uuid(c) for c in self.get_characteristic_types()
         ]
 
-        self._chars_to_poll = []
+        self.pollable_characteristics = []
         self._chars = {}
         self._char_names = {}
 
@@ -75,7 +107,7 @@ def _setup_characteristic(self, char):
         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 +123,10 @@ def _setup_characteristic(self, char):
         # 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():
+    async 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 +137,8 @@ async def async_update(self):
             # 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 +153,7 @@ def name(self):
     @property
     def available(self) -> bool:
         """Return True if entity is available."""
-        return self._available
+        return self._accessory.available
 
     @property
     def device_info(self):
@@ -214,4 +228,9 @@ async def async_setup(hass, config):
 async def async_remove_entry(hass, entry):
     """Cleanup caches before removing config entry."""
     hkid = entry.data['AccessoryPairingID']
+
+    if hkid in hass.data[KNOWN_DEVICES]:
+        connection = hass.data[KNOWN_DEVICES][hkid]
+        await connection.async_unload()
+
     hass.data[ENTITY_MAP].async_delete_map(hkid)

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 @@ def __init__(self, hass, config_entry, pairing_data):
         # 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 @@ async def async_setup(self):
 
         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 @@ async def async_refresh_entity_map(self, config_num):
         # 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 @@ def async_load_platforms(self):
                 )
                 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:

tests/components/homekit_controller/specific_devices/test_koogeek_ls1.py

@@ -80,7 +80,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)