From 57c9aad9b17a37a60f7313f01fc64fa61a4b7005 Mon Sep 17 00:00:00 2001 From: Barry Williams Date: Wed, 21 Jun 2023 17:46:17 +0100 Subject: [PATCH] Migrate Linn / Openhome integration to SSDP config flow (#94564) * Migrate Linn / Openhome integration to SSDP config flow * moved device initialisation into __init__ * wait for user step before adding openhome entities * add CONFIG_SCHEMA * cover confirmation step in config flow test * Address comments --------- Co-authored-by: Paulus Schoutsen --- CODEOWNERS | 1 + homeassistant/components/openhome/__init__.py | 54 ++++++++ .../components/openhome/config_flow.py | 66 ++++++++++ .../components/openhome/manifest.json | 17 ++- .../components/openhome/media_player.py | 42 +++---- homeassistant/generated/config_flows.py | 1 + homeassistant/generated/integrations.json | 2 +- homeassistant/generated/ssdp.py | 14 +++ requirements_test_all.txt | 3 + tests/components/openhome/__init__.py | 1 + tests/components/openhome/test_config_flow.py | 116 ++++++++++++++++++ 11 files changed, 293 insertions(+), 24 deletions(-) create mode 100644 homeassistant/components/openhome/config_flow.py create mode 100644 tests/components/openhome/__init__.py create mode 100644 tests/components/openhome/test_config_flow.py diff --git a/CODEOWNERS b/CODEOWNERS index b41b14d46fb..c82f249a906 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -885,6 +885,7 @@ build.json @home-assistant/supervisor /homeassistant/components/opengarage/ @danielhiversen /tests/components/opengarage/ @danielhiversen /homeassistant/components/openhome/ @bazwilliams +/tests/components/openhome/ @bazwilliams /homeassistant/components/opensky/ @joostlek /homeassistant/components/opentherm_gw/ @mvn23 /tests/components/opentherm_gw/ @mvn23 diff --git a/homeassistant/components/openhome/__init__.py b/homeassistant/components/openhome/__init__.py index 78294ceb6f4..d201646e81c 100644 --- a/homeassistant/components/openhome/__init__.py +++ b/homeassistant/components/openhome/__init__.py @@ -1 +1,55 @@ """The openhome component.""" + +import asyncio +import logging + +import aiohttp +from async_upnp_client.client import UpnpError +from openhomedevice.device import Device + +from homeassistant.config_entries import ConfigEntry +from homeassistant.const import CONF_HOST, Platform +from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ConfigEntryNotReady +from homeassistant.helpers import config_validation as cv + +from .const import DOMAIN + +_LOGGER = logging.getLogger(__name__) + +PLATFORMS = [Platform.MEDIA_PLAYER] + +CONFIG_SCHEMA = cv.empty_config_schema(DOMAIN) + + +async def async_unload_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: + """Cleanup before removing config entry.""" + unload_ok = await hass.config_entries.async_unload_platforms( + config_entry, PLATFORMS + ) + hass.data[DOMAIN].pop(config_entry.entry_id) + + return unload_ok + + +async def async_setup_entry( + hass: HomeAssistant, + config_entry: ConfigEntry, +) -> bool: + """Set up the configuration config entry.""" + _LOGGER.debug("Setting up config entry: %s", config_entry.unique_id) + + device = await hass.async_add_executor_job(Device, config_entry.data[CONF_HOST]) + + try: + await device.init() + except (asyncio.TimeoutError, aiohttp.ClientError, UpnpError) as exc: + raise ConfigEntryNotReady from exc + + _LOGGER.debug("Initialised device: %s", device.uuid()) + + hass.data.setdefault(DOMAIN, {})[config_entry.entry_id] = device + + await hass.config_entries.async_forward_entry_setups(config_entry, PLATFORMS) + + return True diff --git a/homeassistant/components/openhome/config_flow.py b/homeassistant/components/openhome/config_flow.py new file mode 100644 index 00000000000..c8a13a3c7aa --- /dev/null +++ b/homeassistant/components/openhome/config_flow.py @@ -0,0 +1,66 @@ +"""Config flow for Linn / OpenHome.""" + +import logging +from typing import Any + +from homeassistant.components.ssdp import ( + ATTR_UPNP_FRIENDLY_NAME, + ATTR_UPNP_UDN, + SsdpServiceInfo, +) +from homeassistant.config_entries import ConfigFlow +from homeassistant.const import CONF_HOST, CONF_NAME +from homeassistant.data_entry_flow import FlowResult + +from .const import DOMAIN + +_LOGGER = logging.getLogger(__name__) + + +def _is_complete_discovery(discovery_info: SsdpServiceInfo) -> bool: + """Test if discovery is complete and usable.""" + return bool(ATTR_UPNP_UDN in discovery_info.upnp and discovery_info.ssdp_location) + + +class OpenhomeConfigFlow(ConfigFlow, domain=DOMAIN): + """Handle an Openhome config flow.""" + + async def async_step_ssdp(self, discovery_info: SsdpServiceInfo) -> FlowResult: + """Handle a flow initialized by discovery.""" + _LOGGER.debug("async_step_ssdp: started") + + if not _is_complete_discovery(discovery_info): + _LOGGER.debug("async_step_ssdp: Incomplete discovery, ignoring") + return self.async_abort(reason="incomplete_discovery") + + _LOGGER.debug( + "async_step_ssdp: setting unique id %s", discovery_info.upnp[ATTR_UPNP_UDN] + ) + + await self.async_set_unique_id(discovery_info.upnp[ATTR_UPNP_UDN]) + self._abort_if_unique_id_configured({CONF_HOST: discovery_info.ssdp_location}) + + _LOGGER.debug( + "async_step_ssdp: create entry %s", discovery_info.upnp[ATTR_UPNP_UDN] + ) + + self.context[CONF_NAME] = discovery_info.upnp[ATTR_UPNP_FRIENDLY_NAME] + self.context[CONF_HOST] = discovery_info.ssdp_location + + return await self.async_step_confirm() + + async def async_step_confirm( + self, user_input: dict[str, Any] | None = None + ) -> FlowResult: + """Handle user-confirmation of discovered node.""" + + if user_input is not None: + return self.async_create_entry( + title=self.context[CONF_NAME], + data={CONF_HOST: self.context[CONF_HOST]}, + ) + + return self.async_show_form( + step_id="confirm", + description_placeholders={CONF_NAME: self.context[CONF_NAME]}, + ) diff --git a/homeassistant/components/openhome/manifest.json b/homeassistant/components/openhome/manifest.json index aa563151f0b..61d425895bf 100644 --- a/homeassistant/components/openhome/manifest.json +++ b/homeassistant/components/openhome/manifest.json @@ -2,8 +2,23 @@ "domain": "openhome", "name": "Linn / OpenHome", "codeowners": ["@bazwilliams"], + "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/openhome", "iot_class": "local_polling", "loggers": ["async_upnp_client", "openhomedevice"], - "requirements": ["openhomedevice==2.0.2"] + "requirements": ["openhomedevice==2.0.2"], + "ssdp": [ + { + "st": "urn:av-openhome-org:service:Product:1" + }, + { + "st": "urn:av-openhome-org:service:Product:2" + }, + { + "st": "urn:av-openhome-org:service:Product:3" + }, + { + "st": "urn:av-openhome-org:service:Product:4" + } + ] } diff --git a/homeassistant/components/openhome/media_player.py b/homeassistant/components/openhome/media_player.py index b625d9976da..c0941906e40 100644 --- a/homeassistant/components/openhome/media_player.py +++ b/homeassistant/components/openhome/media_player.py @@ -9,7 +9,6 @@ from typing import Any, Concatenate, ParamSpec, TypeVar import aiohttp from async_upnp_client.client import UpnpError -from openhomedevice.device import Device import voluptuous as vol from homeassistant.components import media_source @@ -21,12 +20,13 @@ from homeassistant.components.media_player import ( MediaType, async_process_play_media_url, ) +from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant from homeassistant.helpers import config_validation as cv, entity_platform +from homeassistant.helpers.entity import DeviceInfo from homeassistant.helpers.entity_platform import AddEntitiesCallback -from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType -from .const import ATTR_PIN_INDEX, DATA_OPENHOME, SERVICE_INVOKE_PIN +from .const import ATTR_PIN_INDEX, DOMAIN, SERVICE_INVOKE_PIN _OpenhomeDeviceT = TypeVar("_OpenhomeDeviceT", bound="OpenhomeDevice") _R = TypeVar("_R") @@ -41,34 +41,20 @@ SUPPORT_OPENHOME = ( _LOGGER = logging.getLogger(__name__) -async def async_setup_platform( +async def async_setup_entry( hass: HomeAssistant, - config: ConfigType, + config_entry: ConfigEntry, async_add_entities: AddEntitiesCallback, - discovery_info: DiscoveryInfoType | None = None, ) -> None: - """Set up the Openhome platform.""" + """Set up the Openhome config entry.""" - if not discovery_info: - return + _LOGGER.debug("Setting up config entry: %s", config_entry.unique_id) - openhome_data = hass.data.setdefault(DATA_OPENHOME, set()) - - name = discovery_info.get("name") - description = discovery_info.get("ssdp_description") - - _LOGGER.info("Openhome device found: %s", name) - device = await hass.async_add_executor_job(Device, description) - await device.init() - - # if device has already been discovered - if device.uuid() in openhome_data: - return + device = hass.data[DOMAIN][config_entry.entry_id] entity = OpenhomeDevice(hass, device) async_add_entities([entity]) - openhome_data.add(device.uuid()) platform = entity_platform.async_get_current_platform() @@ -133,6 +119,18 @@ class OpenhomeDevice(MediaPlayerEntity): self._attr_state = MediaPlayerState.PLAYING self._available = True + @property + def device_info(self): + """Return a device description for device registry.""" + return DeviceInfo( + identifiers={ + (DOMAIN, self._device.uuid()), + }, + manufacturer=self._device.device.manufacturer, + model=self._device.device.model_name, + name=self._device.device.friendly_name, + ) + @property def available(self): """Device is available.""" diff --git a/homeassistant/generated/config_flows.py b/homeassistant/generated/config_flows.py index 05de95f902e..13c3a385756 100644 --- a/homeassistant/generated/config_flows.py +++ b/homeassistant/generated/config_flows.py @@ -320,6 +320,7 @@ FLOWS = { "openai_conversation", "openexchangerates", "opengarage", + "openhome", "opentherm_gw", "openuv", "openweathermap", diff --git a/homeassistant/generated/integrations.json b/homeassistant/generated/integrations.json index f1ad6c28828..e67f385f4b4 100644 --- a/homeassistant/generated/integrations.json +++ b/homeassistant/generated/integrations.json @@ -3954,7 +3954,7 @@ "openhome": { "name": "Linn / OpenHome", "integration_type": "hub", - "config_flow": false, + "config_flow": true, "iot_class": "local_polling" }, "opensensemap": { diff --git a/homeassistant/generated/ssdp.py b/homeassistant/generated/ssdp.py index 3a2097a1d30..8e7319917f0 100644 --- a/homeassistant/generated/ssdp.py +++ b/homeassistant/generated/ssdp.py @@ -224,6 +224,20 @@ SSDP = { "manufacturer": "The OctoPrint Project", }, ], + "openhome": [ + { + "st": "urn:av-openhome-org:service:Product:1", + }, + { + "st": "urn:av-openhome-org:service:Product:2", + }, + { + "st": "urn:av-openhome-org:service:Product:3", + }, + { + "st": "urn:av-openhome-org:service:Product:4", + }, + ], "roku": [ { "deviceType": "urn:roku-com:device:player:1-0", diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 0755a43c1bb..5c0faae7555 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -1026,6 +1026,9 @@ openai==0.27.2 # homeassistant.components.openerz openerz-api==0.2.0 +# homeassistant.components.openhome +openhomedevice==2.0.2 + # homeassistant.components.oralb oralb-ble==0.17.6 diff --git a/tests/components/openhome/__init__.py b/tests/components/openhome/__init__.py new file mode 100644 index 00000000000..39c7a3a4dcb --- /dev/null +++ b/tests/components/openhome/__init__.py @@ -0,0 +1 @@ +"""Tests for the Linn / OpenHome integration.""" diff --git a/tests/components/openhome/test_config_flow.py b/tests/components/openhome/test_config_flow.py new file mode 100644 index 00000000000..4cc5c58dda5 --- /dev/null +++ b/tests/components/openhome/test_config_flow.py @@ -0,0 +1,116 @@ +"""Tests for the Openhome config flow module.""" + +from homeassistant import data_entry_flow +from homeassistant.components import ssdp +from homeassistant.components.openhome.const import DOMAIN +from homeassistant.components.ssdp import ATTR_UPNP_FRIENDLY_NAME, ATTR_UPNP_UDN +from homeassistant.config_entries import SOURCE_SSDP +from homeassistant.const import CONF_HOST, CONF_NAME, CONF_SOURCE +from homeassistant.core import HomeAssistant +from homeassistant.data_entry_flow import FlowResultType + +from tests.common import MockConfigEntry + +MOCK_UDN = "uuid:4c494e4e-1234-ab12-abcd-01234567819f" +MOCK_FRIENDLY_NAME = "Test Client" +MOCK_SSDP_LOCATION = "http://device:12345/description.xml" + +MOCK_DISCOVER = ssdp.SsdpServiceInfo( + ssdp_usn="usn", + ssdp_st="st", + ssdp_location=MOCK_SSDP_LOCATION, + upnp={ATTR_UPNP_FRIENDLY_NAME: MOCK_FRIENDLY_NAME, ATTR_UPNP_UDN: MOCK_UDN}, +) + + +async def test_ssdp(hass: HomeAssistant) -> None: + """Test a ssdp import flow.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={CONF_SOURCE: SOURCE_SSDP}, + data=MOCK_DISCOVER, + ) + + assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "confirm" + assert result["description_placeholders"] == {CONF_NAME: MOCK_FRIENDLY_NAME} + + result2 = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result2["title"] == MOCK_FRIENDLY_NAME + assert result2["data"] == {CONF_HOST: MOCK_SSDP_LOCATION} + + +async def test_device_exists(hass: HomeAssistant) -> None: + """Test a ssdp import where device already exists.""" + entry = MockConfigEntry( + domain=DOMAIN, + data={CONF_HOST: MOCK_SSDP_LOCATION}, + title=MOCK_FRIENDLY_NAME, + unique_id=MOCK_UDN, + ) + entry.add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={CONF_SOURCE: SOURCE_SSDP}, + data=MOCK_DISCOVER, + ) + assert result["type"] == data_entry_flow.FlowResultType.ABORT + assert result["reason"] == "already_configured" + + +async def test_missing_udn(hass: HomeAssistant) -> None: + """Test a ssdp import where discovery is missing udn.""" + broken_discovery = ssdp.SsdpServiceInfo( + ssdp_usn="usn", + ssdp_st="st", + ssdp_location=MOCK_SSDP_LOCATION, + upnp={ + ATTR_UPNP_FRIENDLY_NAME: MOCK_FRIENDLY_NAME, + }, + ) + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={CONF_SOURCE: SOURCE_SSDP}, + data=broken_discovery, + ) + assert result["type"] == data_entry_flow.FlowResultType.ABORT + assert result["reason"] == "incomplete_discovery" + + +async def test_missing_ssdp_location(hass: HomeAssistant) -> None: + """Test a ssdp import where discovery is missing udn.""" + broken_discovery = ssdp.SsdpServiceInfo( + ssdp_usn="usn", + ssdp_st="st", + ssdp_location="", + upnp={ATTR_UPNP_FRIENDLY_NAME: MOCK_FRIENDLY_NAME, ATTR_UPNP_UDN: MOCK_UDN}, + ) + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={CONF_SOURCE: SOURCE_SSDP}, + data=broken_discovery, + ) + assert result["type"] == data_entry_flow.FlowResultType.ABORT + assert result["reason"] == "incomplete_discovery" + + +async def test_host_updated(hass: HomeAssistant) -> None: + """Test a ssdp import flow where host changes.""" + entry = MockConfigEntry( + domain=DOMAIN, + data={CONF_HOST: "old_host"}, + title=MOCK_FRIENDLY_NAME, + unique_id=MOCK_UDN, + ) + entry.add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={CONF_SOURCE: SOURCE_SSDP}, + data=MOCK_DISCOVER, + ) + assert result["type"] == data_entry_flow.FlowResultType.ABORT + assert result["reason"] == "already_configured" + + assert entry.data[CONF_HOST] == MOCK_SSDP_LOCATION