BLE pairing reliablity fixes for HomeKit Controller (#76199)

- Remove the cached map from memory when unpairing so
  we do not reuse it again if they unpair/repair

- Fixes for accessories that use a config number of
  0

- General reliablity improvements to the pairing process
  under the hood of aiohomekit

homeassistant/components/homekit_controller/__init__.py

@@ -31,7 +31,7 @@ from homeassistant.helpers.typing import ConfigType
 from .config_flow import normalize_hkid
 from .connection import HKDevice, valid_serial_number
 from .const import ENTITY_MAP, KNOWN_DEVICES, TRIGGERS
-from .storage import async_get_entity_storage
+from .storage import EntityMapStorage, async_get_entity_storage
 from .utils import async_get_controller, folded_name
 
 _LOGGER = logging.getLogger(__name__)
@@ -269,7 +269,7 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     hkid = entry.data["AccessoryPairingID"]
 
     if hkid in hass.data[KNOWN_DEVICES]:
-        connection = hass.data[KNOWN_DEVICES][hkid]
+        connection: HKDevice = hass.data[KNOWN_DEVICES][hkid]
         await connection.async_unload()
 
     return True
@@ -280,7 +280,8 @@ async def async_remove_entry(hass: HomeAssistant, entry: ConfigEntry) -> None:
     hkid = entry.data["AccessoryPairingID"]
 
     # Remove cached type data from .storage/homekit_controller-entity-map
-    hass.data[ENTITY_MAP].async_delete_map(hkid)
+    entity_map_storage: EntityMapStorage = hass.data[ENTITY_MAP]
+    entity_map_storage.async_delete_map(hkid)
 
     controller = await async_get_controller(hass)
 

homeassistant/components/homekit_controller/config_flow.py

@@ -597,7 +597,7 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
         entity_storage = await async_get_entity_storage(self.hass)
         assert self.unique_id is not None
         entity_storage.async_create_or_update_map(
-            self.unique_id,
+            pairing.id,
             accessories_state.config_num,
             accessories_state.accessories.serialize(),
         )

homeassistant/components/homekit_controller/manifest.json

@@ -3,7 +3,7 @@
   "name": "HomeKit Controller",
   "config_flow": true,
   "documentation": "https://www.home-assistant.io/integrations/homekit_controller",
-  "requirements": ["aiohomekit==1.2.3"],
+  "requirements": ["aiohomekit==1.2.4"],
   "zeroconf": ["_hap._tcp.local.", "_hap._udp.local."],
   "bluetooth": [{ "manufacturer_id": 76, "manufacturer_data_start": [6] }],
   "dependencies": ["bluetooth", "zeroconf"],

homeassistant/components/homekit_controller/storage.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import logging
 from typing import Any, TypedDict
 
 from homeassistant.core import HomeAssistant, callback
@@ -12,6 +13,7 @@ from .const import DOMAIN, ENTITY_MAP
 ENTITY_MAP_STORAGE_KEY = f"{DOMAIN}-entity-map"
 ENTITY_MAP_STORAGE_VERSION = 1
 ENTITY_MAP_SAVE_DELAY = 10
+_LOGGER = logging.getLogger(__name__)
 
 
 class Pairing(TypedDict):
@@ -68,6 +70,7 @@ class EntityMapStorage:
         self, homekit_id: str, config_num: int, accessories: list[Any]
     ) -> Pairing:
         """Create a new pairing cache."""
+        _LOGGER.debug("Creating or updating entity map for %s", homekit_id)
         data = Pairing(config_num=config_num, accessories=accessories)
         self.storage_data[homekit_id] = data
         self._async_schedule_save()
@@ -76,11 +79,17 @@ class EntityMapStorage:
     @callback
     def async_delete_map(self, homekit_id: str) -> None:
         """Delete pairing cache."""
-        if homekit_id not in self.storage_data:
-            return
-
-        self.storage_data.pop(homekit_id)
-        self._async_schedule_save()
+        removed_one = False
+        # Previously there was a bug where a lowercase homekit_id was stored
+        # in the storage. We need to account for that.
+        for hkid in (homekit_id, homekit_id.lower()):
+            if hkid not in self.storage_data:
+                continue
+            _LOGGER.debug("Deleting entity map for %s", hkid)
+            self.storage_data.pop(hkid)
+            removed_one = True
+        if removed_one:
+            self._async_schedule_save()
 
     @callback
     def _async_schedule_save(self) -> None:

requirements_all.txt

@@ -168,7 +168,7 @@ aioguardian==2022.07.0
 aioharmony==0.2.9
 
 # homeassistant.components.homekit_controller
-aiohomekit==1.2.3
+aiohomekit==1.2.4
 
 # homeassistant.components.emulated_hue
 # homeassistant.components.http

requirements_test_all.txt

@@ -152,7 +152,7 @@ aioguardian==2022.07.0
 aioharmony==0.2.9
 
 # homeassistant.components.homekit_controller
-aiohomekit==1.2.3
+aiohomekit==1.2.4
 
 # homeassistant.components.emulated_hue
 # homeassistant.components.http

tests/components/homekit_controller/test_config_flow.py

@@ -14,6 +14,7 @@ from homeassistant import config_entries
 from homeassistant.components import zeroconf
 from homeassistant.components.homekit_controller import config_flow
 from homeassistant.components.homekit_controller.const import KNOWN_DEVICES
+from homeassistant.components.homekit_controller.storage import async_get_entity_storage
 from homeassistant.data_entry_flow import (
     RESULT_TYPE_ABORT,
     RESULT_TYPE_FORM,
@@ -1071,6 +1072,8 @@ async def test_bluetooth_valid_device_discovery_paired(hass, controller):
 async def test_bluetooth_valid_device_discovery_unpaired(hass, controller):
     """Test bluetooth discovery with a homekit device and discovery works."""
     setup_mock_accessory(controller)
+    storage = await async_get_entity_storage(hass)
+
     with patch(
         "homeassistant.components.homekit_controller.config_flow.aiohomekit_const.BLE_TRANSPORT_SUPPORTED",
         True,
@@ -1083,6 +1086,7 @@ async def test_bluetooth_valid_device_discovery_unpaired(hass, controller):
 
     assert result["type"] == RESULT_TYPE_FORM
     assert result["step_id"] == "pair"
+    assert storage.get_map("00:00:00:00:00:00") is None
 
     assert get_flow_context(hass, result) == {
         "source": config_entries.SOURCE_BLUETOOTH,
@@ -1098,3 +1102,5 @@ async def test_bluetooth_valid_device_discovery_unpaired(hass, controller):
     assert result3["type"] == FlowResultType.CREATE_ENTRY
     assert result3["title"] == "Koogeek-LS1-20833F"
     assert result3["data"] == {}
+
+    assert storage.get_map("00:00:00:00:00:00") is not None