Improve UPnP configuration flow

homeassistant/components/discovery/__init__.py

@@ -49,7 +49,6 @@
 CONFIG_ENTRY_HANDLERS = {
     SERVICE_DAIKIN: "daikin",
     SERVICE_TELLDUSLIVE: "tellduslive",
-    SERVICE_IGD: "upnp",
 }
 
 SERVICE_HANDLERS = {
@@ -94,6 +93,7 @@
     SERVICE_HEOS,
     "harmony",
     "homekit",
+    SERVICE_IGD,
     "ikea_tradfri",
     "philips_hue",
     "sonos",

homeassistant/components/upnp/__init__.py

@@ -6,6 +6,7 @@
 import voluptuous as vol
 
 from homeassistant import config_entries
+from homeassistant.components import ssdp
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import EVENT_HOMEASSISTANT_STOP
 from homeassistant.exceptions import ConfigEntryNotReady
@@ -94,17 +95,17 @@ async def async_discover_and_construct(
         return None
 
     if udn:
-        # get the discovery info with specified UDN
-        _LOGGER.debug("Discovery_infos: %s", discovery_infos)
+        # get the discovery info with specified UDN/ST
         filtered = [di for di in discovery_infos if di["udn"] == udn]
         if st:
             _LOGGER.debug("Filtering on ST: %s", st)
             filtered = [di for di in discovery_infos if di["st"] == st]
         if not filtered:
             _LOGGER.warning(
-                'Wanted UPnP/IGD device with UDN "%s" not found, ' "aborting", udn
+                'Wanted UPnP/IGD device with UDN "%s" not found, aborting', udn
             )
             return None
+
         # ensure we're always taking the latest
         filtered = sorted(filtered, key=itemgetter("st"), reverse=True)
         discovery_info = filtered[0]
@@ -117,12 +118,13 @@ async def async_discover_and_construct(
             )
             _LOGGER.info("Detected multiple UPnP/IGD devices, using: %s", device_name)
 
-    ssdp_description = discovery_info["ssdp_description"]
-    return await Device.async_create_device(hass, ssdp_description)
+    ssdp_location = discovery_info[ssdp.ATTR_SSDP_LOCATION]
+    return await Device.async_create_device(hass, ssdp_location)
 
 
 async def async_setup(hass: HomeAssistantType, config: ConfigType):
     """Set up UPnP component."""
+    _LOGGER.debug("async_setup, config: %s", config)
     conf_default = CONFIG_SCHEMA({DOMAIN: {}})[DOMAIN]
     conf = config.get(DOMAIN, conf_default)
     local_ip = await hass.async_add_executor_job(get_local_ip)
@@ -133,7 +135,8 @@ async def async_setup(hass: HomeAssistantType, config: ConfigType):
         "ports": conf.get(CONF_PORTS),
     }
 
-    if conf is not None:
+    # Only start if set up via configuration.yaml.
+    if DOMAIN in config:
         hass.async_create_task(
             hass.config_entries.flow.async_init(
                 DOMAIN, context={"source": config_entries.SOURCE_IMPORT}
@@ -145,6 +148,7 @@ async def async_setup(hass: HomeAssistantType, config: ConfigType):
 
 async def async_setup_entry(hass: HomeAssistantType, config_entry: ConfigEntry) -> bool:
     """Set up UPnP/IGD device from a config entry."""
+    _LOGGER.debug("async_setup_entry, config_entry: %s", config_entry.data)
     domain_data = hass.data[DOMAIN]
     conf = domain_data["config"]
 
@@ -160,6 +164,8 @@ async def async_setup_entry(hass: HomeAssistantType, config_entry: ConfigEntry)
     hass.data[DOMAIN]["devices"][device.udn] = device
     hass.config_entries.async_update_entry(
         entry=config_entry,
+        unique_id=device.unique_id,
+        title=device.name,
         data={**config_entry.data, "udn": device.udn, "st": device.device_type},
     )
 

homeassistant/components/upnp/config_flow.py

@@ -1,10 +1,172 @@
 """Config flow for UPNP."""
+from typing import Mapping, Optional
+import voluptuous as vol
+
 from homeassistant import config_entries
-from homeassistant.helpers import config_entry_flow
+from homeassistant.components import ssdp
 
-from .const import DOMAIN
+from .const import (  # pylint: disable=unused-import
+    DOMAIN,
+    LOGGER as _LOGGER,
+)
 from .device import Device
+from urllib.parse import urlparse
 
-config_entry_flow.register_discovery_flow(
-    DOMAIN, "UPnP/IGD", Device.async_discover, config_entries.CONN_CLASS_LOCAL_POLL
-)
+
+LOCATION = "location"
+NAME = "name"
+ST = "st"
+UDN = "udn"
+
+
+class UpnpFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
+    """Handle a UPnP/IGD config flow."""
+
+    VERSION = 1
+    CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_POLL
+
+    # Paths:
+    # - ssdp(discovery_info) --> ssdp_confirm(None) --> ssdp_confirm({}) --> create_entry()
+    # - user(None): scan --> user({...}) --> create_entry()
+    # - import(None) --> create_entry()
+
+    def __init__(self):
+        """Initialize the UPnP/IGD config flow."""
+        self._data: Mapping = None
+        self._discoveries: Mapping = None
+
+    async def async_step_user(self, user_input=None):
+        """Handle a flow start."""
+        _LOGGER.debug("async_step_user: user_input: %s", user_input)
+
+        if user_input is not None:
+            # Ensure wanted device was discovered.
+            matching_discoveries = [
+                discovery
+                for discovery in self._discoveries
+                if discovery["unique_id"] == user_input["unique_id"]
+            ]
+            if not matching_discoveries:
+                errors = {"base": "no_devices_discovered"}
+                return self.async_show_form(step_id="user", errors=errors,)
+
+            # Title/name will be updated later on.
+            self._data = matching_discoveries[0]
+            self._data[NAME] = urlparse(self._data[ssdp.ATTR_SSDP_LOCATION]).hostname
+            return self._async_create_entry_from_data()
+
+        # Discover devices.
+        self._discoveries = await Device.async_discover(self.hass)
+
+        # Filter discoveries which are already configured.
+        current_unique_ids = [
+            entry.unique_id for entry in self._async_current_entries()
+        ]
+        self._discoveries = [
+            discovery
+            for discovery in self._discoveries
+            if discovery["unique_id"] not in current_unique_ids
+        ]
+
+        # Ensure anything to add.
+        if not self._discoveries:
+            errors = {"base": "no_devices_discovered"}
+            return self.async_show_form(step_id="user", errors=errors,)
+
+        data_schema = vol.Schema(
+            {
+                vol.Required("unique_id"): vol.In(
+                    {
+                        discovery["unique_id"]: urlparse(
+                            discovery[ssdp.ATTR_SSDP_LOCATION]
+                        ).hostname
+                        for discovery in self._discoveries
+                    }
+                ),
+            }
+        )
+        return self.async_show_form(step_id="user", data_schema=data_schema,)
+
+    async def async_step_import(self, import_info: Optional[Mapping]):
+        """Import a new UPnP/IGD device as a config entry.
+
+        This flow is triggered by `async_setup`.
+        """
+        _LOGGER.debug("async_step_import: import_info: %s", import_info)
+
+        if import_info is None:
+            # Landed here via configuration.yaml entry.
+            # Any device already added, then abort.
+            if self._async_current_entries():
+                _LOGGER.debug("aborting, already configured")
+                return self.async_abort(reason="already_configured")
+
+        # Test if import_info isn't already configured.
+        if import_info is not None and any(
+            import_info["udn"] == entry.data["udn"]
+            and import_info["st"] == entry.data["st"]
+            for entry in self._async_current_entries()
+        ):
+            return self.async_abort(reason="already_configured")
+
+        # Discover devices.
+        self._discoveries = await Device.async_discover(self.hass)
+
+        # Ensure anything to add. If not, silently abort.
+        if not self._discoveries:
+            _LOGGER.info("No UPnP devices discovered, aborting.")
+            return self.async_abort(reason="no_devices_found")
+
+        # Create new config_entry.
+        self._data = self._discoveries[0]
+        return self._async_create_entry_from_data()
+
+    async def async_step_ssdp(self, discovery_info: Mapping):
+        """Handle a discovered UPnP/IGD device.
+
+        This flow is triggered by the SSDP component. It will check if the
+        host is already configured and delegate to the import step if not.
+        """
+        _LOGGER.debug("async_step_ssdp: discovery_info: %s", discovery_info)
+
+        # Ensure not already configuring/configured.
+        udn = discovery_info[ssdp.ATTR_UPNP_UDN]
+        st = discovery_info[ssdp.ATTR_SSDP_ST]  # pylint: disable=invalid-name
+        usn = f"{udn}::{st}"
+        await self.async_set_unique_id(usn)
+        self._abort_if_unique_id_configured(updates={UDN: udn, ST: st})
+
+        # Store discovery.
+        name = discovery_info.get("friendlyName", "")
+        self._data = {
+            UDN: udn,
+            ST: st,
+            NAME: name,
+        }
+
+        # Ensure user recognizable.
+        location = discovery_info[ssdp.ATTR_SSDP_LOCATION]
+        # pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167
+        self.context["title_placeholders"] = {
+            "name": name,
+            "host": urlparse(location).hostname,
+        }
+
+        return await self.async_step_ssdp_confirm()
+
+    async def async_step_ssdp_confirm(self, user_input=None):
+        """Confirm integration via SSDP."""
+        _LOGGER.debug("async_step_ssdp_confirm: user_input: %s", user_input)
+        if user_input is None:
+            return self.async_show_form(step_id="ssdp_confirm")
+
+        return self._async_create_entry_from_data()
+
+    def _async_create_entry_from_data(self):
+        """Create an entry from own _data."""
+        title = self._data["name"]
+        data = {
+            UDN: self._data["udn"],
+            ST: self._data["st"],
+        }
+        return self.async_create_entry(title=title, data=data)

homeassistant/components/upnp/device.py

@@ -1,13 +1,14 @@
 """Home Assistant representation of an UPnP/IGD."""
 import asyncio
 from ipaddress import IPv4Address
-from typing import Mapping
+from typing import List, Mapping
 
 import aiohttp
 from async_upnp_client import UpnpError, UpnpFactory
 from async_upnp_client.aiohttp import AiohttpSessionRequester
 from async_upnp_client.profiles.igd import IgdDevice
 
+from homeassistant.components import ssdp
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
 from homeassistant.helpers.typing import HomeAssistantType
 import homeassistant.util.dt as dt_util
@@ -33,7 +34,7 @@ def __init__(self, igd_device):
         self._mapped_ports = []
 
     @classmethod
-    async def async_discover(cls, hass: HomeAssistantType):
+    async def async_discover(cls, hass: HomeAssistantType) -> List[Mapping]:
         """Discover UPnP/IGD devices."""
         _LOGGER.debug("Discovering UPnP/IGD devices")
         local_ip = None
@@ -47,8 +48,13 @@ async def async_discover(cls, hass: HomeAssistantType):
         # add extra info and store devices
         devices = []
         for discovery_info in discovery_infos:
-            discovery_info["udn"] = discovery_info["_udn"]
-            discovery_info["ssdp_description"] = discovery_info["location"]
+            # Become more ssdp-component-discovery-like.
+            discovery_info[ssdp.ATTR_UPNP_UDN] = discovery_info["_udn"]
+            discovery_info[ssdp.ATTR_SSDP_ST] = discovery_info["st"]
+            discovery_info[ssdp.ATTR_SSDP_LOCATION] = discovery_info["location"]
+
+            unique_id = f"{discovery_info[ssdp.ATTR_UPNP_UDN]}::{discovery_info[ssdp.ATTR_SSDP_ST]}"
+            discovery_info["unique_id"] = unique_id
             discovery_info["source"] = "async_upnp_client"
             _LOGGER.debug("Discovered device: %s", discovery_info)
 
@@ -57,15 +63,15 @@ async def async_discover(cls, hass: HomeAssistantType):
         return devices
 
     @classmethod
-    async def async_create_device(cls, hass: HomeAssistantType, ssdp_description: str):
+    async def async_create_device(cls, hass: HomeAssistantType, ssdp_location: str):
         """Create UPnP/IGD device."""
         # build async_upnp_client requester
         session = async_get_clientsession(hass)
         requester = AiohttpSessionRequester(session, True)
 
         # create async_upnp_client device
         factory = UpnpFactory(requester, disable_state_variable_validation=True)
-        upnp_device = await factory.async_create_device(ssdp_description)
+        upnp_device = await factory.async_create_device(ssdp_location)
 
         igd_device = IgdDevice(upnp_device, None)
 
@@ -96,6 +102,11 @@ def device_type(self) -> str:
         """Get the device type."""
         return self._igd_device.device_type
 
+    @property
+    def unique_id(self) -> str:
+        """Get the unique id."""
+        return f"{self.udn}::{self.device_type}"
+
     def __str__(self) -> str:
         """Get string representation."""
         return f"IGD Device: {self.name}/{self.udn}"

homeassistant/components/upnp/translations/en.json

@@ -1,26 +1,25 @@
 {
     "config": {
+        "flow_title": "UPnP/IGD: {name} ({host})",
         "abort": {
             "already_configured": "UPnP/IGD is already configured",
-            "incomplete_device": "Ignoring incomplete UPnP device",
             "no_devices_discovered": "No UPnP/IGDs discovered",
-            "no_devices_found": "No UPnP/IGD devices found on the network.",
-            "no_sensors_or_port_mapping": "Enable at least sensors or port mapping",
-            "single_instance_allowed": "Only a single configuration of UPnP/IGD is necessary."
+            "no_devices_found": "No UPnP/IGD devices found on the network."
         },
         "step": {
-            "confirm": {
-                "description": "Do you want to set up UPnP/IGD?",
-                "title": "UPnP/IGD"
-            },
             "init": {
                 "title": "UPnP/IGD"
             },
+            "ssdp_confirm": {
+                "data": {
+                    "name": "Name"
+                },
+                "title": "UPnP/IGD via SSDP Confirm DEBUG",
+                "description": "Do you want to set up the UPnP/IGD device?"
+            },
             "user": {
                 "data": {
-                    "enable_port_mapping": "Enable port mapping for Home Assistant",
-                    "enable_sensors": "Add traffic sensors",
-                    "igd": "UPnP/IGD"
+                    "unique_id": "Host"
                 },
                 "title": "Configuration options"
             }