Remove homekit_controller entity registry entries when backing char or service is gone (#109952)

homeassistant/components/homekit_controller/connection.py

@@ -46,6 +46,7 @@ from .const import (
     SUBSCRIBE_COOLDOWN,
 )
 from .device_trigger import async_fire_triggers, async_setup_triggers_for_entry
+from .utils import IidTuple, unique_id_to_iids
 
 RETRY_INTERVAL = 60  # seconds
 MAX_POLL_FAILURES_TO_DECLARE_UNAVAILABLE = 3
@@ -513,6 +514,54 @@ class HKDevice:
 
             device_registry.async_update_device(device.id, new_identifiers=identifiers)
 
+    @callback
+    def async_reap_stale_entity_registry_entries(self) -> None:
+        """Delete entity registry entities for removed characteristics, services and accessories."""
+        _LOGGER.debug(
+            "Removing stale entity registry entries for pairing %s",
+            self.unique_id,
+        )
+
+        reg = er.async_get(self.hass)
+
+        # For the current config entry only, visit all registry entity entries
+        # Build a set of (unique_id, aid, sid, iid)
+        # For services, (unique_id, aid, sid, None)
+        # For accessories, (unique_id, aid, None, None)
+        entries = er.async_entries_for_config_entry(reg, self.config_entry.entry_id)
+        existing_entities = {
+            iids: entry.entity_id
+            for entry in entries
+            if (iids := unique_id_to_iids(entry.unique_id))
+        }
+
+        # Process current entity map and produce a similar set
+        current_unique_id: set[IidTuple] = set()
+        for accessory in self.entity_map.accessories:
+            current_unique_id.add((accessory.aid, None, None))
+
+            for service in accessory.services:
+                current_unique_id.add((accessory.aid, service.iid, None))
+
+                for char in service.characteristics:
+                    current_unique_id.add(
+                        (
+                            accessory.aid,
+                            service.iid,
+                            char.iid,
+                        )
+                    )
+
+        # Remove the difference
+        if stale := existing_entities.keys() - current_unique_id:
+            for parts in stale:
+                _LOGGER.debug(
+                    "Removing stale entity registry entry %s for pairing %s",
+                    existing_entities[parts],
+                    self.unique_id,
+                )
+                reg.async_remove(existing_entities[parts])
+
     @callback
     def async_migrate_ble_unique_id(self) -> None:
         """Config entries from step_bluetooth used incorrect identifier for unique_id."""
@@ -615,6 +664,8 @@ class HKDevice:
 
         self.async_migrate_ble_unique_id()
 
+        self.async_reap_stale_entity_registry_entries()
+
         self.async_create_devices()
 
         # Load any triggers for this config entry

homeassistant/components/homekit_controller/utils.py

@@ -11,6 +11,31 @@ from homeassistant.core import Event, HomeAssistant
 from .const import CONTROLLER
 from .storage import async_get_entity_storage
 
+IidTuple = tuple[int, int | None, int | None]
+
+
+def unique_id_to_iids(unique_id: str) -> IidTuple | None:
+    """Convert a unique_id to a tuple of accessory id, service iid and characteristic iid.
+
+    Depending on the field in the accessory map that is referenced, some of these may be None.
+
+    Returns None if this unique_id doesn't follow the homekit_controller scheme and is invalid.
+    """
+    try:
+        match unique_id.split("_"):
+            case (unique_id, aid, sid, cid):
+                return (int(aid), int(sid), int(cid))
+            case (unique_id, aid, sid):
+                return (int(aid), int(sid), None)
+            case (unique_id, aid):
+                return (int(aid), None, None)
+    except ValueError:
+        # One of the int conversions failed - this can't be a valid homekit_controller unique id
+        # Fall through and return None
+        pass
+
+    return None
+
 
 @lru_cache
 def folded_name(name: str) -> str:

tests/components/homekit_controller/specific_devices/test_ecobee3.py

@@ -284,8 +284,13 @@ async def test_ecobee3_remove_sensors_at_runtime(
     await device_config_changed(hass, accessories)
 
     assert hass.states.get("binary_sensor.kitchen") is None
+    assert entity_registry.async_get("binary_sensor.kitchen") is None
+
     assert hass.states.get("binary_sensor.porch") is None
+    assert entity_registry.async_get("binary_sensor.porch") is None
+
     assert hass.states.get("binary_sensor.basement") is None
+    assert entity_registry.async_get("binary_sensor.basement") is None
 
     # Now add the sensors back
     accessories = await setup_accessories_from_file(hass, "ecobee3.json")
@@ -302,8 +307,13 @@ async def test_ecobee3_remove_sensors_at_runtime(
 
     # Ensure the sensors are back
     assert hass.states.get("binary_sensor.kitchen") is not None
+    assert occ1.id == entity_registry.async_get("binary_sensor.kitchen").id
+
     assert hass.states.get("binary_sensor.porch") is not None
+    assert occ2.id == entity_registry.async_get("binary_sensor.porch").id
+
     assert hass.states.get("binary_sensor.basement") is not None
+    assert occ3.id == entity_registry.async_get("binary_sensor.basement").id
 
 
 async def test_ecobee3_services_and_chars_removed(
@@ -333,10 +343,15 @@ async def test_ecobee3_services_and_chars_removed(
 
     # Make sure the climate entity is still there
     assert hass.states.get("climate.homew") is not None
+    assert entity_registry.async_get("climate.homew") is not None
 
     # Make sure the basement temperature sensor is gone
     assert hass.states.get("sensor.basement_temperature") is None
+    assert entity_registry.async_get("select.basement_temperature") is None
 
     # Make sure the current mode select and clear hold button are gone
     assert hass.states.get("select.homew_current_mode") is None
+    assert entity_registry.async_get("select.homew_current_mode") is None
+
     assert hass.states.get("button.homew_clear_hold") is None
+    assert entity_registry.async_get("button.homew_clear_hold") is None

tests/components/homekit_controller/specific_devices/test_fan_that_changes_features.py

@@ -136,6 +136,7 @@ async def test_bridge_with_two_fans_one_removed(
 
     # Verify the first fan is still there
     fan_state = hass.states.get("fan.living_room_fan")
+    assert entity_registry.async_get("fan.living_room_fan") is not None
     assert (
         fan_state.attributes[ATTR_SUPPORTED_FEATURES]
         is FanEntityFeature.SET_SPEED
@@ -144,3 +145,4 @@ async def test_bridge_with_two_fans_one_removed(
     )
     # The second fan should have been removed
     assert not hass.states.get("fan.ceiling_fan")
+    assert not entity_registry.async_get("fan.ceiling_fan")

tests/components/homekit_controller/test_utils.py

@@ -0,0 +1,15 @@
+"""Checks for basic helper utils."""
+from homeassistant.components.homekit_controller.utils import unique_id_to_iids
+
+
+def test_unique_id_to_iids():
+    """Check that unique_id_to_iids is safe against different invalid ids."""
+    assert unique_id_to_iids("pairingid_1_2_3") == (1, 2, 3)
+    assert unique_id_to_iids("pairingid_1_2") == (1, 2, None)
+    assert unique_id_to_iids("pairingid_1") == (1, None, None)
+
+    assert unique_id_to_iids("pairingid") is None
+    assert unique_id_to_iids("pairingid_1_2_3_4") is None
+    assert unique_id_to_iids("pairingid_a") is None
+    assert unique_id_to_iids("pairingid_1_a") is None
+    assert unique_id_to_iids("pairingid_1_2_a") is None