Populate upnp devices from ssdp (#51221)

* Populate upnp devices from ssdp

* Update tests since data comes in via HASS format now

* pylint

homeassistant/components/upnp/__init__.py

@@ -5,6 +5,7 @@ from ipaddress import ip_address
 import voluptuous as vol
 
 from homeassistant import config_entries
+from homeassistant.components import ssdp
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import ConfigEntryNotReady
@@ -17,9 +18,6 @@ from .const import (
     CONFIG_ENTRY_HOSTNAME,
     CONFIG_ENTRY_ST,
     CONFIG_ENTRY_UDN,
-    DISCOVERY_LOCATION,
-    DISCOVERY_ST,
-    DISCOVERY_UDN,
     DOMAIN,
     DOMAIN_CONFIG,
     DOMAIN_DEVICES,
@@ -49,24 +47,15 @@ async def async_construct_device(hass: HomeAssistant, udn: str, st: str) -> Devi
     """Discovery devices and construct a Device for one."""
     # pylint: disable=invalid-name
     _LOGGER.debug("Constructing device: %s::%s", udn, st)
+    discovery_info = ssdp.async_get_discovery_info_by_udn_st(hass, udn, st)
 
-    discoveries = [
-        discovery
-        for discovery in await Device.async_discover(hass)
-        if discovery[DISCOVERY_UDN] == udn and discovery[DISCOVERY_ST] == st
-    ]
-    if not discoveries:
+    if not discovery_info:
         _LOGGER.info("Device not discovered")
         return None
 
-    # Some additional clues for remote debugging.
-    if len(discoveries) > 1:
-        _LOGGER.info("Multiple devices discovered: %s", discoveries)
-
-    discovery = discoveries[0]
-    _LOGGER.debug("Constructing from discovery: %s", discovery)
-    location = discovery[DISCOVERY_LOCATION]
-    return await Device.async_create_device(hass, location)
+    return await Device.async_create_device(
+        hass, discovery_info[ssdp.ATTR_SSDP_LOCATION]
+    )
 
 
 async def async_setup(hass: HomeAssistant, config: ConfigType):

homeassistant/components/upnp/config_flow.py

@@ -29,17 +29,7 @@ from .const import (
     DOMAIN_DEVICES,
     LOGGER as _LOGGER,
 )
-from .device import Device
-
-
-def discovery_info_to_discovery(discovery_info: Mapping) -> Mapping:
-    """Convert a SSDP-discovery to 'our' discovery."""
-    return {
-        DISCOVERY_UDN: discovery_info[ssdp.ATTR_UPNP_UDN],
-        DISCOVERY_ST: discovery_info[ssdp.ATTR_SSDP_ST],
-        DISCOVERY_LOCATION: discovery_info[ssdp.ATTR_SSDP_LOCATION],
-        DISCOVERY_USN: discovery_info[ssdp.ATTR_SSDP_USN],
-    }
+from .device import Device, discovery_info_to_discovery
 
 
 class UpnpFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):

homeassistant/components/upnp/device.py

@@ -12,6 +12,7 @@ from async_upnp_client.aiohttp import AiohttpSessionRequester
 from async_upnp_client.device_updater import DeviceUpdater
 from async_upnp_client.profiles.igd import IgdDevice
 
+from homeassistant.components import ssdp
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
 from homeassistant.helpers.update_coordinator import DataUpdateCoordinator
@@ -37,6 +38,16 @@ from .const import (
 )
 
 
+def discovery_info_to_discovery(discovery_info: Mapping) -> Mapping:
+    """Convert a SSDP-discovery to 'our' discovery."""
+    return {
+        DISCOVERY_UDN: discovery_info[ssdp.ATTR_UPNP_UDN],
+        DISCOVERY_ST: discovery_info[ssdp.ATTR_SSDP_ST],
+        DISCOVERY_LOCATION: discovery_info[ssdp.ATTR_SSDP_LOCATION],
+        DISCOVERY_USN: discovery_info[ssdp.ATTR_SSDP_USN],
+    }
+
+
 def _get_local_ip(hass: HomeAssistant) -> IPv4Address | None:
     """Get the configured local ip."""
     if DOMAIN in hass.data and DOMAIN_CONFIG in hass.data[DOMAIN]:
@@ -59,17 +70,10 @@ class Device:
     async def async_discover(cls, hass: HomeAssistant) -> list[Mapping]:
         """Discover UPnP/IGD devices."""
         _LOGGER.debug("Discovering UPnP/IGD devices")
-        local_ip = _get_local_ip(hass)
-        discoveries = await IgdDevice.async_search(source_ip=local_ip, timeout=10)
-
-        # Supplement/standardize discovery.
-        for discovery in discoveries:
-            discovery[DISCOVERY_UDN] = discovery["_udn"]
-            discovery[DISCOVERY_ST] = discovery["st"]
-            discovery[DISCOVERY_LOCATION] = discovery["location"]
-            discovery[DISCOVERY_USN] = discovery["usn"]
-            _LOGGER.debug("Discovered device: %s", discovery)
-
+        discoveries = []
+        for ssdp_st in IgdDevice.DEVICE_TYPES:
+            for discovery_info in ssdp.async_get_discovery_info_by_st(hass, ssdp_st):
+                discoveries.append(discovery_info_to_discovery(discovery_info))
         return discoveries
 
     @classmethod

homeassistant/components/upnp/manifest.json

@@ -4,6 +4,7 @@
   "config_flow": true,
   "documentation": "https://www.home-assistant.io/integrations/upnp",
   "requirements": ["async-upnp-client==0.18.0"],
+  "dependencies": ["ssdp"],
   "codeowners": ["@StevenLooman"],
   "ssdp": [
     {

tests/components/upnp/test_config_flow.py

@@ -1,7 +1,7 @@
 """Test UPnP/IGD config flow."""
 
 from datetime import timedelta
-from unittest.mock import AsyncMock, patch
+from unittest.mock import AsyncMock, Mock, patch
 
 from homeassistant import config_entries, data_entry_flow
 from homeassistant.components import ssdp
@@ -35,6 +35,14 @@ async def test_flow_ssdp_discovery(hass: HomeAssistant):
     udn = "uuid:device_1"
     location = "dummy"
     mock_device = MockDevice(udn)
+    ssdp_discoveries = [
+        {
+            ssdp.ATTR_SSDP_LOCATION: location,
+            ssdp.ATTR_SSDP_ST: mock_device.device_type,
+            ssdp.ATTR_UPNP_UDN: mock_device.udn,
+            ssdp.ATTR_SSDP_USN: mock_device.usn,
+        }
+    ]
     discoveries = [
         {
             DISCOVERY_LOCATION: location,
@@ -49,7 +57,7 @@ async def test_flow_ssdp_discovery(hass: HomeAssistant):
     with patch.object(
         Device, "async_create_device", AsyncMock(return_value=mock_device)
     ), patch.object(
-        Device, "async_discover", AsyncMock(return_value=discoveries)
+        ssdp, "async_get_discovery_info_by_st", Mock(return_value=ssdp_discoveries)
     ), patch.object(
         Device, "async_supplement_discovery", AsyncMock(return_value=discoveries[0])
     ):
@@ -156,6 +164,14 @@ async def test_flow_user(hass: HomeAssistant):
     udn = "uuid:device_1"
     location = "dummy"
     mock_device = MockDevice(udn)
+    ssdp_discoveries = [
+        {
+            ssdp.ATTR_SSDP_LOCATION: location,
+            ssdp.ATTR_SSDP_ST: mock_device.device_type,
+            ssdp.ATTR_UPNP_UDN: mock_device.udn,
+            ssdp.ATTR_SSDP_USN: mock_device.usn,
+        }
+    ]
     discoveries = [
         {
             DISCOVERY_LOCATION: location,
@@ -171,7 +187,7 @@ async def test_flow_user(hass: HomeAssistant):
     with patch.object(
         Device, "async_create_device", AsyncMock(return_value=mock_device)
     ), patch.object(
-        Device, "async_discover", AsyncMock(return_value=discoveries)
+        ssdp, "async_get_discovery_info_by_st", Mock(return_value=ssdp_discoveries)
     ), patch.object(
         Device, "async_supplement_discovery", AsyncMock(return_value=discoveries[0])
     ):
@@ -202,6 +218,14 @@ async def test_flow_import(hass: HomeAssistant):
     udn = "uuid:device_1"
     mock_device = MockDevice(udn)
     location = "dummy"
+    ssdp_discoveries = [
+        {
+            ssdp.ATTR_SSDP_LOCATION: location,
+            ssdp.ATTR_SSDP_ST: mock_device.device_type,
+            ssdp.ATTR_UPNP_UDN: mock_device.udn,
+            ssdp.ATTR_SSDP_USN: mock_device.usn,
+        }
+    ]
     discoveries = [
         {
             DISCOVERY_LOCATION: location,
@@ -217,7 +241,7 @@ async def test_flow_import(hass: HomeAssistant):
     with patch.object(
         Device, "async_create_device", AsyncMock(return_value=mock_device)
     ), patch.object(
-        Device, "async_discover", AsyncMock(return_value=discoveries)
+        ssdp, "async_get_discovery_info_by_st", Mock(return_value=ssdp_discoveries)
     ), patch.object(
         Device, "async_supplement_discovery", AsyncMock(return_value=discoveries[0])
     ):
@@ -261,31 +285,19 @@ async def test_flow_import_already_configured(hass: HomeAssistant):
     assert result["reason"] == "already_configured"
 
 
-async def test_flow_import_incomplete(hass: HomeAssistant):
-    """Test config flow: incomplete discovery, configured through configuration.yaml."""
-    udn = "uuid:device_1"
-    mock_device = MockDevice(udn)
-    location = "dummy"
-    discoveries = [
-        {
-            DISCOVERY_LOCATION: location,
-            DISCOVERY_NAME: mock_device.name,
-            # DISCOVERY_ST: mock_device.device_type,
-            DISCOVERY_UDN: mock_device.udn,
-            DISCOVERY_UNIQUE_ID: mock_device.unique_id,
-            DISCOVERY_USN: mock_device.usn,
-            DISCOVERY_HOSTNAME: mock_device.hostname,
-        }
-    ]
-
-    with patch.object(Device, "async_discover", AsyncMock(return_value=discoveries)):
+async def test_flow_import_no_devices_found(hass: HomeAssistant):
+    """Test config flow: no devices found, configured through configuration.yaml."""
+    ssdp_discoveries = []
+    with patch.object(
+        ssdp, "async_get_discovery_info_by_st", Mock(return_value=ssdp_discoveries)
+    ):
         # Discovered via step import.
         result = await hass.config_entries.flow.async_init(
             DOMAIN, context={"source": config_entries.SOURCE_IMPORT}
         )
 
         assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT
-        assert result["reason"] == "incomplete_discovery"
+        assert result["reason"] == "no_devices_found"
 
 
 async def test_options_flow(hass: HomeAssistant):
@@ -294,15 +306,12 @@ async def test_options_flow(hass: HomeAssistant):
     udn = "uuid:device_1"
     location = "http://192.168.1.1/desc.xml"
     mock_device = MockDevice(udn)
-    discoveries = [
+    ssdp_discoveries = [
         {
-            DISCOVERY_LOCATION: location,
-            DISCOVERY_NAME: mock_device.name,
-            DISCOVERY_ST: mock_device.device_type,
-            DISCOVERY_UDN: mock_device.udn,
-            DISCOVERY_UNIQUE_ID: mock_device.unique_id,
-            DISCOVERY_USN: mock_device.usn,
-            DISCOVERY_HOSTNAME: mock_device.hostname,
+            ssdp.ATTR_SSDP_LOCATION: location,
+            ssdp.ATTR_SSDP_ST: mock_device.device_type,
+            ssdp.ATTR_UPNP_UDN: mock_device.udn,
+            ssdp.ATTR_SSDP_USN: mock_device.usn,
         }
     ]
     config_entry = MockConfigEntry(
@@ -321,7 +330,11 @@ async def test_options_flow(hass: HomeAssistant):
     }
     with patch.object(
         Device, "async_create_device", AsyncMock(return_value=mock_device)
-    ), patch.object(Device, "async_discover", AsyncMock(return_value=discoveries)):
+    ), patch.object(
+        ssdp,
+        "async_get_discovery_info_by_udn_st",
+        Mock(return_value=ssdp_discoveries[0]),
+    ):
         # Initialisation of component.
         await async_setup_component(hass, "upnp", config)
         await hass.async_block_till_done()

tests/components/upnp/test_init.py

@@ -1,17 +1,11 @@
 """Test UPnP/IGD setup process."""
 
-from unittest.mock import AsyncMock, patch
+from unittest.mock import AsyncMock, Mock, patch
 
+from homeassistant.components import ssdp
 from homeassistant.components.upnp.const import (
     CONFIG_ENTRY_ST,
     CONFIG_ENTRY_UDN,
-    DISCOVERY_HOSTNAME,
-    DISCOVERY_LOCATION,
-    DISCOVERY_NAME,
-    DISCOVERY_ST,
-    DISCOVERY_UDN,
-    DISCOVERY_UNIQUE_ID,
-    DISCOVERY_USN,
     DOMAIN,
 )
 from homeassistant.components.upnp.device import Device
@@ -28,17 +22,12 @@ async def test_async_setup_entry_default(hass: HomeAssistant):
     udn = "uuid:device_1"
     location = "http://192.168.1.1/desc.xml"
     mock_device = MockDevice(udn)
-    discoveries = [
-        {
-            DISCOVERY_LOCATION: location,
-            DISCOVERY_NAME: mock_device.name,
-            DISCOVERY_ST: mock_device.device_type,
-            DISCOVERY_UDN: mock_device.udn,
-            DISCOVERY_UNIQUE_ID: mock_device.unique_id,
-            DISCOVERY_USN: mock_device.usn,
-            DISCOVERY_HOSTNAME: mock_device.hostname,
-        }
-    ]
+    discovery = {
+        ssdp.ATTR_SSDP_LOCATION: location,
+        ssdp.ATTR_SSDP_ST: mock_device.device_type,
+        ssdp.ATTR_UPNP_UDN: mock_device.udn,
+        ssdp.ATTR_SSDP_USN: mock_device.usn,
+    }
     entry = MockConfigEntry(
         domain=DOMAIN,
         data={
@@ -51,77 +40,19 @@ async def test_async_setup_entry_default(hass: HomeAssistant):
         # no upnp
     }
     async_create_device = AsyncMock(return_value=mock_device)
-    async_discover = AsyncMock()
+    mock_get_discovery = Mock()
     with patch.object(Device, "async_create_device", async_create_device), patch.object(
-        Device, "async_discover", async_discover
+        ssdp, "async_get_discovery_info_by_udn_st", mock_get_discovery
     ):
         # initialisation of component, no device discovered
-        async_discover.return_value = []
+        mock_get_discovery.return_value = None
         await async_setup_component(hass, "upnp", config)
         await hass.async_block_till_done()
 
         # loading of config_entry, device discovered
-        async_discover.return_value = discoveries
+        mock_get_discovery.return_value = discovery
         entry.add_to_hass(hass)
         assert await hass.config_entries.async_setup(entry.entry_id) is True
 
         # ensure device is stored/used
-        async_create_device.assert_called_with(hass, discoveries[0][DISCOVERY_LOCATION])
-
-
-async def test_sync_setup_entry_multiple_discoveries(hass: HomeAssistant):
-    """Test async_setup_entry."""
-    udn_0 = "uuid:device_1"
-    location_0 = "http://192.168.1.1/desc.xml"
-    mock_device_0 = MockDevice(udn_0)
-    udn_1 = "uuid:device_2"
-    location_1 = "http://192.168.1.2/desc.xml"
-    mock_device_1 = MockDevice(udn_1)
-    discoveries = [
-        {
-            DISCOVERY_LOCATION: location_0,
-            DISCOVERY_NAME: mock_device_0.name,
-            DISCOVERY_ST: mock_device_0.device_type,
-            DISCOVERY_UDN: mock_device_0.udn,
-            DISCOVERY_UNIQUE_ID: mock_device_0.unique_id,
-            DISCOVERY_USN: mock_device_0.usn,
-            DISCOVERY_HOSTNAME: mock_device_0.hostname,
-        },
-        {
-            DISCOVERY_LOCATION: location_1,
-            DISCOVERY_NAME: mock_device_1.name,
-            DISCOVERY_ST: mock_device_1.device_type,
-            DISCOVERY_UDN: mock_device_1.udn,
-            DISCOVERY_UNIQUE_ID: mock_device_1.unique_id,
-            DISCOVERY_USN: mock_device_1.usn,
-            DISCOVERY_HOSTNAME: mock_device_1.hostname,
-        },
-    ]
-    entry = MockConfigEntry(
-        domain=DOMAIN,
-        data={
-            CONFIG_ENTRY_UDN: mock_device_1.udn,
-            CONFIG_ENTRY_ST: mock_device_1.device_type,
-        },
-    )
-
-    config = {
-        # no upnp
-    }
-    async_create_device = AsyncMock(return_value=mock_device_1)
-    async_discover = AsyncMock()
-    with patch.object(Device, "async_create_device", async_create_device), patch.object(
-        Device, "async_discover", async_discover
-    ):
-        # initialisation of component, no device discovered
-        async_discover.return_value = []
-        await async_setup_component(hass, "upnp", config)
-        await hass.async_block_till_done()
-
-        # loading of config_entry, device discovered
-        async_discover.return_value = discoveries
-        entry.add_to_hass(hass)
-        assert await hass.config_entries.async_setup(entry.entry_id) is True
-
-        # ensure device is stored/used
-        async_create_device.assert_called_with(hass, discoveries[1][DISCOVERY_LOCATION])
+        async_create_device.assert_called_with(hass, discovery[ssdp.ATTR_SSDP_LOCATION])