Fix Matter unique_id generation (#86046)

* bae entity unique id on Operational Instance Name standard

* Update homeassistant/components/matter/entity.py

Co-authored-by: Stefan Agner <stefan@agner.ch>

* also adjust unique id for devices

* final adjustment

* remove assert on server_info

* move device info to init

* fabric_id_hex

* use DeviceInfo instead of dict

* fix test

Co-authored-by: Stefan Agner <stefan@agner.ch>

homeassistant/components/matter/adapter.py

@@ -1,11 +1,15 @@
 """Matter to Home Assistant adapter."""
 from __future__ import annotations
 
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, cast
 
 from chip.clusters import Objects as all_clusters
 from matter_server.common.models.events import EventType
-from matter_server.common.models.node_device import AbstractMatterNodeDevice
+from matter_server.common.models.node_device import (
+    AbstractMatterNodeDevice,
+    MatterBridgedNodeDevice,
+)
+from matter_server.common.models.server_information import ServerInfo
 
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import Platform
@@ -15,6 +19,7 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback
 
 from .const import DOMAIN, LOGGER
 from .device_platform import DEVICE_PLATFORM
+from .helpers import get_device_id
 
 if TYPE_CHECKING:
     from matter_server.client import MatterClient
@@ -66,31 +71,49 @@ class MatterAdapter:
         bridge_unique_id: str | None = None
 
         if node.aggregator_device_type_instance is not None and (
-            node_info := node.root_device_type_instance.get_cluster(all_clusters.Basic)
+            node.root_device_type_instance.get_cluster(all_clusters.Basic)
         ):
-            self._create_device_registry(
+            # create virtual (parent) device for bridge node device
-                node_info, node_info.nodeLabel or "Hub device", None
+            bridge_device = MatterBridgedNodeDevice(
+                node.aggregator_device_type_instance
             )
-            bridge_unique_id = node_info.uniqueID
+            self._create_device_registry(bridge_device)
+            server_info = cast(ServerInfo, self.matter_client.server_info)
+            bridge_unique_id = get_device_id(server_info, bridge_device)
 
         for node_device in node.node_devices:
             self._setup_node_device(node_device, bridge_unique_id)
 
     def _create_device_registry(
         self,
-        info: all_clusters.Basic | all_clusters.BridgedDeviceBasic,
+        node_device: AbstractMatterNodeDevice,
-        name: str,
+        bridge_unique_id: str | None = None,
-        bridge_unique_id: str | None,
     ) -> None:
         """Create a device registry entry."""
+        server_info = cast(ServerInfo, self.matter_client.server_info)
+        node_unique_id = get_device_id(
+            server_info,
+            node_device,
+        )
+        basic_info = node_device.device_info()
+        device_type_instances = node_device.device_type_instances()
+
+        name = basic_info.nodeLabel
+        if not name and isinstance(node_device, MatterBridgedNodeDevice):
+            # fallback name for Bridge
+            name = "Hub device"
+        elif not name and device_type_instances:
+            # fallback name based on device type
+            name = f"{device_type_instances[0].device_type.__doc__[:-1]} {node_device.node().node_id}"
+
         dr.async_get(self.hass).async_get_or_create(
             name=name,
             config_entry_id=self.config_entry.entry_id,
-            identifiers={(DOMAIN, info.uniqueID)},
+            identifiers={(DOMAIN, node_unique_id)},
-            hw_version=info.hardwareVersionString,
+            hw_version=basic_info.hardwareVersionString,
-            sw_version=info.softwareVersionString,
+            sw_version=basic_info.softwareVersionString,
-            manufacturer=info.vendorName,
+            manufacturer=basic_info.vendorName,
-            model=info.productName,
+            model=basic_info.productName,
             via_device=(DOMAIN, bridge_unique_id) if bridge_unique_id else None,
         )
 
@@ -98,17 +121,9 @@ class MatterAdapter:
         self, node_device: AbstractMatterNodeDevice, bridge_unique_id: str | None
     ) -> None:
         """Set up a node device."""
-        node = node_device.node()
+        self._create_device_registry(node_device, bridge_unique_id)
-        basic_info = node_device.device_info()
+        # run platform discovery from device type instances
-        device_type_instances = node_device.device_type_instances()
+        for instance in node_device.device_type_instances():
-
-        name = basic_info.nodeLabel
-        if not name and device_type_instances:
-            name = f"{device_type_instances[0].device_type.__doc__[:-1]} {node.node_id}"
-
-        self._create_device_registry(basic_info, name, bridge_unique_id)
-
-        for instance in device_type_instances:
             created = False
 
             for platform, devices in DEVICE_PLATFORM.items():

homeassistant/components/matter/entity.py

@@ -5,16 +5,18 @@ from abc import abstractmethod
 from collections.abc import Callable
 from dataclasses import dataclass
 import logging
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING, Any, cast
 
 from matter_server.common.models.device_type_instance import MatterDeviceTypeInstance
 from matter_server.common.models.events import EventType
 from matter_server.common.models.node_device import AbstractMatterNodeDevice
+from matter_server.common.models.server_information import ServerInfo
 
 from homeassistant.core import callback
 from homeassistant.helpers.entity import DeviceInfo, Entity, EntityDescription
 
 from .const import DOMAIN
+from .helpers import get_device_id, get_operational_instance_id
 
 if TYPE_CHECKING:
     from matter_server.client import MatterClient
@@ -55,24 +57,20 @@ class MatterEntity(Entity):
         self._node_device = node_device
         self._device_type_instance = device_type_instance
         self.entity_description = entity_description
-        node = device_type_instance.node
         self._unsubscribes: list[Callable] = []
         # for fast lookups we create a mapping to the attribute paths
-        self._attributes_map: dict[type, str] = {}
-        server_info = matter_client.server_info
         # The server info is set when the client connects to the server.
-        assert server_info is not None
+        self._attributes_map: dict[type, str] = {}
+        server_info = cast(ServerInfo, self.matter_client.server_info)
+        # create unique_id based on "Operational Instance Name" and endpoint/device type
         self._attr_unique_id = (
-            f"{server_info.compressed_fabric_id}-"
+            f"{get_operational_instance_id(server_info, self._node_device.node())}-"
-            f"{node.unique_id}-"
             f"{device_type_instance.endpoint}-"
             f"{device_type_instance.device_type.device_type}"
         )
-
+        self._attr_device_info = DeviceInfo(
-    @property
+            identifiers={(DOMAIN, get_device_id(server_info, node_device))}
-    def device_info(self) -> DeviceInfo | None:
+        )
-        """Return device info for device registry."""
-        return {"identifiers": {(DOMAIN, self._node_device.device_info().uniqueID)}}
 
     async def async_added_to_hass(self) -> None:
         """Handle being added to Home Assistant."""

homeassistant/components/matter/helpers.py

@@ -10,6 +10,10 @@ from homeassistant.core import HomeAssistant, callback
 from .const import DOMAIN
 
 if TYPE_CHECKING:
+    from matter_server.common.models.node import MatterNode
+    from matter_server.common.models.node_device import AbstractMatterNodeDevice
+    from matter_server.common.models.server_information import ServerInfo
+
     from .adapter import MatterAdapter
 
 
@@ -29,3 +33,27 @@ def get_matter(hass: HomeAssistant) -> MatterAdapter:
     # In case of the config entry we need to fix this.
     matter_entry_data: MatterEntryData = next(iter(hass.data[DOMAIN].values()))
     return matter_entry_data.adapter
+
+
+def get_operational_instance_id(
+    server_info: ServerInfo,
+    node: MatterNode,
+) -> str:
+    """Return `Operational Instance Name` for given MatterNode."""
+    fabric_id_hex = f"{server_info.compressed_fabric_id:016X}"
+    node_id_hex = f"{node.node_id:016X}"
+    # operational instance id matches the mdns advertisement for the node
+    # this is the recommended ID to recognize a unique matter node (within a fabric)
+    return f"{fabric_id_hex}-{node_id_hex}"
+
+
+def get_device_id(
+    server_info: ServerInfo,
+    node_device: AbstractMatterNodeDevice,
+) -> str:
+    """Return HA device_id for the given MatterNodeDevice."""
+    operational_instance_id = get_operational_instance_id(
+        server_info, node_device.node()
+    )
+    # append nodedevice(type) to differentiate between a root node and bridge within HA devices.
+    return f"{operational_instance_id}-{node_device.__class__.__name__}"

tests/components/matter/test_adapter.py

@@ -27,8 +27,9 @@ async def test_device_registry_single_node_device(
     )
 
     dev_reg = dr.async_get(hass)
-
+    entry = dev_reg.async_get_device(
-    entry = dev_reg.async_get_device({(DOMAIN, "mock-onoff-light")})
+        {(DOMAIN, "00000000000004D2-0000000000000001-MatterNodeDevice")}
+    )
     assert entry is not None
 
     assert entry.name == "Mock OnOff Light"