From 9a5324075987ed599ae26ef4bfc247d02919ee2f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 26 May 2020 10:51:50 -0500 Subject: [PATCH] Add ability to ignore heos discovery (#34653) * Add ability to ignore heos discovery * Fetch player_id, update tests * Handle failure state * Update tests as there are two players in the mock now * Adjust and add more tests * Strip out player id lookup * reverts per review * one more revert --- homeassistant/components/heos/__init__.py | 4 ++ homeassistant/components/heos/config_flow.py | 4 ++ tests/components/heos/test_config_flow.py | 48 ++++++++++++++++++-- tests/components/heos/test_init.py | 2 + 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/heos/__init__.py b/homeassistant/components/heos/__init__.py index 53c65a6ab07bbb..a76a29b2ed537a 100644 --- a/homeassistant/components/heos/__init__.py +++ b/homeassistant/components/heos/__init__.py @@ -61,6 +61,10 @@ async def async_setup(hass: HomeAssistantType, config: ConfigType): async def async_setup_entry(hass: HomeAssistantType, entry: ConfigEntry): """Initialize config entry which represents the HEOS controller.""" + # For backwards compat + if entry.unique_id is None: + hass.config_entries.async_update_entry(entry, unique_id=DOMAIN) + host = entry.data[CONF_HOST] # Setting all_progress_events=False ensures that we only receive a # media position update upon start of playback or when media changes diff --git a/homeassistant/components/heos/config_flow.py b/homeassistant/components/heos/config_flow.py index 91dbc19ac952c1..138d1c4462c7b5 100644 --- a/homeassistant/components/heos/config_flow.py +++ b/homeassistant/components/heos/config_flow.py @@ -33,12 +33,16 @@ async def async_step_ssdp(self, discovery_info): # 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="already_setup") + await self.async_set_unique_id(DOMAIN) # Show selection form return self.async_show_form(step_id="user") async def async_step_import(self, user_input=None): """Occurs when an entry is setup through config.""" host = user_input[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=None): diff --git a/tests/components/heos/test_config_flow.py b/tests/components/heos/test_config_flow.py index d90c426324056f..800814df0130d3 100644 --- a/tests/components/heos/test_config_flow.py +++ b/tests/components/heos/test_config_flow.py @@ -6,7 +6,8 @@ from homeassistant import data_entry_flow from homeassistant.components import heos, ssdp from homeassistant.components.heos.config_flow import HeosFlowHandler -from homeassistant.components.heos.const import DATA_DISCOVERED_HOSTS +from homeassistant.components.heos.const import DATA_DISCOVERED_HOSTS, DOMAIN +from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_SSDP from homeassistant.const import CONF_HOST from tests.async_mock import patch @@ -55,6 +56,7 @@ async def test_create_entry_when_host_valid(hass, controller): heos.DOMAIN, context={"source": "user"}, data=data ) assert result["type"] == data_entry_flow.RESULT_TYPE_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 @@ -70,6 +72,7 @@ async def test_create_entry_when_friendly_name_valid(hass, controller): heos.DOMAIN, context={"source": "user"}, data=data ) assert result["type"] == data_entry_flow.RESULT_TYPE_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 @@ -79,28 +82,34 @@ async def test_create_entry_when_friendly_name_valid(hass, controller): async def test_discovery_shows_create_form(hass, controller, discovery_data): """Test discovery shows form to confirm setup and subsequent abort.""" + await hass.config_entries.flow.async_init( heos.DOMAIN, context={"source": "ssdp"}, data=discovery_data ) await hass.async_block_till_done() - assert len(hass.config_entries.flow.async_progress()) == 1 + 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"} port = urlparse(discovery_data[ssdp.ATTR_SSDP_LOCATION]).port discovery_data[ssdp.ATTR_SSDP_LOCATION] = f"http://127.0.0.2:{port}/" discovery_data[ssdp.ATTR_UPNP_FRIENDLY_NAME] = "Bedroom" + await hass.config_entries.flow.async_init( heos.DOMAIN, context={"source": "ssdp"}, data=discovery_data ) await hass.async_block_till_done() - assert len(hass.config_entries.flow.async_progress()) == 1 + 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", "Bedroom (127.0.0.2)": "127.0.0.2", } -async def test_disovery_flow_aborts_already_setup( +async def test_discovery_flow_aborts_already_setup( hass, controller, discovery_data, config_entry ): """Test discovery flow aborts when entry already setup.""" @@ -110,3 +119,34 @@ async def test_disovery_flow_aborts_already_setup( result = await flow.async_step_ssdp(discovery_data) assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT assert result["reason"] == "already_setup" + + +async def test_discovery_sets_the_unique_id(hass, controller, discovery_data): + """Test discovery sets the unique id.""" + + port = urlparse(discovery_data[ssdp.ATTR_SSDP_LOCATION]).port + discovery_data[ssdp.ATTR_SSDP_LOCATION] = f"http://127.0.0.2:{port}/" + discovery_data[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, controller): + """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"] == data_entry_flow.RESULT_TYPE_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 a6852e3db41890..a32ea5dd08ca7a 100644 --- a/tests/components/heos/test_init.py +++ b/tests/components/heos/test_init.py @@ -31,6 +31,7 @@ async def test_async_setup_creates_entry(hass, config): 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, config_entry, config, controller): @@ -44,6 +45,7 @@ async def test_async_setup_updates_entry(hass, config_entry, config, controller) 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, config_entry, config):