From 2db278a7a7ef7042d95fa4af08c159079f81b195 Mon Sep 17 00:00:00 2001 From: Raj Laud <50647620+rajlaud@users.noreply.github.com> Date: Fri, 6 Aug 2021 12:29:52 -0400 Subject: [PATCH] Fix Squeezebox dhcp discovery (#54137) * Fix Squeezebox dhcp discovery and allow ignore * Test ignoring known Squeezebox players * Fix linter errors --- .../components/squeezebox/browse_media.py | 1 - .../components/squeezebox/config_flow.py | 45 ++++++++++++------- .../components/squeezebox/media_player.py | 9 ++-- .../components/squeezebox/test_config_flow.py | 38 ++++++++++++---- 4 files changed, 62 insertions(+), 31 deletions(-) diff --git a/homeassistant/components/squeezebox/browse_media.py b/homeassistant/components/squeezebox/browse_media.py index 4c0ec186707..294a1105a71 100644 --- a/homeassistant/components/squeezebox/browse_media.py +++ b/homeassistant/components/squeezebox/browse_media.py @@ -137,7 +137,6 @@ async def build_item_response(entity, player, payload): async def library_payload(player): """Create response payload to describe contents of library.""" - library_info = { "title": "Music Library", "media_class": MEDIA_CLASS_DIRECTORY, diff --git a/homeassistant/components/squeezebox/config_flow.py b/homeassistant/components/squeezebox/config_flow.py index 1f1c23942db..4b05588e281 100644 --- a/homeassistant/components/squeezebox/config_flow.py +++ b/homeassistant/components/squeezebox/config_flow.py @@ -5,8 +5,9 @@ import logging from pysqueezebox import Server, async_discover import voluptuous as vol -from homeassistant import config_entries -from homeassistant.components.dhcp import IP_ADDRESS, MAC_ADDRESS +from homeassistant import config_entries, data_entry_flow +from homeassistant.components.dhcp import MAC_ADDRESS +from homeassistant.components.media_player import DOMAIN as MP_DOMAIN from homeassistant.const import ( CONF_HOST, CONF_PASSWORD, @@ -15,6 +16,8 @@ from homeassistant.const import ( HTTP_UNAUTHORIZED, ) from homeassistant.helpers.aiohttp_client import async_get_clientsession +from homeassistant.helpers.device_registry import format_mac +from homeassistant.helpers.entity_registry import async_get from .const import DEFAULT_PORT, DOMAIN @@ -166,28 +169,18 @@ class SqueezeboxConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): return self.async_abort(reason=error) return self.async_create_entry(title=config[CONF_HOST], data=config) - async def async_step_discovery(self, discovery_info): - """Handle discovery.""" - _LOGGER.debug("Reached discovery flow with info: %s", discovery_info) + async def async_step_integration_discovery(self, discovery_info): + """Handle discovery of a server.""" + _LOGGER.debug("Reached server discovery flow with info: %s", discovery_info) if "uuid" in discovery_info: await self.async_set_unique_id(discovery_info.pop("uuid")) self._abort_if_unique_id_configured() else: # attempt to connect to server and determine uuid. will fail if # password required - - if CONF_HOST not in discovery_info and IP_ADDRESS in discovery_info: - discovery_info[CONF_HOST] = discovery_info[IP_ADDRESS] - - if CONF_PORT not in discovery_info: - discovery_info[CONF_PORT] = DEFAULT_PORT - error = await self._validate_input(discovery_info) if error: - if MAC_ADDRESS in discovery_info: - await self.async_set_unique_id(discovery_info[MAC_ADDRESS]) - else: - await self._async_handle_discovery_without_unique_id() + await self._async_handle_discovery_without_unique_id() # update schema with suggested values from discovery self.data_schema = _base_schema(discovery_info) @@ -195,3 +188,23 @@ class SqueezeboxConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): self.context.update({"title_placeholders": {"host": discovery_info[CONF_HOST]}}) return await self.async_step_edit() + + async def async_step_dhcp(self, discovery_info): + """Handle dhcp discovery of a Squeezebox player.""" + _LOGGER.debug( + "Reached dhcp discovery of a player with info: %s", discovery_info + ) + await self.async_set_unique_id(format_mac(discovery_info[MAC_ADDRESS])) + self._abort_if_unique_id_configured() + + _LOGGER.debug("Configuring dhcp player with unique id: %s", self.unique_id) + + registry = async_get(self.hass) + + # if we have detected this player, do nothing. if not, there must be a server out there for us to configure, so start the normal user flow (which tries to autodetect server) + if registry.async_get_entity_id(MP_DOMAIN, DOMAIN, self.unique_id) is not None: + # this player is already known, so do nothing other than mark as configured + raise data_entry_flow.AbortFlow("already_configured") + + # if the player is unknown, then we likely need to configure its server + return await self.async_step_user() diff --git a/homeassistant/components/squeezebox/media_player.py b/homeassistant/components/squeezebox/media_player.py index baf8a011c65..1ba406097d7 100644 --- a/homeassistant/components/squeezebox/media_player.py +++ b/homeassistant/components/squeezebox/media_player.py @@ -27,7 +27,7 @@ from homeassistant.components.media_player.const import ( SUPPORT_VOLUME_MUTE, SUPPORT_VOLUME_SET, ) -from homeassistant.config_entries import SOURCE_DISCOVERY +from homeassistant.config_entries import SOURCE_INTEGRATION_DISCOVERY from homeassistant.const import ( ATTR_COMMAND, CONF_HOST, @@ -43,6 +43,7 @@ from homeassistant.const import ( from homeassistant.core import callback from homeassistant.helpers import config_validation as cv, entity_platform from homeassistant.helpers.aiohttp_client import async_get_clientsession +from homeassistant.helpers.device_registry import format_mac from homeassistant.helpers.dispatcher import ( async_dispatcher_connect, async_dispatcher_send, @@ -127,7 +128,7 @@ async def start_server_discovery(hass): asyncio.create_task( hass.config_entries.flow.async_init( DOMAIN, - context={"source": SOURCE_DISCOVERY}, + context={"source": SOURCE_INTEGRATION_DISCOVERY}, data={ CONF_HOST: server.host, CONF_PORT: int(server.port), @@ -146,7 +147,6 @@ async def start_server_discovery(hass): async def async_setup_platform(hass, config, async_add_entities, discovery_info=None): """Set up squeezebox platform from platform entry in configuration.yaml (deprecated).""" - if config: await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, data=config @@ -283,7 +283,7 @@ class SqueezeBoxEntity(MediaPlayerEntity): @property def unique_id(self): """Return a unique ID.""" - return self._player.player_id + return format_mac(self._player.player_id) @property def available(self): @@ -573,7 +573,6 @@ class SqueezeBoxEntity(MediaPlayerEntity): async def async_browse_media(self, media_content_type=None, media_content_id=None): """Implement the websocket media browsing helper.""" - _LOGGER.debug( "Reached async_browse_media with content_type %s and content_id %s", media_content_type, diff --git a/tests/components/squeezebox/test_config_flow.py b/tests/components/squeezebox/test_config_flow.py index e740ea671cd..9460e2235be 100644 --- a/tests/components/squeezebox/test_config_flow.py +++ b/tests/components/squeezebox/test_config_flow.py @@ -178,7 +178,7 @@ async def test_discovery(hass): ): result = await hass.config_entries.flow.async_init( DOMAIN, - context={"source": config_entries.SOURCE_DISCOVERY}, + context={"source": config_entries.SOURCE_INTEGRATION_DISCOVERY}, data={CONF_HOST: HOST, CONF_PORT: PORT, "uuid": UUID}, ) assert result["type"] == RESULT_TYPE_FORM @@ -190,7 +190,7 @@ async def test_discovery_no_uuid(hass): with patch("pysqueezebox.Server.async_query", new=patch_async_query_unauthorized): result = await hass.config_entries.flow.async_init( DOMAIN, - context={"source": config_entries.SOURCE_DISCOVERY}, + context={"source": config_entries.SOURCE_INTEGRATION_DISCOVERY}, data={CONF_HOST: HOST, CONF_PORT: PORT}, ) assert result["type"] == RESULT_TYPE_FORM @@ -199,9 +199,8 @@ async def test_discovery_no_uuid(hass): async def test_dhcp_discovery(hass): """Test we can process discovery from dhcp.""" - with patch( - "pysqueezebox.Server.async_query", - return_value={"uuid": UUID}, + with patch("pysqueezebox.Server.async_query", return_value={"uuid": UUID},), patch( + "homeassistant.components.squeezebox.config_flow.async_discover", mock_discover ): result = await hass.config_entries.flow.async_init( DOMAIN, @@ -216,9 +215,12 @@ async def test_dhcp_discovery(hass): assert result["step_id"] == "edit" -async def test_dhcp_discovery_no_connection(hass): - """Test we can process discovery from dhcp without connecting to squeezebox server.""" - with patch("pysqueezebox.Server.async_query", new=patch_async_query_unauthorized): +async def test_dhcp_discovery_no_server_found(hass): + """Test we can handle dhcp discovery when no server is found.""" + with patch( + "homeassistant.components.squeezebox.config_flow.async_discover", + mock_failed_discover, + ): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_DHCP}, @@ -229,7 +231,25 @@ async def test_dhcp_discovery_no_connection(hass): }, ) assert result["type"] == RESULT_TYPE_FORM - assert result["step_id"] == "edit" + assert result["step_id"] == "user" + + +async def test_dhcp_discovery_existing_player(hass): + """Test that we properly ignore known players during dhcp discover.""" + with patch( + "homeassistant.helpers.entity_registry.EntityRegistry.async_get_entity_id", + return_value="test_entity", + ): + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_DHCP}, + data={ + IP_ADDRESS: "1.1.1.1", + MAC_ADDRESS: "AA:BB:CC:DD:EE:FF", + HOSTNAME: "any", + }, + ) + assert result["type"] == RESULT_TYPE_ABORT async def test_import(hass):