Fix hkid matching in homekit_controller when zeroconf value is not upper case (#100641)

homeassistant/components/homekit_controller/config_flow.py

@@ -80,12 +80,12 @@ def formatted_category(category: Categories) -> str:
 
 
 @callback
-def find_existing_host(
-    hass: HomeAssistant, serial: str
+def find_existing_config_entry(
+    hass: HomeAssistant, upper_case_hkid: str
 ) -> config_entries.ConfigEntry | None:
     """Return a set of the configured hosts."""
     for entry in hass.config_entries.async_entries(DOMAIN):
-        if entry.data.get("AccessoryPairingID") == serial:
+        if entry.data.get("AccessoryPairingID") == upper_case_hkid:
             return entry
     return None
 
@@ -114,7 +114,7 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
     def __init__(self) -> None:
         """Initialize the homekit_controller flow."""
         self.model: str | None = None
-        self.hkid: str | None = None
+        self.hkid: str | None = None  # This is always lower case
         self.name: str | None = None
         self.category: Categories | None = None
         self.devices: dict[str, AbstractDiscovery] = {}
@@ -199,11 +199,12 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
 
         return self._async_step_pair_show_form()
 
-    async def _hkid_is_homekit(self, hkid: str) -> bool:
+    @callback
+    def _hkid_is_homekit(self, hkid: str) -> bool:
         """Determine if the device is a homekit bridge or accessory."""
         dev_reg = dr.async_get(self.hass)
         device = dev_reg.async_get_device(
-            connections={(dr.CONNECTION_NETWORK_MAC, hkid)}
+            connections={(dr.CONNECTION_NETWORK_MAC, dr.format_mac(hkid))}
         )
 
         if device is None:
@@ -244,17 +245,10 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
 
         # The hkid is a unique random number that looks like a pairing code.
         # It changes if a device is factory reset.
-        hkid = properties[zeroconf.ATTR_PROPERTIES_ID]
+        hkid: str = properties[zeroconf.ATTR_PROPERTIES_ID]
         normalized_hkid = normalize_hkid(hkid)
-
-        # If this aiohomekit doesn't support this particular device, ignore it.
-        if not domain_supported(discovery_info.name):
-            return self.async_abort(reason="ignored_model")
-
-        model = properties["md"]
-        name = domain_to_name(discovery_info.name)
+        upper_case_hkid = hkid.upper()
         status_flags = int(properties["sf"])
-        category = Categories(int(properties.get("ci", 0)))
         paired = not status_flags & 0x01
 
         # Set unique-id and error out if it's already configured
@@ -265,23 +259,29 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
             "AccessoryIP": discovery_info.host,
             "AccessoryPort": discovery_info.port,
         }
-
         # If the device is already paired and known to us we should monitor c#
         # (config_num) for changes. If it changes, we check for new entities
-        if paired and hkid in self.hass.data.get(KNOWN_DEVICES, {}):
+        if paired and upper_case_hkid in self.hass.data.get(KNOWN_DEVICES, {}):
             if existing_entry:
                 self.hass.config_entries.async_update_entry(
                     existing_entry, data={**existing_entry.data, **updated_ip_port}
                 )
             return self.async_abort(reason="already_configured")
 
-        _LOGGER.debug("Discovered device %s (%s - %s)", name, model, hkid)
+        # If this aiohomekit doesn't support this particular device, ignore it.
+        if not domain_supported(discovery_info.name):
+            return self.async_abort(reason="ignored_model")
+
+        model = properties["md"]
+        name = domain_to_name(discovery_info.name)
+        _LOGGER.debug("Discovered device %s (%s - %s)", name, model, upper_case_hkid)
 
         # Device isn't paired with us or anyone else.
         # But we have a 'complete' config entry for it - that is probably
         # invalid. Remove it automatically.
-        existing = find_existing_host(self.hass, hkid)
-        if not paired and existing:
+        if not paired and (
+            existing := find_existing_config_entry(self.hass, upper_case_hkid)
+        ):
             if self.controller is None:
                 await self._async_setup_controller()
 
@@ -348,13 +348,13 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
 
         # If this is a HomeKit bridge/accessory exported
         # by *this* HA instance ignore it.
-        if await self._hkid_is_homekit(hkid):
+        if self._hkid_is_homekit(hkid):
             return self.async_abort(reason="ignored_model")
 
         self.name = name
         self.model = model
-        self.category = category
-        self.hkid = hkid
+        self.category = Categories(int(properties.get("ci", 0)))
+        self.hkid = normalized_hkid
 
         # We want to show the pairing form - but don't call async_step_pair
         # directly as it has side effects (will ask the device to show a

tests/components/homekit_controller/test_config_flow.py

@@ -1180,3 +1180,80 @@ async def test_bluetooth_valid_device_discovery_unpaired(
     assert result3["data"] == {}
 
     assert storage.get_map("00:00:00:00:00:00") is not None
+
+
+async def test_discovery_updates_ip_when_config_entry_set_up(
+    hass: HomeAssistant, controller
+) -> None:
+    """Already configured updates ip when config entry set up."""
+    entry = MockConfigEntry(
+        domain="homekit_controller",
+        data={
+            "AccessoryIP": "4.4.4.4",
+            "AccessoryPort": 66,
+            "AccessoryPairingID": "AA:BB:CC:DD:EE:FF",
+        },
+        unique_id="aa:bb:cc:dd:ee:ff",
+    )
+    entry.add_to_hass(hass)
+
+    connection_mock = AsyncMock()
+    hass.data[KNOWN_DEVICES] = {"AA:BB:CC:DD:EE:FF": connection_mock}
+
+    device = setup_mock_accessory(controller)
+    discovery_info = get_device_discovery_info(device)
+
+    # Set device as already paired
+    discovery_info.properties["sf"] = 0x00
+    discovery_info.properties[zeroconf.ATTR_PROPERTIES_ID] = "Aa:bB:cC:dD:eE:fF"
+
+    # Device is discovered
+    result = await hass.config_entries.flow.async_init(
+        "homekit_controller",
+        context={"source": config_entries.SOURCE_ZEROCONF},
+        data=discovery_info,
+    )
+    assert result["type"] == FlowResultType.ABORT
+    assert result["reason"] == "already_configured"
+    await hass.async_block_till_done()
+
+    assert entry.data["AccessoryIP"] == discovery_info.host
+    assert entry.data["AccessoryPort"] == discovery_info.port
+
+
+async def test_discovery_updates_ip_config_entry_not_set_up(
+    hass: HomeAssistant, controller
+) -> None:
+    """Already configured updates ip when the config entry is not set up."""
+    entry = MockConfigEntry(
+        domain="homekit_controller",
+        data={
+            "AccessoryIP": "4.4.4.4",
+            "AccessoryPort": 66,
+            "AccessoryPairingID": "AA:BB:CC:DD:EE:FF",
+        },
+        unique_id="aa:bb:cc:dd:ee:ff",
+    )
+    entry.add_to_hass(hass)
+
+    AsyncMock()
+
+    device = setup_mock_accessory(controller)
+    discovery_info = get_device_discovery_info(device)
+
+    # Set device as already paired
+    discovery_info.properties["sf"] = 0x00
+    discovery_info.properties[zeroconf.ATTR_PROPERTIES_ID] = "Aa:bB:cC:dD:eE:fF"
+
+    # Device is discovered
+    result = await hass.config_entries.flow.async_init(
+        "homekit_controller",
+        context={"source": config_entries.SOURCE_ZEROCONF},
+        data=discovery_info,
+    )
+    assert result["type"] == FlowResultType.ABORT
+    assert result["reason"] == "already_configured"
+    await hass.async_block_till_done()
+
+    assert entry.data["AccessoryIP"] == discovery_info.host
+    assert entry.data["AccessoryPort"] == discovery_info.port