diff --git a/homeassistant/components/matter/adapter.py b/homeassistant/components/matter/adapter.py index 72246f72d003..c0d814ed15f6 100644 --- a/homeassistant/components/matter/adapter.py +++ b/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) diff --git a/tests/components/matter/test_adapter.py b/tests/components/matter/test_adapter.py index f454099c96fd..bed432b3efd6 100644 --- a/tests/components/matter/test_adapter.py +++ b/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