diff --git a/homeassistant/components/heos/__init__.py b/homeassistant/components/heos/__init__.py index de56e541501..e6a46f5a4ca 100644 --- a/homeassistant/components/heos/__init__.py +++ b/homeassistant/components/heos/__init__.py @@ -8,23 +8,19 @@ from datetime import timedelta import logging from pyheos import Heos, HeosError, HeosPlayer, const as heos_const -import voluptuous as vol -from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry +from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_HOST, EVENT_HOMEASSISTANT_STOP, Platform from homeassistant.core import HomeAssistant, callback from homeassistant.exceptions import ConfigEntryNotReady, HomeAssistantError from homeassistant.helpers import device_registry as dr, entity_registry as er -import homeassistant.helpers.config_validation as cv from homeassistant.helpers.dispatcher import ( async_dispatcher_connect, async_dispatcher_send, ) -from homeassistant.helpers.typing import ConfigType from homeassistant.util import Throttle from . import services -from .config_flow import format_title from .const import ( COMMAND_RETRY_ATTEMPTS, COMMAND_RETRY_DELAY, @@ -35,14 +31,6 @@ from .const import ( PLATFORMS = [Platform.MEDIA_PLAYER] -CONFIG_SCHEMA = vol.Schema( - vol.All( - cv.deprecated(DOMAIN), - {DOMAIN: vol.Schema({vol.Required(CONF_HOST): cv.string})}, - ), - extra=vol.ALLOW_EXTRA, -) - MIN_UPDATE_SOURCES = timedelta(seconds=1) _LOGGER = logging.getLogger(__name__) @@ -61,30 +49,6 @@ class HeosRuntimeData: type HeosConfigEntry = ConfigEntry[HeosRuntimeData] -async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: - """Set up the HEOS component.""" - if DOMAIN not in config: - return True - host = config[DOMAIN][CONF_HOST] - entries = hass.config_entries.async_entries(DOMAIN) - if not entries: - # Create new entry based on config - hass.async_create_task( - hass.config_entries.flow.async_init( - DOMAIN, context={"source": SOURCE_IMPORT}, data={CONF_HOST: host} - ) - ) - else: - # Check if host needs to be updated - entry = entries[0] - if entry.data[CONF_HOST] != host: - hass.config_entries.async_update_entry( - entry, title=format_title(host), data={**entry.data, CONF_HOST: host} - ) - - return True - - async def async_setup_entry(hass: HomeAssistant, entry: HeosConfigEntry) -> bool: """Initialize config entry which represents the HEOS controller.""" # For backwards compat diff --git a/homeassistant/components/heos/config_flow.py b/homeassistant/components/heos/config_flow.py index 57ed51a3c05..e8a4dbf7b63 100644 --- a/homeassistant/components/heos/config_flow.py +++ b/homeassistant/components/heos/config_flow.py @@ -10,7 +10,7 @@ from homeassistant.components import ssdp from homeassistant.config_entries import ConfigFlow, ConfigFlowResult from homeassistant.const import CONF_HOST -from .const import DATA_DISCOVERED_HOSTS, DOMAIN +from .const import DOMAIN def format_title(host: str) -> str: @@ -34,43 +34,32 @@ class HeosFlowHandler(ConfigFlow, domain=DOMAIN): friendly_name = ( f"{discovery_info.upnp[ssdp.ATTR_UPNP_FRIENDLY_NAME]} ({hostname})" ) - self.hass.data.setdefault(DATA_DISCOVERED_HOSTS, {}) - self.hass.data[DATA_DISCOVERED_HOSTS][friendly_name] = hostname - # Abort if other flows in progress or an entry already exists - if self._async_in_progress() or self._async_current_entries(): - return self.async_abort(reason="single_instance_allowed") + self.hass.data.setdefault(DOMAIN, {}) + self.hass.data[DOMAIN][friendly_name] = hostname await self.async_set_unique_id(DOMAIN) # Show selection form return self.async_show_form(step_id="user") - async def async_step_import(self, import_data: dict[str, Any]) -> ConfigFlowResult: - """Occurs when an entry is setup through config.""" - host = import_data[CONF_HOST] - # raise_on_progress is False here in case ssdp discovers - # heos first which would block the import - await self.async_set_unique_id(DOMAIN, raise_on_progress=False) - return self.async_create_entry(title=format_title(host), data={CONF_HOST: host}) - async def async_step_user( self, user_input: dict[str, Any] | None = None ) -> ConfigFlowResult: """Obtain host and validate connection.""" - self.hass.data.setdefault(DATA_DISCOVERED_HOSTS, {}) - # Only a single entry is needed for all devices - if self._async_current_entries(): - return self.async_abort(reason="single_instance_allowed") + self.hass.data.setdefault(DOMAIN, {}) + await self.async_set_unique_id(DOMAIN) # Try connecting to host if provided errors = {} host = None if user_input is not None: host = user_input[CONF_HOST] # Map host from friendly name if in discovered hosts - host = self.hass.data[DATA_DISCOVERED_HOSTS].get(host, host) + host = self.hass.data[DOMAIN].get(host, host) heos = Heos(host) try: await heos.connect() - self.hass.data.pop(DATA_DISCOVERED_HOSTS) - return await self.async_step_import({CONF_HOST: host}) + self.hass.data.pop(DOMAIN) + return self.async_create_entry( + title=format_title(host), data={CONF_HOST: host} + ) except HeosError: errors[CONF_HOST] = "cannot_connect" finally: @@ -78,9 +67,7 @@ class HeosFlowHandler(ConfigFlow, domain=DOMAIN): # Return form host_type = ( - str - if not self.hass.data[DATA_DISCOVERED_HOSTS] - else vol.In(list(self.hass.data[DATA_DISCOVERED_HOSTS])) + str if not self.hass.data[DOMAIN] else vol.In(list(self.hass.data[DOMAIN])) ) return self.async_show_form( step_id="user", diff --git a/homeassistant/components/heos/const.py b/homeassistant/components/heos/const.py index 827a0c53fbf..5b2df2b5ebf 100644 --- a/homeassistant/components/heos/const.py +++ b/homeassistant/components/heos/const.py @@ -4,7 +4,6 @@ ATTR_PASSWORD = "password" ATTR_USERNAME = "username" COMMAND_RETRY_ATTEMPTS = 2 COMMAND_RETRY_DELAY = 1 -DATA_DISCOVERED_HOSTS = "heos_discovered_hosts" DOMAIN = "heos" SERVICE_SIGN_IN = "sign_in" SERVICE_SIGN_OUT = "sign_out" diff --git a/homeassistant/components/heos/manifest.json b/homeassistant/components/heos/manifest.json index a90f0aebaae..12f10bcd0e3 100644 --- a/homeassistant/components/heos/manifest.json +++ b/homeassistant/components/heos/manifest.json @@ -7,6 +7,7 @@ "iot_class": "local_push", "loggers": ["pyheos"], "requirements": ["pyheos==0.7.2"], + "single_config_entry": true, "ssdp": [ { "st": "urn:schemas-denon-com:device:ACT-Denon:1" diff --git a/homeassistant/components/heos/quality_scale.yaml b/homeassistant/components/heos/quality_scale.yaml index ed9939bf37c..861ca750780 100644 --- a/homeassistant/components/heos/quality_scale.yaml +++ b/homeassistant/components/heos/quality_scale.yaml @@ -8,19 +8,10 @@ rules: comment: Integration is a local push integration brands: done common-modules: todo - config-flow-test-coverage: - status: todo - comment: - 1. The config flow is 100% covered, however some tests need to let HA create the flow - handler instead of doing it manually in the test. - 2. We should also make sure every test ends in either CREATE_ENTRY or ABORT so we test - that the flow is able to recover from an error. + config-flow-test-coverage: done config-flow: - status: todo - comment: | - 1. YAML import to be removed after core team meeting discussion on approach. - 2. Consider enhnacement to automatically select a host when multiple are discovered. - 3. Move hass.data[heos_discovered_hosts] into hass.data[heos] + status: done + comment: Consider enhnacement to automatically select a host when multiple are discovered. dependency-transparency: done docs-actions: done docs-high-level-description: done @@ -34,15 +25,9 @@ rules: entity-unique-id: done has-entity-name: done runtime-data: done - test-before-configure: todo + test-before-configure: done test-before-setup: done - unique-config-entry: - status: todo - comment: | - The HEOS integration only supports a single config entry, but needs to be migrated to use - the `single_config_entry` flag. HEOS devices interconnect to each other, so connecting to - a single node yields access to all the devices setup with HEOS on your network. The HEOS API - documentation does not recommend connecting to multiple nodes which would provide no bennefit. + unique-config-entry: done # Silver action-exceptions: status: todo diff --git a/homeassistant/components/heos/strings.json b/homeassistant/components/heos/strings.json index df18fc7834a..20a8a2e978b 100644 --- a/homeassistant/components/heos/strings.json +++ b/homeassistant/components/heos/strings.json @@ -16,6 +16,7 @@ "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]" }, "abort": { + "already_in_progress": "[%key:common::config_flow::abort::already_in_progress%]", "single_instance_allowed": "[%key:common::config_flow::abort::single_instance_allowed%]" } }, diff --git a/tests/components/heos/conftest.py b/tests/components/heos/conftest.py index a12f4c610ad..95a388d87a8 100644 --- a/tests/components/heos/conftest.py +++ b/tests/components/heos/conftest.py @@ -164,6 +164,25 @@ def discovery_data_fixture() -> dict: ) +@pytest.fixture(name="discovery_data_bedroom") +def discovery_data_fixture_bedroom() -> dict: + """Return mock discovery data for testing.""" + return ssdp.SsdpServiceInfo( + ssdp_usn="mock_usn", + ssdp_st="mock_st", + ssdp_location="http://127.0.0.2:60006/upnp/desc/aios_device/aios_device.xml", + upnp={ + ssdp.ATTR_UPNP_DEVICE_TYPE: "urn:schemas-denon-com:device:AiosDevice:1", + ssdp.ATTR_UPNP_FRIENDLY_NAME: "Bedroom", + ssdp.ATTR_UPNP_MANUFACTURER: "Denon", + ssdp.ATTR_UPNP_MODEL_NAME: "HEOS Drive", + ssdp.ATTR_UPNP_MODEL_NUMBER: "DWSA-10 4.0", + ssdp.ATTR_UPNP_SERIAL: None, + ssdp.ATTR_UPNP_UDN: "uuid:e61de70c-2250-1c22-0080-0005cdf512be", + }, + ) + + @pytest.fixture(name="quick_selects") def quick_selects_fixture() -> dict[int, str]: """Create a dict of quick selects for testing.""" diff --git a/tests/components/heos/test_config_flow.py b/tests/components/heos/test_config_flow.py index 7b737d7bb4b..464b62df157 100644 --- a/tests/components/heos/test_config_flow.py +++ b/tests/components/heos/test_config_flow.py @@ -1,14 +1,10 @@ """Tests for the Heos config flow module.""" -from unittest.mock import patch -from urllib.parse import urlparse - from pyheos import HeosError from homeassistant.components import heos, ssdp -from homeassistant.components.heos.config_flow import HeosFlowHandler -from homeassistant.components.heos.const import DATA_DISCOVERED_HOSTS, DOMAIN -from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_SSDP, SOURCE_USER +from homeassistant.components.heos.const import DOMAIN +from homeassistant.config_entries import SOURCE_SSDP, SOURCE_USER from homeassistant.const import CONF_HOST from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResultType @@ -17,18 +13,20 @@ from homeassistant.data_entry_flow import FlowResultType async def test_flow_aborts_already_setup(hass: HomeAssistant, config_entry) -> None: """Test flow aborts when entry already setup.""" config_entry.add_to_hass(hass) - flow = HeosFlowHandler() - flow.hass = hass - result = await flow.async_step_user() + + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_USER} + ) + assert result["type"] is FlowResultType.ABORT assert result["reason"] == "single_instance_allowed" async def test_no_host_shows_form(hass: HomeAssistant) -> None: """Test form is shown when host not provided.""" - flow = HeosFlowHandler() - flow.hass = hass - result = await flow.async_step_user() + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_USER} + ) assert result["type"] is FlowResultType.FORM assert result["step_id"] == "user" assert result["errors"] == {} @@ -45,73 +43,69 @@ async def test_cannot_connect_shows_error_form(hass: HomeAssistant, controller) assert result["errors"][CONF_HOST] == "cannot_connect" assert controller.connect.call_count == 1 assert controller.disconnect.call_count == 1 - controller.connect.reset_mock() - controller.disconnect.reset_mock() async def test_create_entry_when_host_valid(hass: HomeAssistant, controller) -> None: """Test result type is create entry when host is valid.""" data = {CONF_HOST: "127.0.0.1"} - with patch("homeassistant.components.heos.async_setup_entry", return_value=True): - result = await hass.config_entries.flow.async_init( - heos.DOMAIN, context={"source": SOURCE_USER}, data=data - ) - assert result["type"] is FlowResultType.CREATE_ENTRY - assert result["result"].unique_id == DOMAIN - assert result["title"] == "Controller (127.0.0.1)" - assert result["data"] == data - assert controller.connect.call_count == 1 - assert controller.disconnect.call_count == 1 + + result = await hass.config_entries.flow.async_init( + heos.DOMAIN, context={"source": SOURCE_USER}, data=data + ) + assert result["type"] is FlowResultType.CREATE_ENTRY + assert result["result"].unique_id == DOMAIN + assert result["title"] == "Controller (127.0.0.1)" + assert result["data"] == data + assert controller.connect.call_count == 2 # Also called in async_setup_entry + assert controller.disconnect.call_count == 1 async def test_create_entry_when_friendly_name_valid( hass: HomeAssistant, controller ) -> None: """Test result type is create entry when friendly name is valid.""" - hass.data[DATA_DISCOVERED_HOSTS] = {"Office (127.0.0.1)": "127.0.0.1"} + hass.data[DOMAIN] = {"Office (127.0.0.1)": "127.0.0.1"} data = {CONF_HOST: "Office (127.0.0.1)"} - with patch("homeassistant.components.heos.async_setup_entry", return_value=True): - result = await hass.config_entries.flow.async_init( - heos.DOMAIN, context={"source": SOURCE_USER}, data=data - ) - assert result["type"] is FlowResultType.CREATE_ENTRY - assert result["result"].unique_id == DOMAIN - assert result["title"] == "Controller (127.0.0.1)" - assert result["data"] == {CONF_HOST: "127.0.0.1"} - assert controller.connect.call_count == 1 - assert controller.disconnect.call_count == 1 - assert DATA_DISCOVERED_HOSTS not in hass.data + + result = await hass.config_entries.flow.async_init( + heos.DOMAIN, context={"source": SOURCE_USER}, data=data + ) + + assert result["type"] is FlowResultType.CREATE_ENTRY + assert result["result"].unique_id == DOMAIN + assert result["title"] == "Controller (127.0.0.1)" + assert result["data"] == {CONF_HOST: "127.0.0.1"} + assert controller.connect.call_count == 2 # Also called in async_setup_entry + assert controller.disconnect.call_count == 1 + assert DOMAIN not in hass.data async def test_discovery_shows_create_form( - hass: HomeAssistant, controller, discovery_data: ssdp.SsdpServiceInfo + hass: HomeAssistant, + controller, + discovery_data: ssdp.SsdpServiceInfo, + discovery_data_bedroom: ssdp.SsdpServiceInfo, ) -> None: - """Test discovery shows form to confirm setup and subsequent abort.""" + """Test discovery shows form to confirm setup.""" - await hass.config_entries.flow.async_init( + # Single discovered host shows form for user to finish setup. + result = await hass.config_entries.flow.async_init( heos.DOMAIN, context={"source": SOURCE_SSDP}, data=discovery_data ) - await hass.async_block_till_done() - flows_in_progress = hass.config_entries.flow.async_progress() - assert flows_in_progress[0]["context"]["unique_id"] == DOMAIN - assert len(flows_in_progress) == 1 - assert hass.data[DATA_DISCOVERED_HOSTS] == {"Office (127.0.0.1)": "127.0.0.1"} + assert hass.data[DOMAIN] == {"Office (127.0.0.1)": "127.0.0.1"} + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "user" - port = urlparse(discovery_data.ssdp_location).port - discovery_data.ssdp_location = f"http://127.0.0.2:{port}/" - discovery_data.upnp[ssdp.ATTR_UPNP_FRIENDLY_NAME] = "Bedroom" - - await hass.config_entries.flow.async_init( - heos.DOMAIN, context={"source": SOURCE_SSDP}, data=discovery_data + # Subsequent discovered hosts append to discovered hosts and abort. + result = await hass.config_entries.flow.async_init( + heos.DOMAIN, context={"source": SOURCE_SSDP}, data=discovery_data_bedroom ) - await hass.async_block_till_done() - flows_in_progress = hass.config_entries.flow.async_progress() - assert flows_in_progress[0]["context"]["unique_id"] == DOMAIN - assert len(flows_in_progress) == 1 - assert hass.data[DATA_DISCOVERED_HOSTS] == { + assert hass.data[DOMAIN] == { "Office (127.0.0.1)": "127.0.0.1", "Bedroom (127.0.0.2)": "127.0.0.2", } + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "already_in_progress" async def test_discovery_flow_aborts_already_setup( @@ -119,41 +113,10 @@ async def test_discovery_flow_aborts_already_setup( ) -> None: """Test discovery flow aborts when entry already setup.""" config_entry.add_to_hass(hass) - flow = HeosFlowHandler() - flow.hass = hass - result = await flow.async_step_ssdp(discovery_data) + + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_SSDP}, data=discovery_data + ) + assert result["type"] is FlowResultType.ABORT assert result["reason"] == "single_instance_allowed" - - -async def test_discovery_sets_the_unique_id( - hass: HomeAssistant, controller, discovery_data: ssdp.SsdpServiceInfo -) -> None: - """Test discovery sets the unique id.""" - - port = urlparse(discovery_data.ssdp_location).port - discovery_data.ssdp_location = f"http://127.0.0.2:{port}/" - discovery_data.upnp[ssdp.ATTR_UPNP_FRIENDLY_NAME] = "Bedroom" - - await hass.config_entries.flow.async_init( - heos.DOMAIN, context={"source": SOURCE_SSDP}, data=discovery_data - ) - await hass.async_block_till_done() - flows_in_progress = hass.config_entries.flow.async_progress() - assert flows_in_progress[0]["context"]["unique_id"] == DOMAIN - assert len(flows_in_progress) == 1 - assert hass.data[DATA_DISCOVERED_HOSTS] == {"Bedroom (127.0.0.2)": "127.0.0.2"} - - -async def test_import_sets_the_unique_id(hass: HomeAssistant, controller) -> None: - """Test import sets the unique id.""" - - with patch("homeassistant.components.heos.async_setup_entry", return_value=True): - result = await hass.config_entries.flow.async_init( - heos.DOMAIN, - context={"source": SOURCE_IMPORT}, - data={CONF_HOST: "127.0.0.2"}, - ) - await hass.async_block_till_done() - assert result["type"] is FlowResultType.CREATE_ENTRY - assert result["result"].unique_id == DOMAIN diff --git a/tests/components/heos/test_init.py b/tests/components/heos/test_init.py index 04b745135d4..8d2e3b68a22 100644 --- a/tests/components/heos/test_init.py +++ b/tests/components/heos/test_init.py @@ -13,40 +13,11 @@ from homeassistant.components.heos import ( async_unload_entry, ) from homeassistant.components.heos.const import DOMAIN -from homeassistant.const import CONF_HOST from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.setup import async_setup_component -async def test_async_setup_creates_entry(hass: HomeAssistant, config) -> None: - """Test component setup creates entry from config.""" - assert await async_setup_component(hass, DOMAIN, config) - await hass.async_block_till_done() - entries = hass.config_entries.async_entries(DOMAIN) - assert len(entries) == 1 - entry = entries[0] - assert entry.title == "Controller (127.0.0.1)" - assert entry.data == {CONF_HOST: "127.0.0.1"} - assert entry.unique_id == DOMAIN - - -async def test_async_setup_updates_entry( - hass: HomeAssistant, config_entry, config, controller -) -> None: - """Test component setup updates entry from config.""" - config[DOMAIN][CONF_HOST] = "127.0.0.2" - config_entry.add_to_hass(hass) - assert await async_setup_component(hass, DOMAIN, config) - await hass.async_block_till_done() - entries = hass.config_entries.async_entries(DOMAIN) - assert len(entries) == 1 - entry = entries[0] - assert entry.title == "Controller (127.0.0.2)" - assert entry.data == {CONF_HOST: "127.0.0.2"} - assert entry.unique_id == DOMAIN - - async def test_async_setup_returns_true( hass: HomeAssistant, config_entry, config ) -> None: