Handle re-adding of accessories/services/chars in homekit_controller after removal (#102192)

homeassistant/components/homekit_controller/connection.py

@@ -103,7 +103,7 @@ class HKDevice:
 
         # Track aid/iid pairs so we know if we already handle triggers for a HK
         # service.
-        self._triggers: list[tuple[int, int]] = []
+        self._triggers: set[tuple[int, int]] = set()
 
         # A list of callbacks that turn HK characteristics into entities
         self.char_factories: list[AddCharacteristicCb] = []
@@ -639,18 +639,25 @@ class HKDevice:
         await self.async_update()
         await self.async_add_new_entities()
 
-    def add_accessory_factory(self, add_entities_cb) -> None:
+    @callback
+    def async_entity_key_removed(self, entity_key: tuple[int, int | None, int | None]):
+        """Handle an entity being removed.
+
+        Releases the entity from self.entities so it can be added again.
+        """
+        self.entities.discard(entity_key)
+
+    def add_accessory_factory(self, add_entities_cb: AddAccessoryCb) -> None:
         """Add a callback to run when discovering new entities for accessories."""
         self.accessory_factories.append(add_entities_cb)
         self._add_new_entities_for_accessory([add_entities_cb])
 
-    def _add_new_entities_for_accessory(self, handlers) -> None:
+    def _add_new_entities_for_accessory(self, handlers: list[AddAccessoryCb]) -> None:
         for accessory in self.entity_map.accessories:
+            entity_key = (accessory.aid, None, None)
             for handler in handlers:
-                if (accessory.aid, None, None) in self.entities:
-                    continue
-                if handler(accessory):
-                    self.entities.add((accessory.aid, None, None))
+                if entity_key not in self.entities and handler(accessory):
+                    self.entities.add(entity_key)
                     break
 
     def add_char_factory(self, add_entities_cb: AddCharacteristicCb) -> None:
@@ -662,11 +669,10 @@ class HKDevice:
         for accessory in self.entity_map.accessories:
             for service in accessory.services:
                 for char in service.characteristics:
+                    entity_key = (accessory.aid, service.iid, char.iid)
                     for handler in handlers:
-                        if (accessory.aid, service.iid, char.iid) in self.entities:
-                            continue
-                        if handler(char):
-                            self.entities.add((accessory.aid, service.iid, char.iid))
+                        if entity_key not in self.entities and handler(char):
+                            self.entities.add(entity_key)
                             break
 
     def add_listener(self, add_entities_cb: AddServiceCb) -> None:
@@ -692,7 +698,7 @@ class HKDevice:
 
                 for add_trigger_cb in callbacks:
                     if add_trigger_cb(service):
-                        self._triggers.append(entity_key)
+                        self._triggers.add(entity_key)
                         break
 
     def add_entities(self) -> None:
@@ -702,19 +708,19 @@ class HKDevice:
         self._add_new_entities_for_char(self.char_factories)
         self._add_new_triggers(self.trigger_factories)
 
-    def _add_new_entities(self, callbacks) -> None:
+    def _add_new_entities(self, callbacks: list[AddServiceCb]) -> None:
         for accessory in self.entity_map.accessories:
             aid = accessory.aid
             for service in accessory.services:
-                iid = service.iid
+                entity_key = (aid, None, service.iid)
 
-                if (aid, None, iid) in self.entities:
+                if entity_key in self.entities:
                     # Don't add the same entity again
                     continue
 
                 for listener in callbacks:
                     if listener(service):
-                        self.entities.add((aid, None, iid))
+                        self.entities.add(entity_key)
                         break
 
     async def async_load_platform(self, platform: str) -> None:

homeassistant/components/homekit_controller/entity.py

@@ -40,8 +40,13 @@ class HomeKitEntity(Entity):
     def __init__(self, accessory: HKDevice, devinfo: ConfigType) -> None:
         """Initialise a generic HomeKit device."""
         self._accessory = accessory
-        self._aid = devinfo["aid"]
-        self._iid = devinfo["iid"]
+        self._aid: int = devinfo["aid"]
+        self._iid: int = devinfo["iid"]
+        self._entity_key: tuple[int, int | None, int | None] = (
+            self._aid,
+            None,
+            self._iid,
+        )
         self._char_name: str | None = None
         self._char_subscription: CALLBACK_TYPE | None = None
         self.async_setup()
@@ -96,6 +101,7 @@ class HomeKitEntity(Entity):
     async def async_will_remove_from_hass(self) -> None:
         """Prepare to be removed from hass."""
         self._async_unsubscribe_chars()
+        self._accessory.async_entity_key_removed(self._entity_key)
 
     @callback
     def _async_unsubscribe_chars(self):
@@ -268,6 +274,7 @@ class BaseCharacteristicEntity(HomeKitEntity):
         """Initialise a generic single characteristic HomeKit entity."""
         self._char = char
         super().__init__(accessory, devinfo)
+        self._entity_key = (self._aid, self._iid, char.iid)
 
     @callback
     def _async_remove_entity_if_characteristics_disappeared(self) -> bool:

tests/components/homekit_controller/common.py

@@ -8,7 +8,7 @@ import os
 from typing import Any, Final
 from unittest import mock
 
-from aiohomekit.controller.abstract import AbstractPairing
+from aiohomekit.controller.abstract import AbstractDescription, AbstractPairing
 from aiohomekit.hkjson import loads as hkloads
 from aiohomekit.model import (
     Accessories,
@@ -17,7 +17,6 @@ from aiohomekit.model import (
     mixin as model_mixin,
 )
 from aiohomekit.testing import FakeController, FakePairing
-from aiohomekit.zeroconf import HomeKitService
 
 from homeassistant.components.device_automation import DeviceAutomationType
 from homeassistant.components.homekit_controller.const import (
@@ -254,26 +253,21 @@ async def device_config_changed(hass: HomeAssistant, accessories: Accessories):
     accessories_obj = Accessories()
     for accessory in accessories:
         accessories_obj.add_accessory(accessory)
-    pairing._accessories_state = AccessoriesState(
-        accessories_obj, pairing.config_num + 1
-    )
+
+    new_config_num = pairing.config_num + 1
     pairing._async_description_update(
-        HomeKitService(
-            name="TestDevice.local",
+        AbstractDescription(
+            name="testdevice.local.",
             id="00:00:00:00:00:00",
-            model="",
-            config_num=2,
-            state_num=3,
-            feature_flags=0,
             status_flags=0,
+            config_num=new_config_num,
             category=1,
-            protocol_version="1.0",
-            type="_hap._tcp.local.",
-            address="127.0.0.1",
-            addresses=["127.0.0.1"],
-            port=8080,
         )
     )
+    # Set the accessories state only after calling
+    # _async_description_update, otherwise the config_num will be
+    # overwritten
+    pairing._accessories_state = AccessoriesState(accessories_obj, new_config_num)
 
     # Wait for services to reconfigure
     await hass.async_block_till_done()

tests/components/homekit_controller/specific_devices/test_ecobee3.py

@@ -300,9 +300,10 @@ async def test_ecobee3_remove_sensors_at_runtime(hass: HomeAssistant) -> None:
     occ3 = entity_registry.async_get("binary_sensor.basement")
     assert occ3.unique_id == "00:00:00:00:00:00_4_56"
 
-    # Currently it is not possible to add the entities back once
-    # they are removed because _add_new_entities has a guard to prevent
-    # the same entity from being added twice.
+    # Ensure the sensors are back
+    assert hass.states.get("binary_sensor.kitchen") is not None
+    assert hass.states.get("binary_sensor.porch") is not None
+    assert hass.states.get("binary_sensor.basement") is not None
 
 
 async def test_ecobee3_services_and_chars_removed(