Fix some Matter bridged devices being merged into their bridge (#179564)

homeassistant/components/matter/adapter.py

@@ -209,16 +209,36 @@ class MatterAdapter:
             server_info,
             endpoint,
         )
-        identifiers = {(DOMAIN, f"{ID_TYPE_DEVICE_ID}_{node_device_id}")}
+        node_device_identifier = (DOMAIN, f"{ID_TYPE_DEVICE_ID}_{node_device_id}")
+        identifiers = {node_device_identifier}
         serial_number: str | None = None
-        # if available, we also add the serialnumber as identifier
+        # if available, we also add the serial number as identifier
         if (
-            basic_info_serial_number := basic_info.serialNumber
-        ) and "test" not in basic_info_serial_number.lower():
+            (basic_info_serial_number := basic_info.serialNumber)
+            and "test" not in basic_info_serial_number.lower()
+            # some bridges report their own serial number for the devices they bridge,
+            # which would make the identifier resolve to the bridge's device
+            and not (
+                isinstance(basic_info, clusters.BridgedDeviceBasicInformation)
+                and basic_info_serial_number == endpoint.node.device_info.serialNumber
+            )
+        ):
             # prefix identifier with 'serial_' to be able to filter it
             identifiers.add((DOMAIN, f"{ID_TYPE_SERIAL}_{basic_info_serial_number}"))
             serial_number = basic_info_serial_number
 
+        # the bridge's device entry keeps every identifier that was ever merged onto
+        # it, so a bridged device can still resolve to it through one left behind
+        if (
+            via_device_id is not UNDEFINED
+            and (via_device := device_registry.async_get(via_device_id))
+            and (stale_identifiers := via_device.identifiers & identifiers)
+        ):
+            device_registry.async_update_device(
+                via_device.id,
+                new_identifiers=via_device.identifiers - stale_identifiers,
+            )
+
         # Model name is the human readable name of the model/product name
         model_name = (
             # productLabel is optional but preferred (e.g. Hue Bloom)

tests/components/matter/test_adapter.py

@@ -10,7 +10,7 @@ from homeassistant.components.matter.adapter import get_clean_name
 from homeassistant.components.matter.const import DOMAIN, ID_TYPE_DEVICE_ID
 from homeassistant.components.matter.helpers import get_device_id
 from homeassistant.core import HomeAssistant
-from homeassistant.helpers import device_registry as dr
+from homeassistant.helpers import device_registry as dr, entity_registry as er
 
 from .common import create_node_from_fixture
 
@@ -329,6 +329,99 @@ async def test_composed_bridged_device_endpoint_removed(
     assert device_registry.async_get_device_by_identifier(identifier, entry_id) is None
 
 
+@pytest.mark.usefixtures("matter_node")
+@pytest.mark.parametrize("node_fixture", ["atios_knx_bridge"])
+@pytest.mark.parametrize(
+    "attributes", [{"29/57/15": "glg5mxh"}], ids=["bridge_serial_number"]
+)
+async def test_device_registry_bridged_device_with_bridge_serial_number(
+    hass: HomeAssistant,
+    device_registry: dr.DeviceRegistry,
+) -> None:
+    """Test a bridged device reporting the serial number of the bridge itself.
+
+    The serial number identifier must be skipped, as it would otherwise resolve
+    to the bridge's own device entry.
+    """
+    entry_id = hass.config_entries.async_entries(DOMAIN)[0].entry_id
+    bridge_entry = device_registry.async_get_device_by_identifier(
+        (DOMAIN, "deviceid_00000000000004D2-000000000000003E-MatterNodeDevice"),
+        entry_id,
+    )
+    assert bridge_entry is not None
+    assert (DOMAIN, "serial_glg5mxh") in bridge_entry.identifiers
+
+    bridged_entry = device_registry.async_get_device_by_identifier(
+        (DOMAIN, "deviceid_00000000000004D2-000000000000003E-29"), entry_id
+    )
+    assert bridged_entry is not None
+    assert bridged_entry.id != bridge_entry.id
+    assert bridged_entry.via_device_id == bridge_entry.id
+    assert (DOMAIN, "serial_glg5mxh") not in bridged_entry.identifiers
+    assert bridged_entry.serial_number is None
+
+
+async def test_device_registry_bridged_device_merged_into_bridge(
+    hass: HomeAssistant,
+    matter_client: MagicMock,
+    device_registry: dr.DeviceRegistry,
+    entity_registry: er.EntityRegistry,
+) -> None:
+    """Test a bridged device that was merged into the bridge's device is split off.
+
+    A bridged device reporting the serial number of the bridge used to be merged
+    into the bridge's device entry, which keeps resolving to it through the
+    identifier that is left behind there.
+    """
+    node = create_node_from_fixture("atios_knx_bridge", {"29/57/15": "glg5mxh"})
+    matter_client.get_nodes.return_value = [node]
+    config_entry = MockConfigEntry(
+        domain=DOMAIN, data={"url": "ws://localhost:5580/ws"}
+    )
+    config_entry.add_to_hass(hass)
+
+    merged_entry = device_registry.async_get_or_create(
+        config_entry_id=config_entry.entry_id,
+        identifiers={
+            (DOMAIN, "deviceid_00000000000004D2-000000000000003E-MatterNodeDevice"),
+            (DOMAIN, "deviceid_00000000000004D2-000000000000003E-29"),
+            (DOMAIN, "serial_glg5mxh"),
+        },
+    )
+    entity_entry = entity_registry.async_get_or_create(
+        "sensor",
+        DOMAIN,
+        "00000000000004D2-000000000000003E-29-29-ElectricalPowerMeasurementWatt-144-8",
+        config_entry=config_entry,
+        device_id=merged_entry.id,
+        suggested_object_id="electricity_monitor_ac_power",
+    )
+
+    assert await hass.config_entries.async_setup(config_entry.entry_id)
+    await hass.async_block_till_done()
+
+    bridge_entry = device_registry.async_get_device_by_identifier(
+        (DOMAIN, "deviceid_00000000000004D2-000000000000003E-MatterNodeDevice"),
+        config_entry.entry_id,
+    )
+    assert bridge_entry is not None
+    assert bridge_entry.id == merged_entry.id
+    assert (DOMAIN, "serial_glg5mxh") in bridge_entry.identifiers
+
+    bridged_entry = device_registry.async_get_device_by_identifier(
+        (DOMAIN, "deviceid_00000000000004D2-000000000000003E-29"), config_entry.entry_id
+    )
+    assert bridged_entry is not None
+    assert bridged_entry.id != bridge_entry.id
+    assert bridged_entry.via_device_id == bridge_entry.id
+    assert hass.states.get("sensor.electricity_monitor_ac_power")
+
+    # the entities of the bridged device move to the device it is split off into
+    assert (
+        entity_registry.async_get(entity_entry.entity_id).device_id == bridged_entry.id
+    )
+
+
 @pytest.mark.usefixtures("matter_node")
 @pytest.mark.parametrize("node_fixture", ["mock_air_purifier"])
 async def test_device_registry_single_node_composed_device(
@@ -375,3 +468,136 @@ async def test_bad_node_not_crash_integration(
     assert hass.states.get("light.mock_onoff_light") is not None
     assert len(hass.states.async_all("light")) == 1
     assert "Error setting up node" in caplog.text
+
+
+@pytest.mark.parametrize("node_fixture", ["mock_composed_bridge"])
+@pytest.mark.parametrize(
+    "attributes", [{"2/57/15": "MB-1234"}], ids=["bridge_serial_number"]
+)
+async def test_composed_bridged_device_with_bridge_serial_number(
+    hass: HomeAssistant,
+    device_registry: dr.DeviceRegistry,
+    matter_client: MagicMock,
+    matter_node: MatterNode,
+) -> None:
+    """Test a composed bridged device reporting the serial number of the bridge."""
+    entry_id = hass.config_entries.async_entries(DOMAIN)[0].entry_id
+    bridge_entry = device_registry.async_get_device_by_identifier(
+        identifier_for(matter_client, matter_node, 0), entry_id
+    )
+    assert bridge_entry is not None
+
+    # the part endpoints 3 and 4 are represented by the device of their compose parent
+    identifier = identifier_for(matter_client, matter_node, 2)
+    assert identifier_for(matter_client, matter_node, 3) == identifier
+    assert identifier_for(matter_client, matter_node, 4) == identifier
+
+    device_entry = device_registry.async_get_device_by_identifier(identifier, entry_id)
+    assert device_entry is not None
+    assert device_entry.id != bridge_entry.id
+    assert device_entry.via_device_id == bridge_entry.id
+    assert (DOMAIN, "serial_MB-1234") not in device_entry.identifiers
+
+
+async def test_device_registry_bridged_device_merged_with_changed_bridge_serial(
+    hass: HomeAssistant,
+    matter_client: MagicMock,
+    device_registry: dr.DeviceRegistry,
+) -> None:
+    """Test splitting off a merged bridged device after the bridge's serial changed.
+
+    The bridge's device entry keeps the shared serial number identifier, so the
+    bridged device has to be split off from it by all of its identifiers, not just
+    its device ID, or it resolves to the bridge again through that serial number.
+    """
+    node = create_node_from_fixture(
+        "atios_knx_bridge", {"0/40/15": "b7hcpwo", "29/57/15": "glg5mxh"}
+    )
+    matter_client.get_nodes.return_value = [node]
+    config_entry = MockConfigEntry(
+        domain=DOMAIN, data={"url": "ws://localhost:5580/ws"}
+    )
+    config_entry.add_to_hass(hass)
+
+    merged_entry = device_registry.async_get_or_create(
+        config_entry_id=config_entry.entry_id,
+        identifiers={
+            (DOMAIN, "deviceid_00000000000004D2-000000000000003E-MatterNodeDevice"),
+            (DOMAIN, "deviceid_00000000000004D2-000000000000003E-29"),
+            (DOMAIN, "serial_glg5mxh"),
+        },
+    )
+
+    assert await hass.config_entries.async_setup(config_entry.entry_id)
+    await hass.async_block_till_done()
+
+    bridge_entry = device_registry.async_get_device_by_identifier(
+        (DOMAIN, "deviceid_00000000000004D2-000000000000003E-MatterNodeDevice"),
+        config_entry.entry_id,
+    )
+    assert bridge_entry is not None
+    assert bridge_entry.id == merged_entry.id
+    # the bridge keeps its own serial number, the stale one goes to the bridged device
+    assert (DOMAIN, "serial_b7hcpwo") in bridge_entry.identifiers
+    assert (DOMAIN, "serial_glg5mxh") not in bridge_entry.identifiers
+
+    bridged_entry = device_registry.async_get_device_by_identifier(
+        (DOMAIN, "deviceid_00000000000004D2-000000000000003E-29"), config_entry.entry_id
+    )
+    assert bridged_entry is not None
+    assert bridged_entry.id != bridge_entry.id
+    assert bridged_entry.via_device_id == bridge_entry.id
+    assert (DOMAIN, "serial_glg5mxh") in bridged_entry.identifiers
+
+
+async def test_device_registry_bridged_device_split_off_with_changed_bridge_serial(
+    hass: HomeAssistant,
+    matter_client: MagicMock,
+    device_registry: dr.DeviceRegistry,
+) -> None:
+    """Test a bridged device that already has its own entry when the serial changes.
+
+    The bridge's device entry still holds the serial number it shared with the
+    bridged device, so that identifier has to be taken from it once the bridged
+    device reports it as its own again.
+    """
+    node = create_node_from_fixture(
+        "atios_knx_bridge", {"0/40/15": "b7hcpwo", "29/57/15": "glg5mxh"}
+    )
+    matter_client.get_nodes.return_value = [node]
+    config_entry = MockConfigEntry(
+        domain=DOMAIN, data={"url": "ws://localhost:5580/ws"}
+    )
+    config_entry.add_to_hass(hass)
+
+    bridge = device_registry.async_get_or_create(
+        config_entry_id=config_entry.entry_id,
+        identifiers={
+            (DOMAIN, "deviceid_00000000000004D2-000000000000003E-MatterNodeDevice"),
+            (DOMAIN, "serial_glg5mxh"),
+        },
+    )
+    device_registry.async_get_or_create(
+        config_entry_id=config_entry.entry_id,
+        identifiers={(DOMAIN, "deviceid_00000000000004D2-000000000000003E-29")},
+        via_device_id=bridge.id,
+    )
+
+    assert await hass.config_entries.async_setup(config_entry.entry_id)
+    await hass.async_block_till_done()
+
+    bridge_entry = device_registry.async_get_device_by_identifier(
+        (DOMAIN, "deviceid_00000000000004D2-000000000000003E-MatterNodeDevice"),
+        config_entry.entry_id,
+    )
+    assert bridge_entry is not None
+    assert (DOMAIN, "serial_b7hcpwo") in bridge_entry.identifiers
+    assert (DOMAIN, "serial_glg5mxh") not in bridge_entry.identifiers
+
+    bridged_entry = device_registry.async_get_device_by_identifier(
+        (DOMAIN, "deviceid_00000000000004D2-000000000000003E-29"), config_entry.entry_id
+    )
+    assert bridged_entry is not None
+    assert bridged_entry.id != bridge_entry.id
+    assert bridged_entry.via_device_id == bridge_entry.id
+    assert (DOMAIN, "serial_glg5mxh") in bridged_entry.identifiers