From 682501eb47868deaa5cbeb5053d11171e7a3f6bb Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Wed, 21 Dec 2022 22:35:52 +0100 Subject: [PATCH] Less tasks when receiving SSDP messages (#84186) Co-authored-by: J. Nick Koston --- .../components/dlna_dmr/manifest.json | 2 +- .../components/dlna_dms/manifest.json | 2 +- .../components/samsungtv/manifest.json | 2 +- homeassistant/components/ssdp/__init__.py | 69 +++++++++++++++---- homeassistant/components/ssdp/manifest.json | 2 +- homeassistant/components/upnp/manifest.json | 2 +- .../components/yeelight/manifest.json | 2 +- homeassistant/components/yeelight/scanner.py | 14 ++-- homeassistant/package_constraints.txt | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/ssdp/test_init.py | 55 +++++++-------- tests/components/yeelight/__init__.py | 5 +- 13 files changed, 103 insertions(+), 58 deletions(-) diff --git a/homeassistant/components/dlna_dmr/manifest.json b/homeassistant/components/dlna_dmr/manifest.json index 0edbc9a9a12..acb2cfd4405 100644 --- a/homeassistant/components/dlna_dmr/manifest.json +++ b/homeassistant/components/dlna_dmr/manifest.json @@ -3,7 +3,7 @@ "name": "DLNA Digital Media Renderer", "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/dlna_dmr", - "requirements": ["async-upnp-client==0.32.3", "getmac==0.8.2"], + "requirements": ["async-upnp-client==0.33.0", "getmac==0.8.2"], "dependencies": ["ssdp"], "after_dependencies": ["media_source"], "ssdp": [ diff --git a/homeassistant/components/dlna_dms/manifest.json b/homeassistant/components/dlna_dms/manifest.json index 3630e5ce309..f141b2c1519 100644 --- a/homeassistant/components/dlna_dms/manifest.json +++ b/homeassistant/components/dlna_dms/manifest.json @@ -3,7 +3,7 @@ "name": "DLNA Digital Media Server", "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/dlna_dms", - "requirements": ["async-upnp-client==0.32.3"], + "requirements": ["async-upnp-client==0.33.0"], "dependencies": ["ssdp"], "after_dependencies": ["media_source"], "ssdp": [ diff --git a/homeassistant/components/samsungtv/manifest.json b/homeassistant/components/samsungtv/manifest.json index 8a922fbbaf0..e1c326ba364 100644 --- a/homeassistant/components/samsungtv/manifest.json +++ b/homeassistant/components/samsungtv/manifest.json @@ -8,7 +8,7 @@ "samsungctl[websocket]==0.7.1", "samsungtvws[async,encrypted]==2.5.0", "wakeonlan==2.1.0", - "async-upnp-client==0.32.3" + "async-upnp-client==0.33.0" ], "ssdp": [ { diff --git a/homeassistant/components/ssdp/__init__.py b/homeassistant/components/ssdp/__init__.py index 8e037602b90..77065818369 100644 --- a/homeassistant/components/ssdp/__init__.py +++ b/homeassistant/components/ssdp/__init__.py @@ -424,7 +424,7 @@ class Scanner: source = fix_ipv6_address_scope_id(source) or source self._ssdp_listeners.append( SsdpListener( - async_callback=self._ssdp_listener_callback, + callback=self._ssdp_listener_callback, source=source, target=target, device_tracker=device_tracker, @@ -458,7 +458,7 @@ class Scanner: if _async_headers_match(combined_headers, lower_match_dict) ] - async def _ssdp_listener_callback( + def _ssdp_listener_callback( self, ssdp_device: SsdpDevice, dst: DeviceOrServiceType, @@ -469,15 +469,49 @@ class Scanner: "SSDP: ssdp_device: %s, dst: %s, source: %s", ssdp_device, dst, source ) + assert self._description_cache + location = ssdp_device.location - info_desc = None + _, info_desc = self._description_cache.peek_description_dict(location) + if info_desc is None: + # Fetch info desc in separate task and process from there. + self.hass.async_create_task( + self._ssdp_listener_process_with_lookup(ssdp_device, dst, source) + ) + return + + # Info desc known, process directly. + self._ssdp_listener_process(ssdp_device, dst, source, info_desc) + + async def _ssdp_listener_process_with_lookup( + self, + ssdp_device: SsdpDevice, + dst: DeviceOrServiceType, + source: SsdpSource, + ) -> None: + """Handle a device/service change.""" + location = ssdp_device.location + self._ssdp_listener_process( + ssdp_device, + dst, + source, + await self._async_get_description_dict(location), + ) + + def _ssdp_listener_process( + self, + ssdp_device: SsdpDevice, + dst: DeviceOrServiceType, + source: SsdpSource, + info_desc: Mapping[str, Any], + ) -> None: + """Handle a device/service change.""" + matching_domains: set[str] = set() combined_headers = ssdp_device.combined_headers(dst) callbacks = self._async_get_matching_callbacks(combined_headers) - matching_domains: set[str] = set() # If there are no changes from a search, do not trigger a config flow if source != SsdpSource.SEARCH_ALIVE: - info_desc = await self._async_get_description_dict(location) or {} matching_domains = self.integration_matchers.async_matching_domains( CaseInsensitiveDict(combined_headers.as_dict(), **info_desc) ) @@ -485,21 +519,24 @@ class Scanner: if not callbacks and not matching_domains: return - if info_desc is None: - info_desc = await self._async_get_description_dict(location) or {} discovery_info = discovery_info_from_headers_and_description( combined_headers, info_desc ) discovery_info.x_homeassistant_matching_domains = matching_domains - ssdp_change = SSDP_SOURCE_SSDP_CHANGE_MAPPING[source] - await _async_process_callbacks(callbacks, discovery_info, ssdp_change) + + if callbacks: + ssdp_change = SSDP_SOURCE_SSDP_CHANGE_MAPPING[source] + self.hass.async_create_task( + _async_process_callbacks(callbacks, discovery_info, ssdp_change) + ) # Config flows should only be created for alive/update messages from alive devices - if ssdp_change == SsdpChange.BYEBYE: + if source == SsdpSource.ADVERTISEMENT_BYEBYE: return _LOGGER.debug("Discovery info: %s", discovery_info) + location = ssdp_device.location for domain in matching_domains: _LOGGER.debug("Discovered %s at %s", domain, location) discovery_flow.async_create_flow( @@ -514,6 +551,13 @@ class Scanner: ) -> Mapping[str, str]: """Get description dict.""" assert self._description_cache is not None + + has_description, description = self._description_cache.peek_description_dict( + location + ) + if has_description: + return description or {} + return await self._description_cache.async_get_description_dict(location) or {} async def _async_headers_to_discovery_info( @@ -524,10 +568,9 @@ class Scanner: Building this is a bit expensive so we only do it on demand. """ assert self._description_cache is not None + location = headers["location"] - info_desc = ( - await self._description_cache.async_get_description_dict(location) or {} - ) + info_desc = await self._async_get_description_dict(location) return discovery_info_from_headers_and_description(headers, info_desc) async def async_get_discovery_info_by_udn_st( # pylint: disable=invalid-name diff --git a/homeassistant/components/ssdp/manifest.json b/homeassistant/components/ssdp/manifest.json index d532dc8f292..10f7a8e49a2 100644 --- a/homeassistant/components/ssdp/manifest.json +++ b/homeassistant/components/ssdp/manifest.json @@ -2,7 +2,7 @@ "domain": "ssdp", "name": "Simple Service Discovery Protocol (SSDP)", "documentation": "https://www.home-assistant.io/integrations/ssdp", - "requirements": ["async-upnp-client==0.32.3"], + "requirements": ["async-upnp-client==0.33.0"], "dependencies": ["network"], "after_dependencies": ["zeroconf"], "codeowners": [], diff --git a/homeassistant/components/upnp/manifest.json b/homeassistant/components/upnp/manifest.json index 214f5ce2931..ff683c883bb 100644 --- a/homeassistant/components/upnp/manifest.json +++ b/homeassistant/components/upnp/manifest.json @@ -3,7 +3,7 @@ "name": "UPnP/IGD", "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/upnp", - "requirements": ["async-upnp-client==0.32.3", "getmac==0.8.2"], + "requirements": ["async-upnp-client==0.33.0", "getmac==0.8.2"], "dependencies": ["network", "ssdp"], "codeowners": ["@StevenLooman"], "ssdp": [ diff --git a/homeassistant/components/yeelight/manifest.json b/homeassistant/components/yeelight/manifest.json index f3d9d8b24ca..c72a9401c52 100644 --- a/homeassistant/components/yeelight/manifest.json +++ b/homeassistant/components/yeelight/manifest.json @@ -2,7 +2,7 @@ "domain": "yeelight", "name": "Yeelight", "documentation": "https://www.home-assistant.io/integrations/yeelight", - "requirements": ["yeelight==0.7.10", "async-upnp-client==0.32.3"], + "requirements": ["yeelight==0.7.10", "async-upnp-client==0.33.0"], "codeowners": ["@zewelor", "@shenxn", "@starkillerOG", "@alexyao2015"], "config_flow": true, "dependencies": ["network"], diff --git a/homeassistant/components/yeelight/scanner.py b/homeassistant/components/yeelight/scanner.py index 7a0d409b434..612edc29791 100644 --- a/homeassistant/components/yeelight/scanner.py +++ b/homeassistant/components/yeelight/scanner.py @@ -2,7 +2,7 @@ from __future__ import annotations import asyncio -from collections.abc import Awaitable, Callable, ValuesView +from collections.abc import Callable, ValuesView import contextlib from datetime import datetime from ipaddress import IPv4Address @@ -64,10 +64,11 @@ class YeelightScanner: for idx, source_ip in enumerate(await self._async_build_source_set()): self._connected_events.append(asyncio.Event()) - def _wrap_async_connected_idx(idx) -> Callable[[], Awaitable[None]]: + def _wrap_async_connected_idx(idx) -> Callable[[], None]: """Create a function to capture the idx cell variable.""" - async def _async_connected() -> None: + @callback + def _async_connected() -> None: self._connected_events[idx].set() return _async_connected @@ -75,11 +76,11 @@ class YeelightScanner: source = (str(source_ip), 0) self._listeners.append( SsdpSearchListener( - async_callback=self._async_process_entry, + callback=self._async_process_entry, search_target=SSDP_ST, target=SSDP_TARGET, source=source, - async_connect_callback=_wrap_async_connected_idx(idx), + connect_callback=_wrap_async_connected_idx(idx), ) ) @@ -180,7 +181,8 @@ class YeelightScanner: # of another discovery async_call_later(self._hass, 1, _async_start_flow) - async def _async_process_entry(self, headers: CaseInsensitiveDict) -> None: + @callback + def _async_process_entry(self, headers: CaseInsensitiveDict) -> None: """Process a discovery.""" _LOGGER.debug("Discovered via SSDP: %s", headers) unique_id = headers["id"] diff --git a/homeassistant/package_constraints.txt b/homeassistant/package_constraints.txt index 2030c0abbef..ffd863988c7 100644 --- a/homeassistant/package_constraints.txt +++ b/homeassistant/package_constraints.txt @@ -4,7 +4,7 @@ aiodiscover==1.4.13 aiohttp==3.8.1 aiohttp_cors==0.7.0 astral==2.2 -async-upnp-client==0.32.3 +async-upnp-client==0.33.0 async_timeout==4.0.2 atomicwrites-homeassistant==1.4.1 attrs==22.1.0 diff --git a/requirements_all.txt b/requirements_all.txt index aa4a77c72bf..b07cd5c353e 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -368,7 +368,7 @@ asterisk_mbox==0.5.0 # homeassistant.components.ssdp # homeassistant.components.upnp # homeassistant.components.yeelight -async-upnp-client==0.32.3 +async-upnp-client==0.33.0 # homeassistant.components.supla asyncpysupla==0.0.5 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 7d04cef4976..d03ecc9ea51 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -322,7 +322,7 @@ arcam-fmj==1.0.1 # homeassistant.components.ssdp # homeassistant.components.upnp # homeassistant.components.yeelight -async-upnp-client==0.32.3 +async-upnp-client==0.33.0 # homeassistant.components.sleepiq asyncsleepiq==1.2.3 diff --git a/tests/components/ssdp/test_init.py b/tests/components/ssdp/test_init.py index 4076ef4685d..959a35dba78 100644 --- a/tests/components/ssdp/test_init.py +++ b/tests/components/ssdp/test_init.py @@ -11,7 +11,6 @@ from async_upnp_client.ssdp_listener import SsdpListener from async_upnp_client.utils import CaseInsensitiveDict import pytest -import homeassistant from homeassistant import config_entries from homeassistant.components import ssdp from homeassistant.const import ( @@ -19,6 +18,7 @@ from homeassistant.const import ( EVENT_HOMEASSISTANT_STOP, MATCH_ALL, ) +from homeassistant.core import HomeAssistant from homeassistant.setup import async_setup_component import homeassistant.util.dt as dt_util @@ -31,7 +31,7 @@ def _ssdp_headers(headers): return ssdp_headers -async def init_ssdp_component(hass: homeassistant) -> SsdpListener: +async def init_ssdp_component(hass: HomeAssistant) -> SsdpListener: """Initialize ssdp component and get SsdpListener.""" await async_setup_component(hass, ssdp.DOMAIN, {ssdp.DOMAIN: {}}) await hass.async_block_till_done() @@ -55,7 +55,7 @@ async def test_ssdp_flow_dispatched_on_st(mock_get_ssdp, hass, caplog, mock_flow } ) ssdp_listener = await init_ssdp_component(hass) - await ssdp_listener._on_search(mock_ssdp_search_response) + ssdp_listener._on_search(mock_ssdp_search_response) await hass.async_block_till_done() hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -98,7 +98,7 @@ async def test_ssdp_flow_dispatched_on_manufacturer_url( } ) ssdp_listener = await init_ssdp_component(hass) - await ssdp_listener._on_search(mock_ssdp_search_response) + ssdp_listener._on_search(mock_ssdp_search_response) await hass.async_block_till_done() hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -148,7 +148,7 @@ async def test_scan_match_upnp_devicedesc_manufacturer( } ) ssdp_listener = await init_ssdp_component(hass) - await ssdp_listener._on_search(mock_ssdp_search_response) + ssdp_listener._on_search(mock_ssdp_search_response) await hass.async_block_till_done() hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -189,7 +189,7 @@ async def test_scan_match_upnp_devicedesc_devicetype( } ) ssdp_listener = await init_ssdp_component(hass) - await ssdp_listener._on_search(mock_ssdp_search_response) + ssdp_listener._on_search(mock_ssdp_search_response) await hass.async_block_till_done() hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) @@ -237,7 +237,7 @@ async def test_scan_not_all_present( } ) ssdp_listener = await init_ssdp_component(hass) - await ssdp_listener._on_search(mock_ssdp_search_response) + ssdp_listener._on_search(mock_ssdp_search_response) await hass.async_block_till_done() hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -278,7 +278,7 @@ async def test_scan_not_all_match(mock_get_ssdp, hass, aioclient_mock, mock_flow } ) ssdp_listener = await init_ssdp_component(hass) - await ssdp_listener._on_search(mock_ssdp_search_response) + ssdp_listener._on_search(mock_ssdp_search_response) await hass.async_block_till_done() hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) await hass.async_block_till_done() @@ -317,7 +317,7 @@ async def test_flow_start_only_alive( "usn": "uuid:mock-udn::mock-st", } ) - await ssdp_listener._on_search(mock_ssdp_search_response) + ssdp_listener._on_search(mock_ssdp_search_response) await hass.async_block_till_done() mock_flow_init.assert_awaited_once_with( @@ -334,7 +334,7 @@ async def test_flow_start_only_alive( "nts": "ssdp:alive", } ) - await ssdp_listener._on_alive(mock_ssdp_advertisement) + ssdp_listener._on_alive(mock_ssdp_advertisement) await hass.async_block_till_done() mock_flow_init.assert_awaited_once_with( "mock-domain", context={"source": config_entries.SOURCE_SSDP}, data=ANY @@ -343,14 +343,14 @@ async def test_flow_start_only_alive( # ssdp:byebye advertisement should not start a flow mock_flow_init.reset_mock() mock_ssdp_advertisement["nts"] = "ssdp:byebye" - await ssdp_listener._on_byebye(mock_ssdp_advertisement) + ssdp_listener._on_byebye(mock_ssdp_advertisement) await hass.async_block_till_done() mock_flow_init.assert_not_called() # ssdp:update advertisement should start a flow mock_flow_init.reset_mock() mock_ssdp_advertisement["nts"] = "ssdp:update" - await ssdp_listener._on_update(mock_ssdp_advertisement) + ssdp_listener._on_update(mock_ssdp_advertisement) await hass.async_block_till_done() mock_flow_init.assert_awaited_once_with( "mock-domain", context={"source": config_entries.SOURCE_SSDP}, data=ANY @@ -388,7 +388,7 @@ async def test_discovery_from_advertisement_sets_ssdp_st( "usn": "uuid:mock-udn::mock-st", } ) - await ssdp_listener._on_alive(mock_ssdp_advertisement) + ssdp_listener._on_alive(mock_ssdp_advertisement) await hass.async_block_till_done() discovery_info = await ssdp.async_get_discovery_info_by_udn(hass, "uuid:mock-udn") @@ -469,34 +469,35 @@ async def test_scan_with_registered_callback( hass, async_integration_callback, {"st": "mock-st"} ) - async_integration_match_all_callback1 = AsyncMock() + async_integration_match_all_callback = AsyncMock() await ssdp.async_register_callback( - hass, async_integration_match_all_callback1, {"x-rincon-bootseq": MATCH_ALL} + hass, async_integration_match_all_callback, {"x-rincon-bootseq": MATCH_ALL} ) - async_integration_match_all_not_present_callback1 = AsyncMock() + async_integration_match_all_not_present_callback = AsyncMock() await ssdp.async_register_callback( hass, - async_integration_match_all_not_present_callback1, + async_integration_match_all_not_present_callback, {"x-not-there": MATCH_ALL}, ) - async_not_matching_integration_callback1 = AsyncMock() + async_not_matching_integration_callback = AsyncMock() await ssdp.async_register_callback( - hass, async_not_matching_integration_callback1, {"st": "not-match-mock-st"} + hass, async_not_matching_integration_callback, {"st": "not-match-mock-st"} ) - async_match_any_callback1 = AsyncMock() - await ssdp.async_register_callback(hass, async_match_any_callback1) + async_match_any_callback = AsyncMock() + await ssdp.async_register_callback(hass, async_match_any_callback) await hass.async_block_till_done() - await ssdp_listener._on_search(mock_ssdp_search_response) + ssdp_listener._on_search(mock_ssdp_search_response) + await hass.async_block_till_done() assert async_integration_callback.call_count == 1 - assert async_integration_match_all_callback1.call_count == 1 - assert async_integration_match_all_not_present_callback1.call_count == 0 - assert async_match_any_callback1.call_count == 1 - assert async_not_matching_integration_callback1.call_count == 0 + assert async_integration_match_all_callback.call_count == 1 + assert async_integration_match_all_not_present_callback.call_count == 0 + assert async_match_any_callback.call_count == 1 + assert async_not_matching_integration_callback.call_count == 0 assert async_integration_callback.call_args[0][1] == ssdp.SsdpChange.ALIVE mock_call_data: ssdp.SsdpServiceInfo = async_integration_callback.call_args[0][0] assert mock_call_data.ssdp_ext == "" @@ -552,7 +553,7 @@ async def test_getting_existing_headers( } ) ssdp_listener = await init_ssdp_component(hass) - await ssdp_listener._on_search(mock_ssdp_search_response) + ssdp_listener._on_search(mock_ssdp_search_response) discovery_info_by_st = await ssdp.async_get_discovery_info_by_st(hass, "mock-st") discovery_info_by_st = discovery_info_by_st[0] diff --git a/tests/components/yeelight/__init__.py b/tests/components/yeelight/__init__.py index ea3c8f4656c..64f6bcdadde 100644 --- a/tests/components/yeelight/__init__.py +++ b/tests/components/yeelight/__init__.py @@ -1,5 +1,4 @@ """Tests for the Yeelight integration.""" -import asyncio from datetime import timedelta from unittest.mock import AsyncMock, MagicMock, patch @@ -164,12 +163,12 @@ def _patched_ssdp_listener(info: CaseInsensitiveDict, *args, **kwargs): async def _async_callback(*_): if kwargs["source"][0] == FAIL_TO_BIND_IP: raise OSError - await listener.async_connect_callback() + listener.connect_callback() @callback def _async_search(*_): if info: - asyncio.create_task(listener.async_callback(info)) + listener.callback(info) listener.async_start = _async_callback listener.async_search = _async_search