From edf89b21c98e258c2b51945630b9ba5311de7c8d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 16 May 2021 18:02:34 -1000 Subject: [PATCH 01/11] wip --- homeassistant/components/zeroconf/__init__.py | 155 +++++++++++------- tests/components/zeroconf/test_init.py | 52 ++++-- 2 files changed, 131 insertions(+), 76 deletions(-) diff --git a/homeassistant/components/zeroconf/__init__.py b/homeassistant/components/zeroconf/__init__.py index 484d4404a66a6a..60e938b7e34ec8 100644 --- a/homeassistant/components/zeroconf/__init__.py +++ b/homeassistant/components/zeroconf/__init__.py @@ -1,6 +1,7 @@ """Support for exposing Home Assistant via Zeroconf.""" from __future__ import annotations +import asyncio from collections.abc import Iterable from contextlib import suppress import fnmatch @@ -13,7 +14,6 @@ from pyroute2 import IPRoute import voluptuous as vol from zeroconf import ( - Error as ZeroconfError, InterfaceChoice, IPVersion, NonUniqueNameException, @@ -29,7 +29,7 @@ EVENT_HOMEASSISTANT_STOP, __version__, ) -from homeassistant.core import Event, HomeAssistant +from homeassistant.core import Event, HomeAssistant, callback import homeassistant.helpers.config_validation as cv from homeassistant.helpers.network import NoURLAvailableError, get_url from homeassistant.loader import async_get_homekit, async_get_zeroconf, bind_hass @@ -90,6 +90,14 @@ class HaServiceInfo(TypedDict): properties: dict[str, Any] +class ZeroconfFlow(TypedDict): + """A queued zeroconf discovery flow.""" + + domain: str + context: dict[str, Any] + data: HaServiceInfo + + @bind_hass async def async_get_instance(hass: HomeAssistant) -> HaZeroconf: """Zeroconf instance to be shared with other integrations that use it.""" @@ -183,6 +191,12 @@ async def async_setup(hass: HomeAssistant, config: dict) -> bool: aio_zc = await _async_get_instance(hass, **zc_args) zeroconf = aio_zc.zeroconf + zeroconf_types, homekit_models = await asyncio.gather( + async_get_zeroconf(hass), async_get_homekit(hass) + ) + discovery = ZeroconfDiscovery(hass, zeroconf, zeroconf_types, homekit_models) + await discovery.async_setup() + async def _async_zeroconf_hass_start(_event: Event) -> None: """Expose Home Assistant on zeroconf when it starts. @@ -191,15 +205,17 @@ async def _async_zeroconf_hass_start(_event: Event) -> None: uuid = await hass.helpers.instance_id.async_get() await _async_register_hass_zc_service(hass, aio_zc, uuid) - async def _async_zeroconf_hass_started(_event: Event) -> None: - """Start the service browser.""" + @callback + def _async_start_discovery(_event: Event) -> None: + """Start processing flows.""" + discovery.async_start() - await _async_start_zeroconf_browser(hass, zeroconf) + async def _async_zeroconf_hass_stop(_event: Event) -> None: + await discovery.async_stop() + hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, _async_zeroconf_hass_stop) hass.bus.async_listen_once(EVENT_HOMEASSISTANT_START, _async_zeroconf_hass_start) - hass.bus.async_listen_once( - EVENT_HOMEASSISTANT_STARTED, _async_zeroconf_hass_started - ) + hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STARTED, _async_start_discovery) return True @@ -259,44 +275,73 @@ async def _async_register_hass_zc_service( ) -async def _async_start_zeroconf_browser( - hass: HomeAssistant, zeroconf: Zeroconf -) -> None: - """Start the zeroconf browser.""" - - zeroconf_types = await async_get_zeroconf(hass) - homekit_models = await async_get_homekit(hass) +class ZeroconfDiscovery: + """Discovery via zeroconf.""" - types = list(zeroconf_types) + def __init__( + self, + hass: HomeAssistant, + zeroconf: Zeroconf, + zeroconf_types: dict[str, list[dict[str, str]]], + homekit_models: dict[str, str], + ) -> None: + """Init discovery.""" + self.hass = hass + self.zeroconf = zeroconf + self.zeroconf_types = zeroconf_types + self.homekit_models = homekit_models + + self.service_browser: HaServiceBrowser | None = None + self.queue: asyncio.Queue | None = None + self.process_task: asyncio.Task | None = None + + async def async_setup(self) -> None: + """Start discovery.""" + self.queue = asyncio.Queue() + types = list(self.zeroconf_types) + for hk_type in HOMEKIT_TYPES: + if hk_type not in self.zeroconf_types: + types.append(hk_type) + _LOGGER.debug("Starting Zeroconf browser") + self.service_browser = HaServiceBrowser( + self.zeroconf, types, handlers=[self.service_update] + ) - for hk_type in HOMEKIT_TYPES: - if hk_type not in zeroconf_types: - types.append(hk_type) + async def async_stop(self) -> None: + """Cancel the service browser and stop processing the queue.""" + if self.service_browser: + await self.hass.async_add_executor_job(self.service_browser.cancel) + if self.process_task: + self.process_task.cancel() + + @callback + def async_start(self) -> None: + """Start processing discovery flows.""" + self.process_task = asyncio.create_task(self._async_process()) + + async def _async_process(self) -> None: + assert self.queue is not None + while flow := await self.queue.get(): + self.hass.async_create_task( + self.hass.config_entries.flow.async_init( + flow["domain"], context=flow["context"], data=flow["data"] + ) + ) + self.queue.task_done() def service_update( + self, zeroconf: Zeroconf, service_type: str, name: str, state_change: ServiceStateChange, ) -> None: """Service state changed.""" - nonlocal zeroconf_types - nonlocal homekit_models - if state_change == ServiceStateChange.Removed: return - try: - service_info = zeroconf.get_service_info(service_type, name) - except ZeroconfError: - _LOGGER.exception("Failed to get info for device %s", name) - return - - if not service_info: - # Prevent the browser thread from collapsing as - # service_info can be None - _LOGGER.debug("Failed to get info for device %s", name) - return + service_info = ServiceInfo(service_type, name) + service_info.load_from_cache(zeroconf) info = info_from_service(service_info) if not info: @@ -305,10 +350,12 @@ def service_update( return _LOGGER.debug("Discovered new device %s %s", name, info) + assert self.queue is not None # If we can handle it as a HomeKit discovery, we do that here. if service_type in HOMEKIT_TYPES: - discovery_was_forwarded = handle_homekit(hass, homekit_models, info) + if pending_flow := handle_homekit(self.hass, self.homekit_models, info): + self.hass.loop.call_soon_threadsafe(self.queue.put_nowait, pending_flow) # Continue on here as homekit_controller # still needs to get updates on devices # so it can see when the 'c#' field is updated. @@ -316,10 +363,7 @@ def service_update( # 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 ( - discovery_was_forwarded - and HOMEKIT_PAIRED_STATUS_FLAG in info["properties"] - ): + if pending_flow and HOMEKIT_PAIRED_STATUS_FLAG in info["properties"]: try: # 0 means paired and not discoverable by iOS clients) if int(info["properties"][HOMEKIT_PAIRED_STATUS_FLAG]): @@ -348,7 +392,7 @@ def service_update( # Not all homekit types are currently used for discovery # so not all service type exist in zeroconf_types - for matcher in zeroconf_types.get(service_type, []): + for matcher in self.zeroconf_types.get(service_type, []): if len(matcher) > 1: if "macaddress" in matcher and ( uppercase_mac is None @@ -368,19 +412,17 @@ def service_update( ): continue - hass.add_job( - hass.config_entries.flow.async_init( - matcher["domain"], context={"source": DOMAIN}, data=info - ) # type: ignore - ) - - _LOGGER.debug("Starting Zeroconf browser") - HaServiceBrowser(zeroconf, types, handlers=[service_update]) + flow: ZeroconfFlow = { + "domain": matcher["domain"], + "context": {"source": config_entries.SOURCE_ZEROCONF}, + "data": info, + } + self.hass.loop.call_soon_threadsafe(self.queue.put_nowait, flow) def handle_homekit( hass: HomeAssistant, homekit_models: dict[str, str], info: HaServiceInfo -) -> bool: +) -> ZeroconfFlow | None: """Handle a HomeKit discovery. Return if discovery was forwarded. @@ -394,7 +436,7 @@ def handle_homekit( break if model is None: - return False + return None for test_model in homekit_models: if ( @@ -404,16 +446,13 @@ def handle_homekit( ): continue - hass.add_job( - hass.config_entries.flow.async_init( - homekit_models[test_model], - context={"source": config_entries.SOURCE_HOMEKIT}, - data=info, - ) # type: ignore - ) - return True + return { + "domain": homekit_models[test_model], + "context": {"source": config_entries.SOURCE_HOMEKIT}, + "data": info, + } - return False + return None def info_from_service(service: ServiceInfo) -> HaServiceInfo | None: diff --git a/tests/components/zeroconf/test_init.py b/tests/components/zeroconf/test_init.py index 809177b6089af3..6af1e554f6cea4 100644 --- a/tests/components/zeroconf/test_init.py +++ b/tests/components/zeroconf/test_init.py @@ -149,8 +149,10 @@ async def test_setup(hass, mock_zeroconf): hass.config_entries.flow, "async_init" ) as mock_config_flow, patch.object( zeroconf, "HaServiceBrowser", side_effect=service_update_mock - ) as mock_service_browser: - mock_zeroconf.get_service_info.side_effect = get_service_info_mock + ) as mock_service_browser, patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_service_info_mock, + ): assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -181,8 +183,10 @@ async def test_setup_with_overly_long_url_and_name(hass, mock_zeroconf, caplog): hass.config, "location_name", "\u00dcBER \u00dcber German Umlaut long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string", + ), patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_service_info_mock, ): - mock_zeroconf.get_service_info.side_effect = get_service_info_mock assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_START) await hass.async_block_till_done() @@ -195,8 +199,10 @@ async def test_setup_with_default_interface(hass, mock_zeroconf): """Test default interface config.""" with patch.object(hass.config_entries.flow, "async_init"), patch.object( zeroconf, "HaServiceBrowser", side_effect=service_update_mock + ), patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_service_info_mock, ): - mock_zeroconf.get_service_info.side_effect = get_service_info_mock assert await async_setup_component( hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {CONF_DEFAULT_INTERFACE: True}} ) @@ -210,8 +216,10 @@ async def test_setup_without_default_interface(hass, mock_zeroconf): """Test without default interface config.""" with patch.object(hass.config_entries.flow, "async_init"), patch.object( zeroconf, "HaServiceBrowser", side_effect=service_update_mock + ), patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_service_info_mock, ): - mock_zeroconf.get_service_info.side_effect = get_service_info_mock assert await async_setup_component( hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {CONF_DEFAULT_INTERFACE: False}} ) @@ -223,8 +231,10 @@ async def test_setup_without_ipv6(hass, mock_zeroconf): """Test without ipv6.""" with patch.object(hass.config_entries.flow, "async_init"), patch.object( zeroconf, "HaServiceBrowser", side_effect=service_update_mock + ), patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_service_info_mock, ): - mock_zeroconf.get_service_info.side_effect = get_service_info_mock assert await async_setup_component( hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {CONF_IPV6: False}} ) @@ -238,8 +248,10 @@ async def test_setup_with_ipv6(hass, mock_zeroconf): """Test without ipv6.""" with patch.object(hass.config_entries.flow, "async_init"), patch.object( zeroconf, "HaServiceBrowser", side_effect=service_update_mock + ), patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_service_info_mock, ): - mock_zeroconf.get_service_info.side_effect = get_service_info_mock assert await async_setup_component( hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {CONF_IPV6: True}} ) @@ -253,8 +265,10 @@ async def test_setup_with_ipv6_default(hass, mock_zeroconf): """Test without ipv6 as default.""" with patch.object(hass.config_entries.flow, "async_init"), patch.object( zeroconf, "HaServiceBrowser", side_effect=service_update_mock + ), patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_service_info_mock, ): - mock_zeroconf.get_service_info.side_effect = get_service_info_mock assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -266,8 +280,10 @@ async def test_service_with_invalid_name(hass, mock_zeroconf, caplog): """Test we do not crash on service with an invalid name.""" with patch.object( zeroconf, "HaServiceBrowser", side_effect=service_update_mock - ) as mock_service_browser: - mock_zeroconf.get_service_info.side_effect = BadTypeInNameException + ) as mock_service_browser, patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=BadTypeInNameException, + ): assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -300,10 +316,10 @@ def http_only_service_update_mock(zeroconf, services, handlers): hass.config_entries.flow, "async_init" ) as mock_config_flow, patch.object( zeroconf, "HaServiceBrowser", side_effect=http_only_service_update_mock - ) as mock_service_browser: - mock_zeroconf.get_service_info.side_effect = get_zeroconf_info_mock( - "FFAADDCC11DD" - ) + ) as mock_service_browser, patch( + "homeassistant.components.zeroconf.ServiceInfo", + return_value=get_service_info_mock("FFAADDCC11DD"), + ): assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -537,10 +553,10 @@ async def test_homekit_already_paired(hass, mock_zeroconf): side_effect=lambda *args, **kwargs: service_update_mock( *args, **kwargs, limit_service="_hap._tcp.local." ), - ) as mock_service_browser: - mock_zeroconf.get_service_info.side_effect = get_homekit_info_mock( - "tado", HOMEKIT_STATUS_PAIRED - ) + ) as mock_service_browser, patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_homekit_info_mock("tado", HOMEKIT_STATUS_PAIRED), + ): assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() From a5f30175eb4b877a06e25c7ef09a434125e9b07c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 17 May 2021 05:54:09 -1000 Subject: [PATCH 02/11] wip --- homeassistant/components/zeroconf/__init__.py | 59 ++++++++++++------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/homeassistant/components/zeroconf/__init__.py b/homeassistant/components/zeroconf/__init__.py index 60e938b7e34ec8..a1ea921a2381fc 100644 --- a/homeassistant/components/zeroconf/__init__.py +++ b/homeassistant/components/zeroconf/__init__.py @@ -9,7 +9,7 @@ from ipaddress import ip_address import logging import socket -from typing import Any, TypedDict, cast +from typing import Any, Coroutine, TypedDict, cast from pyroute2 import IPRoute import voluptuous as vol @@ -275,6 +275,37 @@ async def _async_register_hass_zc_service( ) +class FlowDispatcher: + """Dispatch discovery flows.""" + + def __init__(self, hass: HomeAssistant): + """Init the discovery dispatcher.""" + self.hass = hass + self.pending_flows: list[ZeroconfFlow] = [] + self.started = False + + @callback + def async_start(self) -> None: + """Start processing pending flows.""" + self.started = True + for flow in list(self.pending_flows): + self.hass.async_create_task(self._init_flow(flow)) + self.pending_flows = [] + + def create(self, flow: ZeroconfFlow) -> None: + """Create and add or queue a flow.""" + if self.started: + self.hass.add_job(self._init_flow(flow)) # type: ignore + else: + self.pending_flows.append(flow) + + def _init_flow(self, flow: ZeroconfFlow) -> Coroutine[Any, Any, Any]: + """Create a flow.""" + return self.hass.config_entries.flow.async_init( + flow["domain"], context=flow["context"], data=flow["data"] + ) + + class ZeroconfDiscovery: """Discovery via zeroconf.""" @@ -291,13 +322,12 @@ def __init__( self.zeroconf_types = zeroconf_types self.homekit_models = homekit_models + self.flow_dispatcher: FlowDispatcher | None = None self.service_browser: HaServiceBrowser | None = None - self.queue: asyncio.Queue | None = None - self.process_task: asyncio.Task | None = None async def async_setup(self) -> None: """Start discovery.""" - self.queue = asyncio.Queue() + self.flow_dispatcher = FlowDispatcher(self.hass) types = list(self.zeroconf_types) for hk_type in HOMEKIT_TYPES: if hk_type not in self.zeroconf_types: @@ -311,23 +341,12 @@ async def async_stop(self) -> None: """Cancel the service browser and stop processing the queue.""" if self.service_browser: await self.hass.async_add_executor_job(self.service_browser.cancel) - if self.process_task: - self.process_task.cancel() @callback def async_start(self) -> None: """Start processing discovery flows.""" - self.process_task = asyncio.create_task(self._async_process()) - - async def _async_process(self) -> None: - assert self.queue is not None - while flow := await self.queue.get(): - self.hass.async_create_task( - self.hass.config_entries.flow.async_init( - flow["domain"], context=flow["context"], data=flow["data"] - ) - ) - self.queue.task_done() + assert self.flow_dispatcher is not None + self.flow_dispatcher.async_start() def service_update( self, @@ -350,12 +369,12 @@ def service_update( return _LOGGER.debug("Discovered new device %s %s", name, info) - assert self.queue is not None + assert self.flow_dispatcher is not None # If we can handle it as a HomeKit discovery, we do that here. if service_type in HOMEKIT_TYPES: if pending_flow := handle_homekit(self.hass, self.homekit_models, info): - self.hass.loop.call_soon_threadsafe(self.queue.put_nowait, pending_flow) + self.flow_dispatcher.create(pending_flow) # Continue on here as homekit_controller # still needs to get updates on devices # so it can see when the 'c#' field is updated. @@ -417,7 +436,7 @@ def service_update( "context": {"source": config_entries.SOURCE_ZEROCONF}, "data": info, } - self.hass.loop.call_soon_threadsafe(self.queue.put_nowait, flow) + self.flow_dispatcher.create(flow) def handle_homekit( From 905281554cfaa8e1ca1775e82663f1fa25a68554 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 17 May 2021 05:54:23 -1000 Subject: [PATCH 03/11] wip --- homeassistant/components/zeroconf/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/zeroconf/__init__.py b/homeassistant/components/zeroconf/__init__.py index a1ea921a2381fc..b63f537b3add4c 100644 --- a/homeassistant/components/zeroconf/__init__.py +++ b/homeassistant/components/zeroconf/__init__.py @@ -295,7 +295,7 @@ def async_start(self) -> None: def create(self, flow: ZeroconfFlow) -> None: """Create and add or queue a flow.""" if self.started: - self.hass.add_job(self._init_flow(flow)) # type: ignore + self.hass.add_job(self._init_flow(flow)) # type: ignore[arg-type] else: self.pending_flows.append(flow) From 7e40951dd5c3d7c0cabf0cefb997e3dfd80e64ef Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 17 May 2021 07:49:32 -1000 Subject: [PATCH 04/11] ensure we browse ZEROCONF_TYPE --- homeassistant/components/zeroconf/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/zeroconf/__init__.py b/homeassistant/components/zeroconf/__init__.py index b63f537b3add4c..05c71eb60f2270 100644 --- a/homeassistant/components/zeroconf/__init__.py +++ b/homeassistant/components/zeroconf/__init__.py @@ -329,7 +329,7 @@ async def async_setup(self) -> None: """Start discovery.""" self.flow_dispatcher = FlowDispatcher(self.hass) types = list(self.zeroconf_types) - for hk_type in HOMEKIT_TYPES: + for hk_type in (ZEROCONF_TYPE, *HOMEKIT_TYPES): if hk_type not in self.zeroconf_types: types.append(hk_type) _LOGGER.debug("Starting Zeroconf browser") From c8aae97f45d52cc0454d4065a2ea3d019484a840 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 17 May 2021 07:50:00 -1000 Subject: [PATCH 05/11] ensure we browse ZEROCONF_TYPE --- homeassistant/components/zeroconf/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/homeassistant/components/zeroconf/__init__.py b/homeassistant/components/zeroconf/__init__.py index 05c71eb60f2270..6179d7f6a3cbf9 100644 --- a/homeassistant/components/zeroconf/__init__.py +++ b/homeassistant/components/zeroconf/__init__.py @@ -329,6 +329,9 @@ async def async_setup(self) -> None: """Start discovery.""" self.flow_dispatcher = FlowDispatcher(self.hass) types = list(self.zeroconf_types) + # We want to make sure we know about other HomeAssistant + # instances as soon as possible to avoid name conflicts + # so we always browse for ZEROCONF_TYPE for hk_type in (ZEROCONF_TYPE, *HOMEKIT_TYPES): if hk_type not in self.zeroconf_types: types.append(hk_type) From 2275d1d8635d7f35520f028a940aaa40691d0ed0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 17 May 2021 08:44:10 -1000 Subject: [PATCH 06/11] Bump zeroconf version --- homeassistant/components/zeroconf/manifest.json | 2 +- homeassistant/package_constraints.txt | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/zeroconf/manifest.json b/homeassistant/components/zeroconf/manifest.json index d76639f3b5b5e3..3abd8824eba446 100644 --- a/homeassistant/components/zeroconf/manifest.json +++ b/homeassistant/components/zeroconf/manifest.json @@ -2,7 +2,7 @@ "domain": "zeroconf", "name": "Zero-configuration networking (zeroconf)", "documentation": "https://www.home-assistant.io/integrations/zeroconf", - "requirements": ["zeroconf==0.30.0","pyroute2==0.5.18"], + "requirements": ["zeroconf==0.31.0","pyroute2==0.5.18"], "dependencies": ["api"], "codeowners": ["@bdraco"], "quality_scale": "internal", diff --git a/homeassistant/package_constraints.txt b/homeassistant/package_constraints.txt index d957f9c7cd9941..4eab183c2c308c 100644 --- a/homeassistant/package_constraints.txt +++ b/homeassistant/package_constraints.txt @@ -34,7 +34,7 @@ sqlalchemy==1.4.13 voluptuous-serialize==2.4.0 voluptuous==0.12.1 yarl==1.6.3 -zeroconf==0.30.0 +zeroconf==0.31.0 pycryptodome>=3.6.6 diff --git a/requirements_all.txt b/requirements_all.txt index 5b84ddc46dfd38..504d7aa4afac33 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -2399,7 +2399,7 @@ zeep[async]==4.0.0 zengge==0.2 # homeassistant.components.zeroconf -zeroconf==0.30.0 +zeroconf==0.31.0 # homeassistant.components.zha zha-quirks==0.0.57 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 15a8c0af2989c7..3697bd2330e466 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -1290,7 +1290,7 @@ yeelight==0.6.2 zeep[async]==4.0.0 # homeassistant.components.zeroconf -zeroconf==0.30.0 +zeroconf==0.31.0 # homeassistant.components.zha zha-quirks==0.0.57 From c8076bdd552a5b69833fe456fa7e599f157df625 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 17 May 2021 09:04:57 -1000 Subject: [PATCH 07/11] typing --- homeassistant/components/zeroconf/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/zeroconf/__init__.py b/homeassistant/components/zeroconf/__init__.py index 6179d7f6a3cbf9..0c61b491876766 100644 --- a/homeassistant/components/zeroconf/__init__.py +++ b/homeassistant/components/zeroconf/__init__.py @@ -2,14 +2,14 @@ from __future__ import annotations import asyncio -from collections.abc import Iterable +from collections.abc import Coroutine, Iterable from contextlib import suppress import fnmatch import ipaddress from ipaddress import ip_address import logging import socket -from typing import Any, Coroutine, TypedDict, cast +from typing import Any, TypedDict, cast from pyroute2 import IPRoute import voluptuous as vol @@ -295,7 +295,7 @@ def async_start(self) -> None: def create(self, flow: ZeroconfFlow) -> None: """Create and add or queue a flow.""" if self.started: - self.hass.add_job(self._init_flow(flow)) # type: ignore[arg-type] + self.hass.create_task(self._init_flow(flow)) else: self.pending_flows.append(flow) From 870a97ac1a2384c9bff28a847fa574cccf7d1bca Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 17 May 2021 12:00:22 -1000 Subject: [PATCH 08/11] update tests --- tests/components/zeroconf/test_init.py | 150 ++++++++++++------------- 1 file changed, 73 insertions(+), 77 deletions(-) diff --git a/tests/components/zeroconf/test_init.py b/tests/components/zeroconf/test_init.py index 6af1e554f6cea4..d8a9a94da96f50 100644 --- a/tests/components/zeroconf/test_init.py +++ b/tests/components/zeroconf/test_init.py @@ -1,14 +1,7 @@ """Test Zeroconf component setup process.""" from unittest.mock import patch -from zeroconf import ( - BadTypeInNameException, - Error as ZeroconfError, - InterfaceChoice, - IPVersion, - ServiceInfo, - ServiceStateChange, -) +from zeroconf import InterfaceChoice, IPVersion, ServiceInfo, ServiceStateChange from homeassistant.components import zeroconf from homeassistant.components.zeroconf import CONF_DEFAULT_INTERFACE, CONF_IPV6 @@ -184,8 +177,7 @@ async def test_setup_with_overly_long_url_and_name(hass, mock_zeroconf, caplog): "location_name", "\u00dcBER \u00dcber German Umlaut long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string long string", ), patch( - "homeassistant.components.zeroconf.ServiceInfo", - side_effect=get_service_info_mock, + "homeassistant.components.zeroconf.ServiceInfo.request", ): assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_START) @@ -276,22 +268,6 @@ async def test_setup_with_ipv6_default(hass, mock_zeroconf): assert mock_zeroconf.called_with() -async def test_service_with_invalid_name(hass, mock_zeroconf, caplog): - """Test we do not crash on service with an invalid name.""" - with patch.object( - zeroconf, "HaServiceBrowser", side_effect=service_update_mock - ) as mock_service_browser, patch( - "homeassistant.components.zeroconf.ServiceInfo", - side_effect=BadTypeInNameException, - ): - 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 "Failed to get info for device" in caplog.text - - async def test_zeroconf_match_macaddress(hass, mock_zeroconf): """Test configured options for a device are loaded via config entry.""" @@ -318,7 +294,7 @@ def http_only_service_update_mock(zeroconf, services, handlers): zeroconf, "HaServiceBrowser", side_effect=http_only_service_update_mock ) as mock_service_browser, patch( "homeassistant.components.zeroconf.ServiceInfo", - return_value=get_service_info_mock("FFAADDCC11DD"), + side_effect=get_zeroconf_info_mock("FFAADDCC11DD"), ): assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) @@ -349,10 +325,10 @@ def http_only_service_update_mock(zeroconf, services, handlers): hass.config_entries.flow, "async_init" ) as mock_config_flow, patch.object( zeroconf, "HaServiceBrowser", side_effect=http_only_service_update_mock - ) as mock_service_browser: - mock_zeroconf.get_service_info.side_effect = ( - get_zeroconf_info_mock_manufacturer("Samsung Electronics") - ) + ) as mock_service_browser, patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_zeroconf_info_mock_manufacturer("Samsung Electronics"), + ): assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -382,10 +358,10 @@ def http_only_service_update_mock(zeroconf, services, handlers): hass.config_entries.flow, "async_init" ) as mock_config_flow, patch.object( zeroconf, "HaServiceBrowser", side_effect=http_only_service_update_mock - ) as mock_service_browser: - mock_zeroconf.get_service_info.side_effect = get_zeroconf_info_mock( - "aa:bb:cc:dd:ee:ff" - ) + ) as mock_service_browser, patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_zeroconf_info_mock("aabbccddeeff"), + ): assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -414,10 +390,10 @@ def http_only_service_update_mock(zeroconf, services, handlers): hass.config_entries.flow, "async_init" ) as mock_config_flow, patch.object( zeroconf, "HaServiceBrowser", side_effect=http_only_service_update_mock - ) as mock_service_browser: - mock_zeroconf.get_service_info.side_effect = get_zeroconf_info_mock( - "FFAADDCC11DD" - ) + ) as mock_service_browser, patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_zeroconf_info_mock("FFAADDCC11DD"), + ): assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -446,10 +422,10 @@ def http_only_service_update_mock(zeroconf, services, handlers): hass.config_entries.flow, "async_init" ) as mock_config_flow, patch.object( zeroconf, "HaServiceBrowser", side_effect=http_only_service_update_mock - ) as mock_service_browser: - mock_zeroconf.get_service_info.side_effect = ( - get_zeroconf_info_mock_manufacturer("Not Samsung Electronics") - ) + ) as mock_service_browser, patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_zeroconf_info_mock_manufacturer("Not Samsung Electronics"), + ): assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -472,10 +448,10 @@ async def test_homekit_match_partial_space(hass, mock_zeroconf): side_effect=lambda *args, **kwargs: service_update_mock( *args, **kwargs, limit_service="_hap._tcp.local." ), - ) as mock_service_browser: - mock_zeroconf.get_service_info.side_effect = get_homekit_info_mock( - "LIFX bulb", HOMEKIT_STATUS_UNPAIRED - ) + ) as mock_service_browser, patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_homekit_info_mock("LIFX bulb", 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() @@ -499,10 +475,10 @@ async def test_homekit_match_partial_dash(hass, mock_zeroconf): side_effect=lambda *args, **kwargs: service_update_mock( *args, **kwargs, limit_service="_hap._udp.local." ), - ) as mock_service_browser: - mock_zeroconf.get_service_info.side_effect = get_homekit_info_mock( - "Rachio-fa46ba", HOMEKIT_STATUS_UNPAIRED - ) + ) as mock_service_browser, patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_homekit_info_mock("Rachio-fa46ba", 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() @@ -526,10 +502,10 @@ async def test_homekit_match_full(hass, mock_zeroconf): side_effect=lambda *args, **kwargs: service_update_mock( *args, **kwargs, limit_service="_hap._udp.local." ), - ) as mock_service_browser: - mock_zeroconf.get_service_info.side_effect = get_homekit_info_mock( - "BSB002", HOMEKIT_STATUS_UNPAIRED - ) + ) as mock_service_browser, patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_homekit_info_mock("BSB002", 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() @@ -581,10 +557,10 @@ async def test_homekit_invalid_paring_status(hass, mock_zeroconf): side_effect=lambda *args, **kwargs: service_update_mock( *args, **kwargs, limit_service="_hap._tcp.local." ), - ) as mock_service_browser: - mock_zeroconf.get_service_info.side_effect = get_homekit_info_mock( - "tado", b"invalid" - ) + ) as mock_service_browser, patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_homekit_info_mock("tado", b"invalid"), + ): assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -604,10 +580,12 @@ async def test_homekit_not_paired(hass, mock_zeroconf): hass.config_entries.flow, "async_init" ) as mock_config_flow, patch.object( zeroconf, "HaServiceBrowser", side_effect=service_update_mock - ) as mock_service_browser: - mock_zeroconf.get_service_info.side_effect = get_homekit_info_mock( + ) as mock_service_browser, patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_homekit_info_mock( "this_will_not_match_any_integration", 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() @@ -652,34 +630,44 @@ async def test_get_instance(hass, mock_zeroconf): async def test_removed_ignored(hass, mock_zeroconf): """Test we remove it when a zeroconf entry is removed.""" - mock_zeroconf.get_service_info.side_effect = ZeroconfError def service_update_mock(zeroconf, services, handlers): """Call service update handler.""" handlers[0]( - zeroconf, "_service.added", "name._service.added", ServiceStateChange.Added + zeroconf, + "_service.added.local.", + "name._service.added.local.", + ServiceStateChange.Added, ) handlers[0]( zeroconf, - "_service.updated", - "name._service.updated", + "_service.updated.local.", + "name._service.updated.local.", ServiceStateChange.Updated, ) handlers[0]( zeroconf, - "_service.removed", - "name._service.removed", + "_service.removed.local.", + "name._service.removed.local.", ServiceStateChange.Removed, ) - with patch.object(zeroconf, "HaServiceBrowser", side_effect=service_update_mock): + with patch.object( + zeroconf, "HaServiceBrowser", side_effect=service_update_mock + ), patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_service_info_mock, + ) as mock_service_info: 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_zeroconf.get_service_info.mock_calls) == 2 - assert mock_zeroconf.get_service_info.mock_calls[0][1][0] == "_service.added" - assert mock_zeroconf.get_service_info.mock_calls[1][1][0] == "_service.updated" + assert len(mock_service_info.mock_calls) == 2 + import pprint + + pprint.pprint(mock_service_info.mock_calls[0][1]) + assert mock_service_info.mock_calls[0][1][0] == "_service.added.local." + assert mock_service_info.mock_calls[1][1][0] == "_service.updated.local." async def test_async_detect_interfaces_setting_non_loopback_route(hass, mock_zeroconf): @@ -689,8 +677,10 @@ async def test_async_detect_interfaces_setting_non_loopback_route(hass, mock_zer ), patch( "homeassistant.components.zeroconf.IPRoute.route", return_value=_ROUTE_NO_LOOPBACK, + ), patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_service_info_mock, ): - mock_zeroconf.get_service_info.side_effect = get_service_info_mock assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -704,8 +694,10 @@ async def test_async_detect_interfaces_setting_loopback_route(hass, mock_zerocon zeroconf, "HaServiceBrowser", side_effect=service_update_mock ), patch( "homeassistant.components.zeroconf.IPRoute.route", return_value=_ROUTE_LOOPBACK + ), patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_service_info_mock, ): - mock_zeroconf.get_service_info.side_effect = get_service_info_mock assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -717,8 +709,10 @@ async def test_async_detect_interfaces_setting_empty_route(hass, mock_zeroconf): """Test without default interface config and the route returns nothing.""" with patch.object(hass.config_entries.flow, "async_init"), patch.object( zeroconf, "HaServiceBrowser", side_effect=service_update_mock - ), patch("homeassistant.components.zeroconf.IPRoute.route", return_value=[]): - mock_zeroconf.get_service_info.side_effect = get_service_info_mock + ), patch("homeassistant.components.zeroconf.IPRoute.route", return_value=[]), patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_service_info_mock, + ): assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -732,8 +726,10 @@ async def test_async_detect_interfaces_setting_exception(hass, mock_zeroconf): zeroconf, "HaServiceBrowser", side_effect=service_update_mock ), patch( "homeassistant.components.zeroconf.IPRoute.route", side_effect=AttributeError + ), patch( + "homeassistant.components.zeroconf.ServiceInfo", + side_effect=get_service_info_mock, ): - mock_zeroconf.get_service_info.side_effect = get_service_info_mock assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}}) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() From 0e30e87b7f93c0b406c9a2d73d899a6e1d82e104 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 17 May 2021 18:14:59 -0400 Subject: [PATCH 09/11] Update homeassistant/components/zeroconf/__init__.py Co-authored-by: Ruslan Sayfutdinov --- homeassistant/components/zeroconf/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/zeroconf/__init__.py b/homeassistant/components/zeroconf/__init__.py index 0c61b491876766..704b58dff8b947 100644 --- a/homeassistant/components/zeroconf/__init__.py +++ b/homeassistant/components/zeroconf/__init__.py @@ -299,7 +299,7 @@ def create(self, flow: ZeroconfFlow) -> None: else: self.pending_flows.append(flow) - def _init_flow(self, flow: ZeroconfFlow) -> Coroutine[Any, Any, Any]: + def _init_flow(self, flow: ZeroconfFlow) -> Coroutine[None, None, FlowResult]: """Create a flow.""" return self.hass.config_entries.flow.async_init( flow["domain"], context=flow["context"], data=flow["data"] From aa1fe7b90d75897f2e620191052dcf06e8076c05 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 17 May 2021 12:17:17 -1000 Subject: [PATCH 10/11] add import --- homeassistant/components/zeroconf/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/homeassistant/components/zeroconf/__init__.py b/homeassistant/components/zeroconf/__init__.py index 704b58dff8b947..cc7710168bf4bf 100644 --- a/homeassistant/components/zeroconf/__init__.py +++ b/homeassistant/components/zeroconf/__init__.py @@ -30,6 +30,7 @@ __version__, ) from homeassistant.core import Event, HomeAssistant, callback +from homeassistant.data_entry_flow import FlowResult import homeassistant.helpers.config_validation as cv from homeassistant.helpers.network import NoURLAvailableError, get_url from homeassistant.loader import async_get_homekit, async_get_zeroconf, bind_hass From efffb0d81b4cc14446004907c38339e2878d5f7f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 17 May 2021 13:02:47 -1000 Subject: [PATCH 11/11] Race fix --- homeassistant/components/zeroconf/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/zeroconf/__init__.py b/homeassistant/components/zeroconf/__init__.py index cc7710168bf4bf..ceaba3f02a12d1 100644 --- a/homeassistant/components/zeroconf/__init__.py +++ b/homeassistant/components/zeroconf/__init__.py @@ -289,7 +289,10 @@ def __init__(self, hass: HomeAssistant): def async_start(self) -> None: """Start processing pending flows.""" self.started = True - for flow in list(self.pending_flows): + self.hass.loop.call_soon(self._async_process_pending_flows) + + def _async_process_pending_flows(self) -> None: + for flow in self.pending_flows: self.hass.async_create_task(self._init_flow(flow)) self.pending_flows = []