diff --git a/homeassistant/components/thread/discovery.py b/homeassistant/components/thread/discovery.py index ce721a20e28..3395353b7bf 100644 --- a/homeassistant/components/thread/discovery.py +++ b/homeassistant/components/thread/discovery.py @@ -31,11 +31,11 @@ TYPE_PTR = 12 class ThreadRouterDiscoveryData: """Thread router discovery data.""" - addresses: list[str] | None + addresses: list[str] border_agent_id: str | None brand: str | None - extended_address: str | None - extended_pan_id: str | None + extended_address: str + extended_pan_id: str model_name: str | None network_name: str | None server: str | None @@ -46,6 +46,8 @@ class ThreadRouterDiscoveryData: def async_discovery_data_from_service( service: AsyncServiceInfo, + ext_addr: bytes, + ext_pan_id: bytes, ) -> ThreadRouterDiscoveryData: """Get a ThreadRouterDiscoveryData from an AsyncServiceInfo.""" @@ -64,8 +66,6 @@ def async_discovery_data_from_service( service_properties = cast(dict[bytes, bytes | None], service.properties) border_agent_id = service_properties.get(b"id") - ext_addr = service_properties.get(b"xa") - ext_pan_id = service_properties.get(b"xp") model_name = try_decode(service_properties.get(b"mn")) network_name = try_decode(service_properties.get(b"nn")) server = service.server @@ -90,8 +90,8 @@ def async_discovery_data_from_service( addresses=service.parsed_addresses(), border_agent_id=border_agent_id.hex() if border_agent_id is not None else None, brand=brand, - extended_address=ext_addr.hex() if ext_addr is not None else None, - extended_pan_id=ext_pan_id.hex() if ext_pan_id is not None else None, + extended_address=ext_addr.hex(), + extended_pan_id=ext_pan_id.hex(), model_name=model_name, network_name=network_name, server=server, @@ -121,7 +121,19 @@ def async_read_zeroconf_cache(aiozc: AsyncZeroconf) -> list[ThreadRouterDiscover # data is not fully in the cache, so ignore for now continue - results.append(async_discovery_data_from_service(info)) + # Service properties are always bytes if they are set from the network. + # For legacy backwards compatibility zeroconf allows properties to be set + # as strings but we never do that so we can safely cast here. + service_properties = cast(dict[bytes, bytes | None], info.properties) + + if not (xa := service_properties.get(b"xa")): + _LOGGER.debug("Ignoring record without xa %s", info) + continue + if not (xp := service_properties.get(b"xp")): + _LOGGER.debug("Ignoring record without xp %s", info) + continue + + results.append(async_discovery_data_from_service(info, xa, xp)) return results @@ -182,18 +194,22 @@ class ThreadRouterDiscovery: # as strings but we never do that so we can safely cast here. service_properties = cast(dict[bytes, bytes | None], service.properties) + # We need xa and xp, bail out if either is missing if not (xa := service_properties.get(b"xa")): - _LOGGER.debug("_add_update_service failed to find xa in %s", service) + _LOGGER.info( + "Discovered unsupported Thread router without extended address: %s", + service, + ) + return + if not (xp := service_properties.get(b"xp")): + _LOGGER.info( + "Discovered unsupported Thread router without extended pan ID: %s", + service, + ) return - # We use the extended mac address as key, bail out if it's missing - try: - extended_mac_address = xa.hex() - except UnicodeDecodeError as err: - _LOGGER.debug("_add_update_service failed to parse service %s", err) - return - - data = async_discovery_data_from_service(service) + data = async_discovery_data_from_service(service, xa, xp) + extended_mac_address = xa.hex() if name in self._known_routers and self._known_routers[name] == ( extended_mac_address, data, diff --git a/tests/components/thread/__init__.py b/tests/components/thread/__init__.py index f9d527919b4..7ca6cbaf2ed 100644 --- a/tests/components/thread/__init__.py +++ b/tests/components/thread/__init__.py @@ -150,6 +150,7 @@ ROUTER_DISCOVERY_HASS_MISSING_DATA = { "properties": { b"rv": b"1", b"id": b"#\x0cj\x1a\xc5\x7foK\xe2b\xac\xf3.^\xf5,", + # vn is missing b"mn": b"OpenThreadBorderRouter", b"nn": b"OpenThread HC", b"xp": b"\xe6\x0f\xc7\xc1\x86!,\xe5", @@ -167,7 +168,7 @@ ROUTER_DISCOVERY_HASS_MISSING_DATA = { } -ROUTER_DISCOVERY_HASS_MISSING_MANDATORY_DATA = { +ROUTER_DISCOVERY_HASS_MISSING_MANDATORY_DATA_XA = { "type_": "_meshcop._udp.local.", "name": "HomeAssistant OpenThreadBorderRouter #0BBF._meshcop._udp.local.", "addresses": [b"\xc0\xa8\x00s"], @@ -195,6 +196,34 @@ ROUTER_DISCOVERY_HASS_MISSING_MANDATORY_DATA = { } +ROUTER_DISCOVERY_HASS_MISSING_MANDATORY_DATA_XP = { + "type_": "_meshcop._udp.local.", + "name": "HomeAssistant OpenThreadBorderRouter #0BBF._meshcop._udp.local.", + "addresses": [b"\xc0\xa8\x00s"], + "port": 49153, + "weight": 0, + "priority": 0, + "server": "core-silabs-multiprotocol.local.", + "properties": { + b"rv": b"1", + b"id": b"#\x0cj\x1a\xc5\x7foK\xe2b\xac\xf3.^\xf5,", + b"vn": b"HomeAssistant", + b"mn": b"OpenThreadBorderRouter", + b"nn": b"OpenThread HC", + b"tv": b"1.3.0", + b"xa": b"\xae\xeb/YKW\x0b\xbf", + b"sb": b"\x00\x00\x01\xb1", + b"at": b"\x00\x00\x00\x00\x00\x01\x00\x00", + b"pt": b"\x8f\x06Q~", + b"sq": b"3", + b"bb": b"\xf0\xbf", + b"dn": b"DefaultDomain", + b"omr": b"@\xfd \xbe\x89IZ\x00\x01", + }, + "interface_index": None, +} + + ROUTER_DISCOVERY_HASS_NO_ACTIVE_TIMESTAMP = { "type_": "_meshcop._udp.local.", "name": "HomeAssistant OpenThreadBorderRouter #0BBF._meshcop._udp.local.", diff --git a/tests/components/thread/test_diagnostics.py b/tests/components/thread/test_diagnostics.py index a551315205b..94ca4373715 100644 --- a/tests/components/thread/test_diagnostics.py +++ b/tests/components/thread/test_diagnostics.py @@ -106,6 +106,48 @@ TEST_ZEROCONF_RECORD_4 = ServiceInfo( # Make sure this generates an invalid DNSPointer TEST_ZEROCONF_RECORD_4.name = "office._meshcop._udp.lo\x00cal." +# This has no XA +TEST_ZEROCONF_RECORD_5 = ServiceInfo( + type_="_meshcop._udp.local.", + name="bad_1._meshcop._udp.local.", + addresses=["127.0.0.1", "fe80::10ed:6406:4ee9:85e0"], + port=8080, + properties={ + "rv": "1", + "vn": "Apple", + "nn": "OpenThread HC", + "xp": "\xe6\x0f\xc7\xc1\x86!,\xe5", + "tv": "1.2.0", + "sb": "\x00\x00\x01\xb1", + "at": "\x00\x00\x00\x00\x00\x01\x00\x00", + "pt": "\x8f\x06Q~", + "sq": "3", + "bb": "\xf0\xbf", + "dn": "DefaultDomain", + }, +) + +# This has no XP +TEST_ZEROCONF_RECORD_6 = ServiceInfo( + type_="_meshcop._udp.local.", + name="bad_2._meshcop._udp.local.", + addresses=["127.0.0.1", "fe80::10ed:6406:4ee9:85e0"], + port=8080, + properties={ + "rv": "1", + "vn": "Apple", + "nn": "OpenThread HC", + "tv": "1.2.0", + "xa": "\xae\xeb/YKW\x0b\xbf", + "sb": "\x00\x00\x01\xb1", + "at": "\x00\x00\x00\x00\x00\x01\x00\x00", + "pt": "\x8f\x06Q~", + "sq": "3", + "bb": "\xf0\xbf", + "dn": "DefaultDomain", + }, +) + @dataclasses.dataclass class MockRoute: @@ -177,6 +219,24 @@ async def test_diagnostics( TEST_ZEROCONF_RECORD_4.dns_pointer(created=now), ] ) + # Test for record without xa + cache.async_add_records( + [ + *TEST_ZEROCONF_RECORD_5.dns_addresses(created=now), + TEST_ZEROCONF_RECORD_5.dns_service(created=now), + TEST_ZEROCONF_RECORD_5.dns_text(created=now), + TEST_ZEROCONF_RECORD_5.dns_pointer(created=now), + ] + ) + # Test for record without xp + cache.async_add_records( + [ + *TEST_ZEROCONF_RECORD_6.dns_addresses(created=now), + TEST_ZEROCONF_RECORD_6.dns_service(created=now), + TEST_ZEROCONF_RECORD_6.dns_text(created=now), + TEST_ZEROCONF_RECORD_6.dns_pointer(created=now), + ] + ) assert await async_setup_component(hass, DOMAIN, {}) await hass.async_block_till_done() diff --git a/tests/components/thread/test_discovery.py b/tests/components/thread/test_discovery.py index 4d43142b7b7..12eddb0b92a 100644 --- a/tests/components/thread/test_discovery.py +++ b/tests/components/thread/test_discovery.py @@ -16,7 +16,8 @@ from . import ( ROUTER_DISCOVERY_HASS_BAD_DATA, ROUTER_DISCOVERY_HASS_BAD_STATE_BITMAP, ROUTER_DISCOVERY_HASS_MISSING_DATA, - ROUTER_DISCOVERY_HASS_MISSING_MANDATORY_DATA, + ROUTER_DISCOVERY_HASS_MISSING_MANDATORY_DATA_XA, + ROUTER_DISCOVERY_HASS_MISSING_MANDATORY_DATA_XP, ROUTER_DISCOVERY_HASS_NO_ACTIVE_TIMESTAMP, ROUTER_DISCOVERY_HASS_NO_STATE_BITMAP, ROUTER_DISCOVERY_HASS_STATE_BITMAP_NOT_ACTIVE, @@ -152,7 +153,7 @@ async def test_discover_routers(hass: HomeAssistant, mock_async_zeroconf: None) async def test_discover_routers_unconfigured( hass: HomeAssistant, mock_async_zeroconf: None, data, unconfigured ) -> None: - """Test discovering thread routers with bad or missing vendor mDNS data.""" + """Test discovering thread routers and setting the unconfigured flag.""" mock_async_zeroconf.async_add_service_listener = AsyncMock() mock_async_zeroconf.async_remove_service_listener = AsyncMock() mock_async_zeroconf.async_get_service_info = AsyncMock() @@ -195,7 +196,7 @@ async def test_discover_routers_unconfigured( @pytest.mark.parametrize( "data", (ROUTER_DISCOVERY_HASS_BAD_DATA, ROUTER_DISCOVERY_HASS_MISSING_DATA) ) -async def test_discover_routers_bad_data( +async def test_discover_routers_bad_or_missing_optional_data( hass: HomeAssistant, mock_async_zeroconf: None, data ) -> None: """Test discovering thread routers with bad or missing vendor mDNS data.""" @@ -238,8 +239,15 @@ async def test_discover_routers_bad_data( ) -async def test_discover_routers_missing_mandatory_data( - hass: HomeAssistant, mock_async_zeroconf: None +@pytest.mark.parametrize( + "service", + [ + ROUTER_DISCOVERY_HASS_MISSING_MANDATORY_DATA_XA, + ROUTER_DISCOVERY_HASS_MISSING_MANDATORY_DATA_XP, + ], +) +async def test_discover_routers_bad_or_missing_mandatory_data( + hass: HomeAssistant, mock_async_zeroconf: None, service ) -> None: """Test discovering thread routers with missing mandatory mDNS data.""" mock_async_zeroconf.async_add_service_listener = AsyncMock() @@ -261,12 +269,12 @@ async def test_discover_routers_missing_mandatory_data( # Discover a service with missing mandatory data mock_async_zeroconf.async_get_service_info.return_value = AsyncServiceInfo( - **ROUTER_DISCOVERY_HASS_MISSING_MANDATORY_DATA + **service ) listener.add_service( None, - ROUTER_DISCOVERY_HASS_MISSING_MANDATORY_DATA["type_"], - ROUTER_DISCOVERY_HASS_MISSING_MANDATORY_DATA["name"], + service["type_"], + service["name"], ) await hass.async_block_till_done() router_discovered_removed.assert_not_called()