diff --git a/homeassistant/components/upnp/__init__.py b/homeassistant/components/upnp/__init__.py index 24be3b119dc..5788ec1b3ef 100644 --- a/homeassistant/components/upnp/__init__.py +++ b/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): diff --git a/homeassistant/components/upnp/config_flow.py b/homeassistant/components/upnp/config_flow.py index 8e2dfc1f43f..f52ce89660d 100644 --- a/homeassistant/components/upnp/config_flow.py +++ b/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): diff --git a/homeassistant/components/upnp/device.py b/homeassistant/components/upnp/device.py index 496293926d3..9af7cf55c24 100644 --- a/homeassistant/components/upnp/device.py +++ b/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 diff --git a/homeassistant/components/upnp/manifest.json b/homeassistant/components/upnp/manifest.json index b130e721e35..5acd260ec9a 100644 --- a/homeassistant/components/upnp/manifest.json +++ b/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": [ { diff --git a/tests/components/upnp/test_config_flow.py b/tests/components/upnp/test_config_flow.py index 93f21911c78..6a911f8d4db 100644 --- a/tests/components/upnp/test_config_flow.py +++ b/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() diff --git a/tests/components/upnp/test_init.py b/tests/components/upnp/test_init.py index e6e37ca52fb..0770906f0da 100644 --- a/tests/components/upnp/test_init.py +++ b/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])