From ee375ff42d062cfb42e793a671bbf5ec72cab9e9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 4 Jan 2022 05:51:38 -1000 Subject: [PATCH] Offer local control option when there are multiple zeroconf homekit matches (#62649) --- homeassistant/components/zeroconf/__init__.py | 53 +++++++++++++------ tests/components/zeroconf/test_init.py | 42 +++++++++++++-- 2 files changed, 76 insertions(+), 19 deletions(-) diff --git a/homeassistant/components/zeroconf/__init__.py b/homeassistant/components/zeroconf/__init__.py index 98d371d2d2f..1dc70cde610 100644 --- a/homeassistant/components/zeroconf/__init__.py +++ b/homeassistant/components/zeroconf/__init__.py @@ -2,6 +2,7 @@ from __future__ import annotations import asyncio +import contextlib from contextlib import suppress from dataclasses import dataclass import fnmatch @@ -32,7 +33,13 @@ import homeassistant.helpers.config_validation as cv from homeassistant.helpers.frame import report from homeassistant.helpers.network import NoURLAvailableError, get_url from homeassistant.helpers.typing import ConfigType -from homeassistant.loader import async_get_homekit, async_get_zeroconf, bind_hass +from homeassistant.loader import ( + Integration, + async_get_homekit, + async_get_integration, + async_get_zeroconf, + bind_hass, +) from .models import HaAsyncServiceBrowser, HaAsyncZeroconf, HaZeroconf from .usage import install_multiple_zeroconf_catcher @@ -72,7 +79,6 @@ ATTR_PROPERTIES: Final = "properties" # Attributes for ZeroconfServiceInfo[ATTR_PROPERTIES] ATTR_PROPERTIES_ID: Final = "id" - CONFIG_SCHEMA = vol.Schema( { DOMAIN: vol.All( @@ -340,6 +346,17 @@ def _match_against_props(matcher: dict[str, str], props: dict[str, str]) -> bool ) +def is_homekit_paired(props: dict[str, Any]) -> bool: + """Check properties to see if a device is homekit paired.""" + if HOMEKIT_PAIRED_STATUS_FLAG not in props: + return False + with contextlib.suppress(ValueError): + # 0 means paired and not discoverable by iOS clients) + return int(props[HOMEKIT_PAIRED_STATUS_FLAG]) == 0 + # If we cannot tell, we assume its not paired + return False + + class ZeroconfDiscovery: """Discovery via zeroconf.""" @@ -417,11 +434,12 @@ class ZeroconfDiscovery: props: dict[str, str] = info.properties # If we can handle it as a HomeKit discovery, we do that here. - if service_type in HOMEKIT_TYPES: - if domain := async_get_homekit_discovery_domain(self.homekit_models, props): - discovery_flow.async_create_flow( - self.hass, domain, {"source": config_entries.SOURCE_HOMEKIT}, info - ) + if service_type in HOMEKIT_TYPES and ( + domain := async_get_homekit_discovery_domain(self.homekit_models, props) + ): + discovery_flow.async_create_flow( + self.hass, domain, {"source": config_entries.SOURCE_HOMEKIT}, info + ) # Continue on here as homekit_controller # still needs to get updates on devices # so it can see when the 'c#' field is updated. @@ -429,14 +447,19 @@ class ZeroconfDiscovery: # We only send updates to homekit_controller # if the device is already paired in order to avoid # offering a second discovery for the same device - if domain and HOMEKIT_PAIRED_STATUS_FLAG in props: - try: - # 0 means paired and not discoverable by iOS clients) - if int(props[HOMEKIT_PAIRED_STATUS_FLAG]): - return - except ValueError: - # HomeKit pairing status unknown - # likely bad homekit data + if not is_homekit_paired(props): + integration: Integration = await async_get_integration( + self.hass, domain + ) + # Since we prefer local control, if the integration that is being discovered + # is cloud AND the homekit device is UNPAIRED we still want to discovery it. + # + # As soon as the device becomes paired, the config flow will be dismissed + # in the event the user does not want to pair with Home Assistant. + # + if not integration.iot_class or not integration.iot_class.startswith( + "cloud" + ): return match_data: dict[str, str] = {} diff --git a/tests/components/zeroconf/test_init.py b/tests/components/zeroconf/test_init.py index 0482f38415b..24b6ec97ec6 100644 --- a/tests/components/zeroconf/test_init.py +++ b/tests/components/zeroconf/test_init.py @@ -541,7 +541,7 @@ async def test_homekit_match_partial_dash(hass, mock_async_zeroconf): ), ) as mock_service_browser, patch( "homeassistant.components.zeroconf.AsyncServiceInfo", - side_effect=get_homekit_info_mock("Rachio-fa46ba", HOMEKIT_STATUS_UNPAIRED), + side_effect=get_homekit_info_mock("Smart Bridge-001", HOMEKIT_STATUS_UNPAIRED), ): assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) @@ -549,7 +549,7 @@ async def test_homekit_match_partial_dash(hass, mock_async_zeroconf): assert len(mock_service_browser.mock_calls) == 1 assert len(mock_config_flow.mock_calls) == 1 - assert mock_config_flow.mock_calls[0][1][0] == "rachio" + assert mock_config_flow.mock_calls[0][1][0] == "lutron_caseta" async def test_homekit_match_partial_fnmatch(hass, mock_async_zeroconf): @@ -650,7 +650,7 @@ async def test_homekit_invalid_paring_status(hass, mock_async_zeroconf): ), ) as mock_service_browser, patch( "homeassistant.components.zeroconf.AsyncServiceInfo", - side_effect=get_homekit_info_mock("tado", b"invalid"), + side_effect=get_homekit_info_mock("Smart Bridge", b"invalid"), ): assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) @@ -658,7 +658,7 @@ async def test_homekit_invalid_paring_status(hass, mock_async_zeroconf): assert len(mock_service_browser.mock_calls) == 1 assert len(mock_config_flow.mock_calls) == 1 - assert mock_config_flow.mock_calls[0][1][0] == "tado" + assert mock_config_flow.mock_calls[0][1][0] == "lutron_caseta" async def test_homekit_not_paired(hass, mock_async_zeroconf): @@ -686,6 +686,40 @@ async def test_homekit_not_paired(hass, mock_async_zeroconf): assert mock_config_flow.mock_calls[0][1][0] == "homekit_controller" +async def test_homekit_controller_still_discovered_unpaired_for_cloud( + hass, mock_async_zeroconf +): + """Test discovery is still passed to homekit controller when unpaired and discovered by cloud integration. + + Since we prefer local control, if the integration that is being discovered + is cloud AND the homekit device is unpaired we still want to discovery it + """ + with patch.dict( + zc_gen.ZEROCONF, + {"_hap._udp.local.": [{"domain": "homekit_controller"}]}, + clear=True, + ), patch.object( + hass.config_entries.flow, "async_init" + ) as mock_config_flow, patch.object( + zeroconf, + "HaAsyncServiceBrowser", + side_effect=lambda *args, **kwargs: service_update_mock( + *args, **kwargs, limit_service="_hap._udp.local." + ), + ) as mock_service_browser, patch( + "homeassistant.components.zeroconf.AsyncServiceInfo", + side_effect=get_homekit_info_mock("Rachio-xyz", HOMEKIT_STATUS_UNPAIRED), + ): + assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) + hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) + await hass.async_block_till_done() + + assert len(mock_service_browser.mock_calls) == 1 + assert len(mock_config_flow.mock_calls) == 2 + assert mock_config_flow.mock_calls[0][1][0] == "rachio" + assert mock_config_flow.mock_calls[1][1][0] == "homekit_controller" + + async def test_info_from_service_non_utf8(hass): """Test info_from_service handles non UTF-8 property keys and values correctly.""" service_type = "_test._tcp.local."