From 4bebf00598f5a5f98cda4240c86747389896f89f Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Wed, 18 Jan 2023 17:17:33 +0100 Subject: [PATCH] Adjust device registry for Matter devices (#86108) * adjust device registry * ignore test unique id * update test * ditch uniqueid + prefix serial * adjust test * add tests * fix switch test * prefix all identifiers * Update homeassistant/components/matter/adapter.py Co-authored-by: Martin Hjelmare * no underscore in id * fix test Co-authored-by: Martin Hjelmare --- homeassistant/components/matter/adapter.py | 26 ++++++++++------- homeassistant/components/matter/const.py | 4 +++ homeassistant/components/matter/entity.py | 5 ++-- .../fixtures/nodes/on-off-plugin-unit.json | 2 +- .../matter/fixtures/nodes/onoff-light.json | 2 +- tests/components/matter/test_adapter.py | 29 ++++++++++++++++++- tests/components/matter/test_switch.py | 10 +++---- 7 files changed, 57 insertions(+), 21 deletions(-) diff --git a/homeassistant/components/matter/adapter.py b/homeassistant/components/matter/adapter.py index 16e7d456212..08763e38327 100644 --- a/homeassistant/components/matter/adapter.py +++ b/homeassistant/components/matter/adapter.py @@ -17,7 +17,7 @@ from homeassistant.core import HomeAssistant from homeassistant.helpers import device_registry as dr from homeassistant.helpers.entity_platform import AddEntitiesCallback -from .const import DOMAIN, LOGGER +from .const import DOMAIN, ID_TYPE_DEVICE_ID, ID_TYPE_SERIAL, LOGGER from .device_platform import DEVICE_PLATFORM from .helpers import get_device_id @@ -91,10 +91,7 @@ class MatterAdapter: ) -> 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() @@ -103,16 +100,23 @@ class MatterAdapter: # 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]}" - f" {node_device.node().node_id}" - ) + # use the productName if no node label is present + name = basic_info.productName + + node_device_id = get_device_id( + server_info, + node_device, + ) + identifiers = {(DOMAIN, f"{ID_TYPE_DEVICE_ID}_{node_device_id}")} + # if available, we also add the serialnumber as identifier + if basic_info.serialNumber and "test" not in basic_info.serialNumber.lower(): + # prefix identifier with 'serial_' to be able to filter it + identifiers.add((DOMAIN, f"{ID_TYPE_SERIAL}_{basic_info.serialNumber}")) dr.async_get(self.hass).async_get_or_create( name=name, config_entry_id=self.config_entry.entry_id, - identifiers={(DOMAIN, node_unique_id)}, + identifiers=identifiers, hw_version=basic_info.hardwareVersionString, sw_version=basic_info.softwareVersionString, manufacturer=basic_info.vendorName, diff --git a/homeassistant/components/matter/const.py b/homeassistant/components/matter/const.py index c5ec1173ac0..e7f96bd2448 100644 --- a/homeassistant/components/matter/const.py +++ b/homeassistant/components/matter/const.py @@ -8,3 +8,7 @@ CONF_USE_ADDON = "use_addon" DOMAIN = "matter" LOGGER = logging.getLogger(__package__) + +# prefixes to identify device identifier id types +ID_TYPE_DEVICE_ID = "deviceid" +ID_TYPE_SERIAL = "serial" diff --git a/homeassistant/components/matter/entity.py b/homeassistant/components/matter/entity.py index f239cec0342..820d0f72846 100644 --- a/homeassistant/components/matter/entity.py +++ b/homeassistant/components/matter/entity.py @@ -15,7 +15,7 @@ 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 .const import DOMAIN, ID_TYPE_DEVICE_ID from .helpers import get_device_id, get_operational_instance_id if TYPE_CHECKING: @@ -68,8 +68,9 @@ class MatterEntity(Entity): f"{device_type_instance.endpoint}-" f"{device_type_instance.device_type.device_type}" ) + node_device_id = get_device_id(server_info, node_device) self._attr_device_info = DeviceInfo( - identifiers={(DOMAIN, get_device_id(server_info, node_device))} + identifiers={(DOMAIN, f"{ID_TYPE_DEVICE_ID}_{node_device_id}")} ) async def async_added_to_hass(self) -> None: diff --git a/tests/components/matter/fixtures/nodes/on-off-plugin-unit.json b/tests/components/matter/fixtures/nodes/on-off-plugin-unit.json index cbbe39b1f09..e26450a9a28 100644 --- a/tests/components/matter/fixtures/nodes/on-off-plugin-unit.json +++ b/tests/components/matter/fixtures/nodes/on-off-plugin-unit.json @@ -175,7 +175,7 @@ "attribute_id": 5, "attribute_type": "chip.clusters.Objects.Basic.Attributes.NodeLabel", "attribute_name": "NodeLabel", - "value": "Mock OnOff Plugin Unit" + "value": "" }, "0/40/6": { "node_id": 1, diff --git a/tests/components/matter/fixtures/nodes/onoff-light.json b/tests/components/matter/fixtures/nodes/onoff-light.json index cc6521aa2e3..340d7cb71c9 100644 --- a/tests/components/matter/fixtures/nodes/onoff-light.json +++ b/tests/components/matter/fixtures/nodes/onoff-light.json @@ -469,7 +469,7 @@ "attribute_id": 15, "attribute_type": "chip.clusters.Objects.Basic.Attributes.SerialNumber", "attribute_name": "SerialNumber", - "value": "TEST_SN" + "value": "12345678" }, "0/40/16": { "node_id": 1, diff --git a/tests/components/matter/test_adapter.py b/tests/components/matter/test_adapter.py index c89b45e4c0b..f83f21dc5e0 100644 --- a/tests/components/matter/test_adapter.py +++ b/tests/components/matter/test_adapter.py @@ -28,10 +28,13 @@ async def test_device_registry_single_node_device( dev_reg = dr.async_get(hass) entry = dev_reg.async_get_device( - {(DOMAIN, "00000000000004D2-0000000000000001-MatterNodeDevice")} + {(DOMAIN, "deviceid_00000000000004D2-0000000000000001-MatterNodeDevice")} ) assert entry is not None + # test serial id present as additional identifier + assert (DOMAIN, "serial_12345678") in entry.identifiers + assert entry.name == "Mock OnOff Light" assert entry.manufacturer == "Nabu Casa" assert entry.model == "Mock Light" @@ -39,6 +42,30 @@ async def test_device_registry_single_node_device( assert entry.sw_version == "v1.0" +async def test_device_registry_single_node_device_alt( + hass: HomeAssistant, + matter_client: MagicMock, +) -> None: + """Test additional device with different attribute values.""" + await setup_integration_with_node_fixture( + hass, + "on-off-plugin-unit", + matter_client, + ) + + dev_reg = dr.async_get(hass) + entry = dev_reg.async_get_device( + {(DOMAIN, "deviceid_00000000000004D2-0000000000000001-MatterNodeDevice")} + ) + assert entry is not None + + # test name is derived from productName (because nodeLabel is absent) + assert entry.name == "Mock OnOffPluginUnit (powerplug/switch)" + + # test serial id NOT present as additional identifier + assert (DOMAIN, "serial_TEST_SN") not in entry.identifiers + + @pytest.mark.skip("Waiting for a new test fixture") async def test_device_registry_bridge( hass: HomeAssistant, diff --git a/tests/components/matter/test_switch.py b/tests/components/matter/test_switch.py index a79edd6010b..9fe225b1b13 100644 --- a/tests/components/matter/test_switch.py +++ b/tests/components/matter/test_switch.py @@ -30,7 +30,7 @@ async def test_turn_on( switch_node: MatterNode, ) -> None: """Test turning on a switch.""" - state = hass.states.get("switch.mock_onoff_plugin_unit") + state = hass.states.get("switch.mock_onoffpluginunit_powerplug_switch") assert state assert state.state == "off" @@ -38,7 +38,7 @@ async def test_turn_on( "switch", "turn_on", { - "entity_id": "switch.mock_onoff_plugin_unit", + "entity_id": "switch.mock_onoffpluginunit_powerplug_switch", }, blocking=True, ) @@ -53,7 +53,7 @@ async def test_turn_on( set_node_attribute(switch_node, 1, 6, 0, True) await trigger_subscription_callback(hass, matter_client) - state = hass.states.get("switch.mock_onoff_plugin_unit") + state = hass.states.get("switch.mock_onoffpluginunit_powerplug_switch") assert state assert state.state == "on" @@ -64,7 +64,7 @@ async def test_turn_off( switch_node: MatterNode, ) -> None: """Test turning off a switch.""" - state = hass.states.get("switch.mock_onoff_plugin_unit") + state = hass.states.get("switch.mock_onoffpluginunit_powerplug_switch") assert state assert state.state == "off" @@ -72,7 +72,7 @@ async def test_turn_off( "switch", "turn_off", { - "entity_id": "switch.mock_onoff_plugin_unit", + "entity_id": "switch.mock_onoffpluginunit_powerplug_switch", }, blocking=True, )