From 1f4ec26eeeb165abd9aa8869a346b764d01d1b07 Mon Sep 17 00:00:00 2001 From: Barrett Lowe Date: Sat, 1 May 2021 15:06:35 +0000 Subject: [PATCH 01/20] initial stab at snapcast config flow --- .coveragerc | 1 - homeassistant/components/snapcast/__init__.py | 39 +++++++++- .../components/snapcast/config_flow.py | 72 +++++++++++++++++++ homeassistant/components/snapcast/const.py | 4 ++ .../components/snapcast/manifest.json | 17 +++-- .../components/snapcast/media_player.py | 65 ++++++++++++++--- .../components/snapcast/strings.json | 18 +++++ .../components/snapcast/translations/en.json | 18 +++++ homeassistant/generated/config_flows.py | 1 + requirements_test_all.txt | 3 + tests/components/snapcast/__init__.py | 10 +++ tests/components/snapcast/test_init.py | 61 ++++++++++++++++ 12 files changed, 289 insertions(+), 20 deletions(-) create mode 100644 homeassistant/components/snapcast/config_flow.py create mode 100644 homeassistant/components/snapcast/strings.json create mode 100644 homeassistant/components/snapcast/translations/en.json create mode 100644 tests/components/snapcast/__init__.py create mode 100644 tests/components/snapcast/test_init.py diff --git a/.coveragerc b/.coveragerc index 9c030123f720e2..723d0744b1da19 100644 --- a/.coveragerc +++ b/.coveragerc @@ -921,7 +921,6 @@ omit = homeassistant/components/smarthab/light.py homeassistant/components/sms/* homeassistant/components/smtp/notify.py - homeassistant/components/snapcast/* homeassistant/components/snmp/* homeassistant/components/sochain/sensor.py homeassistant/components/solaredge/__init__.py diff --git a/homeassistant/components/snapcast/__init__.py b/homeassistant/components/snapcast/__init__.py index b5279fa3ce06c2..069e50a6ead944 100644 --- a/homeassistant/components/snapcast/__init__.py +++ b/homeassistant/components/snapcast/__init__.py @@ -1 +1,38 @@ -"""The snapcast component.""" +"""Snapcast Integration.""" +import logging +import socket + +import snapcast.control + +from homeassistant.config_entries import ConfigEntry +from homeassistant.core import HomeAssistant + +from .const import DOMAIN + +_LOGGER = logging.getLogger(__name__) + + +async def async_setup(hass: HomeAssistant, config: dict): + """Set up the Snapcast component.""" + hass.data.setdefault(DOMAIN, {}) + return True + + +async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: + """Set up Snapcast from a config entry.""" + + host = entry.data["host"] + port = entry.data["port"] + try: + hass.data[DOMAIN][entry.entry_id] = await snapcast.control.create_server( + hass.loop, host, port, reconnect=True + ) + except socket.gaierror: + _LOGGER.error("Could not connect to Snapcast server at %s:%d", host, port) + return False + + hass.async_create_task( + hass.config_entries.async_forward_entry_setup(entry, "media_player") + ) + + return True diff --git a/homeassistant/components/snapcast/config_flow.py b/homeassistant/components/snapcast/config_flow.py new file mode 100644 index 00000000000000..809c0e304afa5a --- /dev/null +++ b/homeassistant/components/snapcast/config_flow.py @@ -0,0 +1,72 @@ +"""Snapcast config flow.""" + +from __future__ import annotations + +import logging +import socket + +import snapcast.control +import voluptuous as vol + +from homeassistant.config_entries import ConfigFlow +from homeassistant.const import CONF_HOST, CONF_PORT + +from .const import DEFAULT_PORT, DOMAIN + +_LOGGER = logging.getLogger(__name__) + +SNAPCAST_SCHEMA = vol.Schema( + { + vol.Required(CONF_HOST): str, + vol.Required(CONF_PORT, default=DEFAULT_PORT): int, + } +) + + +class SnapcastConfigFlow(ConfigFlow, domain=DOMAIN): + """Snapcast config flow.""" + + async def async_step_user(self, user_input=None): + """Handle first step.""" + + def _show_form(errors={}): + return self.async_show_form( + step_id="user", + data_schema=SNAPCAST_SCHEMA, + errors=errors, + ) + + if not user_input: + return _show_form() + + host = user_input[CONF_HOST] + port = user_input[CONF_PORT] + errors = {} + + # Attempt to create the server - make sure it's going to work + try: + client = await snapcast.control.create_server( + self.hass.loop, host, port, reconnect=True + ) + except socket.gaierror: + errors["base"] = "unknown" + except ConnectionRefusedError: + errors["base"] = "cannot_connect" + finally: + if "client" in locals(): + del client + + await self.async_set_unique_id("Snapcast") + self._abort_if_unique_id_configured() + + if errors: + _LOGGER.error( + "Could not connect to a Snapcast Server at the provided address" + ) + _LOGGER.error(f"Error: {errors['base']}") + return _show_form(errors=errors) + + return self.async_create_entry( + title=f"{host}:{port}", + data=user_input, + ) diff --git a/homeassistant/components/snapcast/const.py b/homeassistant/components/snapcast/const.py index 674a22993b910c..0a0edcce2dd044 100644 --- a/homeassistant/components/snapcast/const.py +++ b/homeassistant/components/snapcast/const.py @@ -15,3 +15,7 @@ ATTR_MASTER = "master" ATTR_LATENCY = "latency" + +DEFAULT_PORT = 1705 + +DOMAIN = "snapcast" diff --git a/homeassistant/components/snapcast/manifest.json b/homeassistant/components/snapcast/manifest.json index 2e3249f4551d74..9be1e121d287f4 100644 --- a/homeassistant/components/snapcast/manifest.json +++ b/homeassistant/components/snapcast/manifest.json @@ -1,8 +1,11 @@ { - "domain": "snapcast", - "name": "Snapcast", - "documentation": "https://www.home-assistant.io/integrations/snapcast", - "requirements": ["snapcast==2.1.3"], - "codeowners": [], - "iot_class": "local_polling" -} + "domain": "snapcast", + "name": "Snapcast", + "documentation": "https://www.home-assistant.io/integrations/snapcast", + "requirements": [ + "snapcast==2.1.3" + ], + "codeowners": [], + "iot_class": "local_polling", + "config_flow": true +} \ No newline at end of file diff --git a/homeassistant/components/snapcast/media_player.py b/homeassistant/components/snapcast/media_player.py index e1c5b7d875b61a..82ef8d509b1151 100644 --- a/homeassistant/components/snapcast/media_player.py +++ b/homeassistant/components/snapcast/media_player.py @@ -1,26 +1,29 @@ """Support for interacting with Snapcast clients.""" import logging import socket +from typing import Callable import snapcast.control -from snapcast.control.server import CONTROL_PORT +from snapcast.control.server import CONTROL_PORT, Snapserver import voluptuous as vol from homeassistant.components.media_player import PLATFORM_SCHEMA, MediaPlayerEntity from homeassistant.components.media_player.const import ( + SUPPORT_GROUPING, SUPPORT_SELECT_SOURCE, SUPPORT_VOLUME_MUTE, SUPPORT_VOLUME_SET, ) +from homeassistant.config_entries import ConfigEntry from homeassistant.const import ( CONF_HOST, CONF_PORT, STATE_IDLE, STATE_OFF, - STATE_ON, STATE_PLAYING, STATE_UNKNOWN, ) +from homeassistant.core import HomeAssistant from homeassistant.helpers import config_validation as cv, entity_platform from .const import ( @@ -29,6 +32,7 @@ CLIENT_PREFIX, CLIENT_SUFFIX, DATA_KEY, + DOMAIN, GROUP_PREFIX, GROUP_SUFFIX, SERVICE_JOIN, @@ -44,7 +48,7 @@ SUPPORT_VOLUME_MUTE | SUPPORT_VOLUME_SET | SUPPORT_SELECT_SOURCE ) SUPPORT_SNAPCAST_GROUP = ( - SUPPORT_VOLUME_MUTE | SUPPORT_VOLUME_SET | SUPPORT_SELECT_SOURCE + SUPPORT_VOLUME_MUTE | SUPPORT_VOLUME_SET | SUPPORT_SELECT_SOURCE | SUPPORT_GROUPING ) PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( @@ -52,12 +56,8 @@ ) -async def async_setup_platform(hass, config, async_add_entities, discovery_info=None): - """Set up the Snapcast platform.""" - - host = config.get(CONF_HOST) - port = config.get(CONF_PORT, CONTROL_PORT) - +def register_services(): + """Register snapcast services.""" platform = entity_platform.current_platform.get() platform.async_register_entity_service(SERVICE_SNAPSHOT, {}, "snapshot") platform.async_register_entity_service(SERVICE_RESTORE, {}, "async_restore") @@ -71,6 +71,37 @@ async def async_setup_platform(hass, config, async_add_entities, discovery_info= handle_set_latency, ) + +async def async_setup_entry( + hass: HomeAssistant, config_entry: ConfigEntry, async_add_entities: Callable +) -> bool: + """Set up the snapcast config entry.""" + theServer: Snapserver = hass.data[DOMAIN][config_entry.entry_id] + _LOGGER.debug(theServer) + + register_services() + + host = config_entry.data["host"] + port = config_entry.data["port"] + hpid = f"{host}:{port}" + + groups = [SnapcastGroupDevice(group, hpid) for group in theServer.groups] + clients = [SnapcastClientDevice(client, hpid) for client in theServer.clients] + devices = groups + clients + hass.data[DATA_KEY] = devices + async_add_entities(devices) + + return False + + +async def async_setup_platform(hass, config, async_add_entities, discovery_info=None): + """Set up the Snapcast platform.""" + + host = config.get(CONF_HOST) + port = config.get(CONF_PORT, CONTROL_PORT) + + register_services() + try: server = await snapcast.control.create_server( hass.loop, host, port, reconnect=True @@ -174,6 +205,11 @@ def should_poll(self): """Do not poll for state.""" return False + @property + def group_members(self): + """Get the members of the group.""" + return self._group.clients + async def async_select_source(self, source): """Set input source.""" streams = self._group.streams_by_name() @@ -231,7 +267,9 @@ def name(self): @property def source(self): """Return the current input source.""" - return self._client.group.stream + stream = self._client.group.stream + _LOGGER.debug(f"Client source queried - {self._client.identifier}:{stream}") + return stream @property def volume_level(self): @@ -257,7 +295,11 @@ def source_list(self): def state(self): """Return the state of the player.""" if self._client.connected: - return STATE_ON + return { + "idle": STATE_IDLE, + "playing": STATE_PLAYING, + "unknown": STATE_UNKNOWN, + }.get(self._client.group.stream_status, STATE_UNKNOWN) return STATE_OFF @property @@ -311,6 +353,7 @@ async def async_join(self, master): for group in self._client.groups_available() if master_entity.identifier in group.clients ) + _LOGGER.debug(f"master group: {master_group}") await master_group.add_client(self._client.identifier) self.async_write_ha_state() diff --git a/homeassistant/components/snapcast/strings.json b/homeassistant/components/snapcast/strings.json new file mode 100644 index 00000000000000..55c50bb67ee013 --- /dev/null +++ b/homeassistant/components/snapcast/strings.json @@ -0,0 +1,18 @@ +{ + "config": { + "step": { + "user": { + "description": "Please enter your server connection details", + "data": { + "host": "[%key:common::config_flow::data::host%]", + "port": "[%key:common::config_flow::data::port%]" + }, + "title": "Connect" + } + }, + "error": { + "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", + "unknown": "[%key:common::config_flow::error::unknown%]" + } + } +} \ No newline at end of file diff --git a/homeassistant/components/snapcast/translations/en.json b/homeassistant/components/snapcast/translations/en.json new file mode 100644 index 00000000000000..f6343f2f749c29 --- /dev/null +++ b/homeassistant/components/snapcast/translations/en.json @@ -0,0 +1,18 @@ +{ + "config": { + "error": { + "cannot_connect": "Failed to connect", + "unknown": "Unexpected error" + }, + "step": { + "user": { + "data": { + "host": "Host", + "port": "Port" + }, + "description": "Please enter your server connection details", + "title": "Connect" + } + } + } +} \ No newline at end of file diff --git a/homeassistant/generated/config_flows.py b/homeassistant/generated/config_flows.py index 3b408860d59a2c..054065cd2bd80d 100644 --- a/homeassistant/generated/config_flows.py +++ b/homeassistant/generated/config_flows.py @@ -222,6 +222,7 @@ "smarttub", "smhi", "sms", + "snapcast", "solaredge", "solarlog", "soma", diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 7d8c83396d6e1a..143d5606475e5c 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -1118,6 +1118,9 @@ smarthab==0.21 # homeassistant.components.smhi smhi-pkg==1.0.13 +# homeassistant.components.snapcast +snapcast==2.1.3 + # homeassistant.components.solaredge solaredge==0.0.2 diff --git a/tests/components/snapcast/__init__.py b/tests/components/snapcast/__init__.py new file mode 100644 index 00000000000000..bd4dabbdcd2490 --- /dev/null +++ b/tests/components/snapcast/__init__.py @@ -0,0 +1,10 @@ +"""Tests for the Snapcast integration.""" + +from unittest.mock import AsyncMock + + +def create_mock_snapcast() -> AsyncMock: + """Create mock snapcast connection.""" + mock_connection = AsyncMock() + mock_connection.start = AsyncMock(return_value=None) + return mock_connection diff --git a/tests/components/snapcast/test_init.py b/tests/components/snapcast/test_init.py new file mode 100644 index 00000000000000..e816414dc5adad --- /dev/null +++ b/tests/components/snapcast/test_init.py @@ -0,0 +1,61 @@ +"""Test the Snapcast module.""" + +import socket +from unittest.mock import patch + +from homeassistant import config_entries, setup +from homeassistant.components.snapcast.const import DOMAIN +from homeassistant.const import CONF_HOST, CONF_PORT +from homeassistant.core import HomeAssistant + +from . import create_mock_snapcast + +TEST_CONNECTION = {CONF_HOST: "snapserver.test", CONF_PORT: 1705} + + +async def test_success(hass: HomeAssistant) -> None: + """Test successful connection.""" + await setup.async_setup_component(hass, "persistent_notification", {}) + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] == "form" + assert not result["errors"] + + mock_connection = create_mock_snapcast() + + with patch( + "snapcast.control.create_server", + return_value=mock_connection, + ) as mock_setup_entry: + result = await hass.config_entries.flow.async_configure( + result["flow_id"], TEST_CONNECTION + ) + await hass.async_block_till_done() + + assert result["type"] == "create_entry" + assert ( + result["title"] == f"{TEST_CONNECTION[CONF_HOST]}:{TEST_CONNECTION[CONF_PORT]}" + ) + assert result["data"] == TEST_CONNECTION + assert len(mock_setup_entry.mock_calls) == 2 + + +async def test_error(hass: HomeAssistant) -> None: + """Test what happens when there is no server to connect.""" + await setup.async_setup_component(hass, "persistent_notification", {}) + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] == "form" + assert not result["errors"] + + with patch("snapcast.control.create_server", side_effect=socket.gaierror): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + TEST_CONNECTION, + ) + await hass.async_block_till_done() + + assert result["type"] == "form" + assert result["errors"] == {"base": "unknown_error"} From 878c4eab44e37d8fe7a08b86da9ca411f02ca6e7 Mon Sep 17 00:00:00 2001 From: Barrett Lowe Date: Sat, 1 May 2021 17:06:49 +0000 Subject: [PATCH 02/20] fix linting errors --- homeassistant/components/snapcast/__init__.py | 1 - .../components/snapcast/config_flow.py | 4 +-- .../components/snapcast/media_player.py | 33 ++++++++++--------- tests/components/snapcast/test_init.py | 24 ++++++++++++-- 4 files changed, 41 insertions(+), 21 deletions(-) diff --git a/homeassistant/components/snapcast/__init__.py b/homeassistant/components/snapcast/__init__.py index 069e50a6ead944..85dd02ee54ac72 100644 --- a/homeassistant/components/snapcast/__init__.py +++ b/homeassistant/components/snapcast/__init__.py @@ -20,7 +20,6 @@ async def async_setup(hass: HomeAssistant, config: dict): async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up Snapcast from a config entry.""" - host = entry.data["host"] port = entry.data["port"] try: diff --git a/homeassistant/components/snapcast/config_flow.py b/homeassistant/components/snapcast/config_flow.py index 809c0e304afa5a..48ec75688c034b 100644 --- a/homeassistant/components/snapcast/config_flow.py +++ b/homeassistant/components/snapcast/config_flow.py @@ -29,7 +29,7 @@ class SnapcastConfigFlow(ConfigFlow, domain=DOMAIN): async def async_step_user(self, user_input=None): """Handle first step.""" - def _show_form(errors={}): + def _show_form(errors=None): return self.async_show_form( step_id="user", data_schema=SNAPCAST_SCHEMA, @@ -63,7 +63,7 @@ def _show_form(errors={}): _LOGGER.error( "Could not connect to a Snapcast Server at the provided address" ) - _LOGGER.error(f"Error: {errors['base']}") + _LOGGER.error("Error: %s", (errors["base"])) return _show_form(errors=errors) return self.async_create_entry( diff --git a/homeassistant/components/snapcast/media_player.py b/homeassistant/components/snapcast/media_player.py index 82ef8d509b1151..6866c1ceca2305 100644 --- a/homeassistant/components/snapcast/media_player.py +++ b/homeassistant/components/snapcast/media_player.py @@ -3,7 +3,7 @@ import socket from typing import Callable -import snapcast.control +import snapcast from snapcast.control.server import CONTROL_PORT, Snapserver import voluptuous as vol @@ -76,8 +76,8 @@ async def async_setup_entry( hass: HomeAssistant, config_entry: ConfigEntry, async_add_entities: Callable ) -> bool: """Set up the snapcast config entry.""" - theServer: Snapserver = hass.data[DOMAIN][config_entry.entry_id] - _LOGGER.debug(theServer) + the_server: Snapserver = hass.data[DOMAIN][config_entry.entry_id] + _LOGGER.debug(the_server) register_services() @@ -85,18 +85,18 @@ async def async_setup_entry( port = config_entry.data["port"] hpid = f"{host}:{port}" - groups = [SnapcastGroupDevice(group, hpid) for group in theServer.groups] - clients = [SnapcastClientDevice(client, hpid) for client in theServer.clients] - devices = groups + clients - hass.data[DATA_KEY] = devices - async_add_entities(devices) + groups = [SnapcastGroupDevice(group, hpid) for group in the_server.groups] + clients = [SnapcastClientDevice(client, hpid) for client in the_server.clients] + hass.data[DATA_KEY]["clients"] = clients + hass.data[DATA_KEY]["groups"] = groups + async_add_entities(clients) + async_add_entities(groups) return False async def async_setup_platform(hass, config, async_add_entities, discovery_info=None): """Set up the Snapcast platform.""" - host = config.get(CONF_HOST) port = config.get(CONF_PORT, CONTROL_PORT) @@ -115,9 +115,10 @@ async def async_setup_platform(hass, config, async_add_entities, discovery_info= groups = [SnapcastGroupDevice(group, hpid) for group in server.groups] clients = [SnapcastClientDevice(client, hpid) for client in server.clients] - devices = groups + clients - hass.data[DATA_KEY] = devices - async_add_entities(devices) + hass.data[DATA_KEY]["clients"] = clients + hass.data[DATA_KEY]["groups"] = groups + async_add_entities(clients) + async_add_entities(groups) async def handle_async_join(entity, service_call): @@ -268,7 +269,7 @@ def name(self): def source(self): """Return the current input source.""" stream = self._client.group.stream - _LOGGER.debug(f"Client source queried - {self._client.identifier}:{stream}") + _LOGGER.debug("Client source queried - %s:%s", self._client.identifier, stream) return stream @property @@ -341,9 +342,10 @@ async def async_set_volume_level(self, volume): async def async_join(self, master): """Join the group of the master player.""" - master_entity = next( - entity for entity in self.hass.data[DATA_KEY] if entity.entity_id == master + entity + for entity in self.hass.data[DATA_KEY]["clients"] + if entity.entity_id == master ) if not isinstance(master_entity, SnapcastClientDevice): raise ValueError("Master is not a client device. Can only join clients.") @@ -353,7 +355,6 @@ async def async_join(self, master): for group in self._client.groups_available() if master_entity.identifier in group.clients ) - _LOGGER.debug(f"master group: {master_group}") await master_group.add_client(self._client.identifier) self.async_write_ha_state() diff --git a/tests/components/snapcast/test_init.py b/tests/components/snapcast/test_init.py index e816414dc5adad..cb3434cc79bab0 100644 --- a/tests/components/snapcast/test_init.py +++ b/tests/components/snapcast/test_init.py @@ -41,7 +41,7 @@ async def test_success(hass: HomeAssistant) -> None: assert len(mock_setup_entry.mock_calls) == 2 -async def test_error(hass: HomeAssistant) -> None: +async def test_unknown_error(hass: HomeAssistant) -> None: """Test what happens when there is no server to connect.""" await setup.async_setup_component(hass, "persistent_notification", {}) result = await hass.config_entries.flow.async_init( @@ -58,4 +58,24 @@ async def test_error(hass: HomeAssistant) -> None: await hass.async_block_till_done() assert result["type"] == "form" - assert result["errors"] == {"base": "unknown_error"} + assert result["errors"] == {"base": "unknown"} + + +async def test_connection_error(hass: HomeAssistant) -> None: + """Test what happens when there is no server to connect.""" + await setup.async_setup_component(hass, "persistent_notification", {}) + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] == "form" + assert not result["errors"] + + with patch("snapcast.control.create_server", side_effect=ConnectionRefusedError): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + TEST_CONNECTION, + ) + await hass.async_block_till_done() + + assert result["type"] == "form" + assert result["errors"] == {"base": "cannot_connect"} From 7140820f7a85e7e4d4e78a3783632b70e5744b5f Mon Sep 17 00:00:00 2001 From: raul Date: Thu, 13 Oct 2022 23:05:06 +0200 Subject: [PATCH 03/20] Fix linter errors --- .../components/snapcast/config_flow.py | 4 +-- .../components/snapcast/strings.json | 30 +++++++++---------- .../components/snapcast/translations/en.json | 30 +++++++++---------- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/homeassistant/components/snapcast/config_flow.py b/homeassistant/components/snapcast/config_flow.py index 48ec75688c034b..9839d56c5c5b5e 100644 --- a/homeassistant/components/snapcast/config_flow.py +++ b/homeassistant/components/snapcast/config_flow.py @@ -42,6 +42,7 @@ def _show_form(errors=None): host = user_input[CONF_HOST] port = user_input[CONF_PORT] errors = {} + client = None # Attempt to create the server - make sure it's going to work try: @@ -53,8 +54,7 @@ def _show_form(errors=None): except ConnectionRefusedError: errors["base"] = "cannot_connect" finally: - if "client" in locals(): - del client + del client await self.async_set_unique_id("Snapcast") self._abort_if_unique_id_configured() diff --git a/homeassistant/components/snapcast/strings.json b/homeassistant/components/snapcast/strings.json index 55c50bb67ee013..906e57a202c6d2 100644 --- a/homeassistant/components/snapcast/strings.json +++ b/homeassistant/components/snapcast/strings.json @@ -1,18 +1,18 @@ { - "config": { - "step": { - "user": { - "description": "Please enter your server connection details", - "data": { - "host": "[%key:common::config_flow::data::host%]", - "port": "[%key:common::config_flow::data::port%]" - }, - "title": "Connect" - } + "config": { + "step": { + "user": { + "description": "Please enter your server connection details", + "data": { + "host": "[%key:common::config_flow::data::host%]", + "port": "[%key:common::config_flow::data::port%]" }, - "error": { - "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", - "unknown": "[%key:common::config_flow::error::unknown%]" - } + "title": "Connect" + } + }, + "error": { + "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", + "unknown": "[%key:common::config_flow::error::unknown%]" } -} \ No newline at end of file + } +} diff --git a/homeassistant/components/snapcast/translations/en.json b/homeassistant/components/snapcast/translations/en.json index f6343f2f749c29..2f13da048cf4e9 100644 --- a/homeassistant/components/snapcast/translations/en.json +++ b/homeassistant/components/snapcast/translations/en.json @@ -1,18 +1,18 @@ { - "config": { - "error": { - "cannot_connect": "Failed to connect", - "unknown": "Unexpected error" + "config": { + "error": { + "cannot_connect": "Failed to connect", + "unknown": "Unexpected error" + }, + "step": { + "user": { + "data": { + "host": "Host", + "port": "Port" }, - "step": { - "user": { - "data": { - "host": "Host", - "port": "Port" - }, - "description": "Please enter your server connection details", - "title": "Connect" - } - } + "description": "Please enter your server connection details", + "title": "Connect" + } } -} \ No newline at end of file + } +} From 45610fac7ef3dbf96601b3224ae736a4831847a4 Mon Sep 17 00:00:00 2001 From: raul Date: Sat, 15 Oct 2022 15:45:34 +0200 Subject: [PATCH 04/20] Add import flow, support unloading --- CODEOWNERS | 2 + homeassistant/components/snapcast/__init__.py | 9 ++++ .../components/snapcast/config_flow.py | 7 ++- .../components/snapcast/manifest.json | 2 +- .../components/snapcast/media_player.py | 45 +++++++------------ .../components/snapcast/strings.json | 6 +++ .../components/snapcast/translations/en.json | 6 +++ 7 files changed, 46 insertions(+), 31 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index 5783cf31cab124..ee196decd21f53 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1041,6 +1041,8 @@ build.json @home-assistant/supervisor /homeassistant/components/smhi/ @gjohansson-ST /tests/components/smhi/ @gjohansson-ST /homeassistant/components/sms/ @ocalvo +/homeassistant/components/snapcast/ @luar123 +/tests/components/snapcast/ @luar123 /homeassistant/components/snooz/ @AustinBrunkhorst /tests/components/snooz/ @AustinBrunkhorst /homeassistant/components/solaredge/ @frenck diff --git a/homeassistant/components/snapcast/__init__.py b/homeassistant/components/snapcast/__init__.py index 6d600fc09f70d8..392ef908f02456 100644 --- a/homeassistant/components/snapcast/__init__.py +++ b/homeassistant/components/snapcast/__init__.py @@ -33,3 +33,12 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: ) return True + + +async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: + """Unload a config entry.""" + if unload_ok := await hass.config_entries.async_forward_entry_unload( + entry, "media_player" + ): + hass.data[DOMAIN].pop(entry.entry_id) + return unload_ok diff --git a/homeassistant/components/snapcast/config_flow.py b/homeassistant/components/snapcast/config_flow.py index 9839d56c5c5b5e..dc2da82c9895ee 100644 --- a/homeassistant/components/snapcast/config_flow.py +++ b/homeassistant/components/snapcast/config_flow.py @@ -10,6 +10,7 @@ from homeassistant.config_entries import ConfigFlow from homeassistant.const import CONF_HOST, CONF_PORT +from homeassistant.data_entry_flow import FlowResult from .const import DEFAULT_PORT, DOMAIN @@ -26,7 +27,7 @@ class SnapcastConfigFlow(ConfigFlow, domain=DOMAIN): """Snapcast config flow.""" - async def async_step_user(self, user_input=None): + async def async_step_user(self, user_input=None) -> FlowResult: """Handle first step.""" def _show_form(errors=None): @@ -70,3 +71,7 @@ def _show_form(errors=None): title=f"{host}:{port}", data=user_input, ) + + async def async_step_import(self, import_config: dict[str, str]) -> FlowResult: + """Import a config entry from configuration.yaml.""" + return await self.async_step_user(import_config) diff --git a/homeassistant/components/snapcast/manifest.json b/homeassistant/components/snapcast/manifest.json index 2f55b3a8e06654..92da00312734a6 100644 --- a/homeassistant/components/snapcast/manifest.json +++ b/homeassistant/components/snapcast/manifest.json @@ -3,7 +3,7 @@ "name": "Snapcast", "documentation": "https://www.home-assistant.io/integrations/snapcast", "requirements": ["snapcast==2.1.3"], - "codeowners": [], + "codeowners": ["@luar123"], "iot_class": "local_polling", "loggers": ["construct", "snapcast"], "config_flow": true diff --git a/homeassistant/components/snapcast/media_player.py b/homeassistant/components/snapcast/media_player.py index d40aeb42932548..3064886d0e4395 100644 --- a/homeassistant/components/snapcast/media_player.py +++ b/homeassistant/components/snapcast/media_player.py @@ -2,9 +2,7 @@ from __future__ import annotations import logging -import socket -import snapcast.control from snapcast.control.server import CONTROL_PORT, Snapserver import voluptuous as vol @@ -14,12 +12,12 @@ MediaPlayerEntityFeature, MediaPlayerState, ) -from homeassistant.config_entries import ConfigEntry +from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry from homeassistant.const import CONF_HOST, CONF_PORT from homeassistant.core import HomeAssistant -from homeassistant.exceptions import PlatformNotReady from homeassistant.helpers import config_validation as cv, entity_platform from homeassistant.helpers.entity_platform import AddEntitiesCallback +from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType from .const import ( @@ -94,34 +92,23 @@ async def async_setup_platform( discovery_info: DiscoveryInfoType | None = None, ) -> None: """Set up the Snapcast platform.""" + async_create_issue( + hass, + DOMAIN, + "deprecated_yaml", + breaks_in_ha_version="2022.12.0", + is_fixable=False, + severity=IssueSeverity.WARNING, + translation_key="deprecated_yaml", + ) - host = config.get(CONF_HOST) - port = config.get(CONF_PORT, CONTROL_PORT) - - register_services() - hass.data.setdefault(DOMAIN, {}) + config[CONF_PORT] = config.get(CONF_PORT, CONTROL_PORT) - try: - server = await snapcast.control.create_server( - hass.loop, host, port, reconnect=True + hass.async_create_task( + hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_IMPORT}, data=config ) - except socket.gaierror as ex: - raise PlatformNotReady( - f"Could not connect to Snapcast server at {host}:{port}" - ) from ex - - # Note: Host part is needed, when using multiple snapservers - hpid = f"{host}:{port}" - - groups: list[MediaPlayerEntity] = [ - SnapcastGroupDevice(group, hpid) for group in server.groups - ] - clients: list[MediaPlayerEntity] = [ - SnapcastClientDevice(client, hpid) for client in server.clients - ] - hass.data[DOMAIN]["clients"] = clients - hass.data[DOMAIN]["groups"] = groups - async_add_entities(clients + groups) + ) async def handle_async_join(entity, service_call): diff --git a/homeassistant/components/snapcast/strings.json b/homeassistant/components/snapcast/strings.json index 906e57a202c6d2..f39e50cc34c77a 100644 --- a/homeassistant/components/snapcast/strings.json +++ b/homeassistant/components/snapcast/strings.json @@ -14,5 +14,11 @@ "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", "unknown": "[%key:common::config_flow::error::unknown%]" } + }, + "issues": { + "deprecated_yaml": { + "title": "The Snapcast YAML configuration is being removed", + "description": "Configuring Snapcast using YAML is being removed.\n\nYour existing YAML configuration has been imported into the UI automatically.\n\nRemove the Snapcast YAML configuration from your configuration.yaml file and restart Home Assistant to fix this issue." + } } } diff --git a/homeassistant/components/snapcast/translations/en.json b/homeassistant/components/snapcast/translations/en.json index 2f13da048cf4e9..de89f4e4d1e460 100644 --- a/homeassistant/components/snapcast/translations/en.json +++ b/homeassistant/components/snapcast/translations/en.json @@ -14,5 +14,11 @@ "title": "Connect" } } + }, + "issues": { + "deprecated_yaml": { + "title": "The Snapcast YAML configuration is being removed", + "description": "Configuring Snapcast using YAML is being removed.\n\nYour existing YAML configuration has been imported into the UI automatically.\n\nRemove the Snapcast YAML configuration from your configuration.yaml file and restart Home Assistant to fix this issue." + } } } From 9d6e5290e0737bb069eb0a4872d83f808ff00fd7 Mon Sep 17 00:00:00 2001 From: raul Date: Sat, 15 Oct 2022 16:59:01 +0200 Subject: [PATCH 05/20] Add test for import flow --- tests/components/snapcast/test_init.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/components/snapcast/test_init.py b/tests/components/snapcast/test_init.py index cb3434cc79bab0..f78a25692a9ccd 100644 --- a/tests/components/snapcast/test_init.py +++ b/tests/components/snapcast/test_init.py @@ -79,3 +79,26 @@ async def test_connection_error(hass: HomeAssistant) -> None: assert result["type"] == "form" assert result["errors"] == {"base": "cannot_connect"} + + +async def test_import(hass: HomeAssistant) -> None: + """Test successful import.""" + mock_connection = create_mock_snapcast() + + with patch( + "snapcast.control.create_server", + return_value=mock_connection, + ) as mock_setup_entry: + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_IMPORT}, + data=TEST_CONNECTION, + ) + await hass.async_block_till_done() + + assert result["type"] == "create_entry" + assert ( + result["title"] == f"{TEST_CONNECTION[CONF_HOST]}:{TEST_CONNECTION[CONF_PORT]}" + ) + assert result["data"] == TEST_CONNECTION + assert len(mock_setup_entry.mock_calls) == 2 From 8d5059589c361e30303c89e238aecff313733b73 Mon Sep 17 00:00:00 2001 From: raul Date: Tue, 18 Oct 2022 23:55:57 +0200 Subject: [PATCH 06/20] Add dataclass and remove unique ID in config-flow --- homeassistant/components/snapcast/__init__.py | 32 ++++++++++++------- .../components/snapcast/config_flow.py | 18 ++++------- homeassistant/components/snapcast/const.py | 5 +-- .../components/snapcast/media_player.py | 19 +++++------ .../components/snapcast/strings.json | 2 +- .../components/snapcast/translations/en.json | 2 +- tests/components/snapcast/test_init.py | 2 +- 7 files changed, 43 insertions(+), 37 deletions(-) diff --git a/homeassistant/components/snapcast/__init__.py b/homeassistant/components/snapcast/__init__.py index 392ef908f02456..aa9fd839faf01c 100644 --- a/homeassistant/components/snapcast/__init__.py +++ b/homeassistant/components/snapcast/__init__.py @@ -1,26 +1,36 @@ """Snapcast Integration.""" +from dataclasses import dataclass import logging import socket import snapcast.control +from homeassistant.components.media_player import MediaPlayerEntity from homeassistant.config_entries import ConfigEntry +from homeassistant.const import CONF_HOST, CONF_PORT from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady -from .const import DOMAIN +from .const import DOMAIN, PLATFORMS _LOGGER = logging.getLogger(__name__) +@dataclass +class HomeAssistantSnapcastData: + """Snapcast data stored in the Home Assistant data object.""" + + server: snapcast.control.Snapserver + clients: list[MediaPlayerEntity] + groups: list[MediaPlayerEntity] + + async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up Snapcast from a config entry.""" - hass.data.setdefault(DOMAIN, {}) - - host = entry.data["host"] - port = entry.data["port"] + host = entry.data[CONF_HOST] + port = entry.data[CONF_PORT] try: - hass.data[DOMAIN][entry.entry_id] = await snapcast.control.create_server( + server = await snapcast.control.create_server( hass.loop, host, port, reconnect=True ) except socket.gaierror as ex: @@ -28,17 +38,17 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: f"Could not connect to Snapcast server at {host}:{port}" ) from ex - hass.async_create_task( - hass.config_entries.async_forward_entry_setup(entry, "media_player") + hass.data.setdefault(DOMAIN, {})[entry.entry_id] = HomeAssistantSnapcastData( + server, [], [] ) + await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) + return True async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Unload a config entry.""" - if unload_ok := await hass.config_entries.async_forward_entry_unload( - entry, "media_player" - ): + if unload_ok := await hass.config_entries.async_unload_platforms(entry, PLATFORMS): hass.data[DOMAIN].pop(entry.entry_id) return unload_ok diff --git a/homeassistant/components/snapcast/config_flow.py b/homeassistant/components/snapcast/config_flow.py index dc2da82c9895ee..26d3010fb36d1a 100644 --- a/homeassistant/components/snapcast/config_flow.py +++ b/homeassistant/components/snapcast/config_flow.py @@ -6,20 +6,21 @@ import socket import snapcast.control +from snapcast.control.server import CONTROL_PORT import voluptuous as vol from homeassistant.config_entries import ConfigFlow from homeassistant.const import CONF_HOST, CONF_PORT from homeassistant.data_entry_flow import FlowResult -from .const import DEFAULT_PORT, DOMAIN +from .const import DOMAIN _LOGGER = logging.getLogger(__name__) SNAPCAST_SCHEMA = vol.Schema( { vol.Required(CONF_HOST): str, - vol.Required(CONF_PORT, default=DEFAULT_PORT): int, + vol.Required(CONF_PORT, default=CONTROL_PORT): int, } ) @@ -48,23 +49,16 @@ def _show_form(errors=None): # Attempt to create the server - make sure it's going to work try: client = await snapcast.control.create_server( - self.hass.loop, host, port, reconnect=True + self.hass.loop, host, port, reconnect=False ) except socket.gaierror: - errors["base"] = "unknown" - except ConnectionRefusedError: + errors["base"] = "invalid_host" + except OSError: errors["base"] = "cannot_connect" finally: del client - await self.async_set_unique_id("Snapcast") - self._abort_if_unique_id_configured() - if errors: - _LOGGER.error( - "Could not connect to a Snapcast Server at the provided address" - ) - _LOGGER.error("Error: %s", (errors["base"])) return _show_form(errors=errors) return self.async_create_entry( diff --git a/homeassistant/components/snapcast/const.py b/homeassistant/components/snapcast/const.py index 43c2c851c11d93..b0033c0c7c7168 100644 --- a/homeassistant/components/snapcast/const.py +++ b/homeassistant/components/snapcast/const.py @@ -1,4 +1,7 @@ """Constants for Snapcast.""" +from homeassistant.const import Platform + +PLATFORMS: list[Platform] = [Platform.MEDIA_PLAYER] GROUP_PREFIX = "snapcast_group_" GROUP_SUFFIX = "Snapcast Group" @@ -14,6 +17,4 @@ ATTR_MASTER = "master" ATTR_LATENCY = "latency" -DEFAULT_PORT = 1705 - DOMAIN = "snapcast" diff --git a/homeassistant/components/snapcast/media_player.py b/homeassistant/components/snapcast/media_player.py index 3064886d0e4395..96860f75c17cff 100644 --- a/homeassistant/components/snapcast/media_player.py +++ b/homeassistant/components/snapcast/media_player.py @@ -65,23 +65,23 @@ async def async_setup_entry( async_add_entities: AddEntitiesCallback, ) -> None: """Set up the snapcast config entry.""" - the_server: Snapserver = hass.data[DOMAIN][config_entry.entry_id] - _LOGGER.debug(the_server) + the_server: Snapserver = hass.data[DOMAIN][config_entry.entry_id].server register_services() - host = config_entry.data["host"] - port = config_entry.data["port"] + host = config_entry.data[CONF_HOST] + port = config_entry.data[CONF_PORT] hpid = f"{host}:{port}" groups: list[MediaPlayerEntity] = [ SnapcastGroupDevice(group, hpid) for group in the_server.groups ] clients: list[MediaPlayerEntity] = [ - SnapcastClientDevice(client, hpid) for client in the_server.clients + SnapcastClientDevice(client, hpid, config_entry.entry_id) + for client in the_server.clients ] - hass.data[DOMAIN]["clients"] = clients - hass.data[DOMAIN]["groups"] = groups + hass.data[DOMAIN][config_entry.entry_id].clients = clients + hass.data[DOMAIN][config_entry.entry_id].groups = groups async_add_entities(clients + groups) @@ -237,10 +237,11 @@ class SnapcastClientDevice(MediaPlayerEntity): | MediaPlayerEntityFeature.SELECT_SOURCE ) - def __init__(self, client, uid_part): + def __init__(self, client, uid_part, entry_id): """Initialize the Snapcast client device.""" self._client = client self._uid = f"{CLIENT_PREFIX}{uid_part}_{self._client.identifier}" + self.entry_id = entry_id async def async_added_to_hass(self) -> None: """Subscribe to client events.""" @@ -332,7 +333,7 @@ async def async_join(self, master): """Join the group of the master player.""" master_entity = next( entity - for entity in self.hass.data[DOMAIN]["clients"] + for entity in self.hass.data[DOMAIN][self.entry_id].clients if entity.entity_id == master ) if not isinstance(master_entity, SnapcastClientDevice): diff --git a/homeassistant/components/snapcast/strings.json b/homeassistant/components/snapcast/strings.json index f39e50cc34c77a..463285879114cb 100644 --- a/homeassistant/components/snapcast/strings.json +++ b/homeassistant/components/snapcast/strings.json @@ -12,7 +12,7 @@ }, "error": { "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", - "unknown": "[%key:common::config_flow::error::unknown%]" + "invalid_host": "[%key:common::config_flow::error::invalid_host%]" } }, "issues": { diff --git a/homeassistant/components/snapcast/translations/en.json b/homeassistant/components/snapcast/translations/en.json index de89f4e4d1e460..b2a9dd1d43b72d 100644 --- a/homeassistant/components/snapcast/translations/en.json +++ b/homeassistant/components/snapcast/translations/en.json @@ -2,7 +2,7 @@ "config": { "error": { "cannot_connect": "Failed to connect", - "unknown": "Unexpected error" + "invalid_host": "Host name or IP address is not valid" }, "step": { "user": { diff --git a/tests/components/snapcast/test_init.py b/tests/components/snapcast/test_init.py index f78a25692a9ccd..99d15adbdcdc15 100644 --- a/tests/components/snapcast/test_init.py +++ b/tests/components/snapcast/test_init.py @@ -58,7 +58,7 @@ async def test_unknown_error(hass: HomeAssistant) -> None: await hass.async_block_till_done() assert result["type"] == "form" - assert result["errors"] == {"base": "unknown"} + assert result["errors"] == {"base": "invalid_host"} async def test_connection_error(hass: HomeAssistant) -> None: From 4b52865a2acbeca576f6367cc91a0a52afe9cf37 Mon Sep 17 00:00:00 2001 From: raul Date: Wed, 8 Mar 2023 21:09:20 +0100 Subject: [PATCH 07/20] remove translations --- .../components/snapcast/translations/en.json | 24 ------------------- 1 file changed, 24 deletions(-) delete mode 100644 homeassistant/components/snapcast/translations/en.json diff --git a/homeassistant/components/snapcast/translations/en.json b/homeassistant/components/snapcast/translations/en.json deleted file mode 100644 index b196607f0f9e80..00000000000000 --- a/homeassistant/components/snapcast/translations/en.json +++ /dev/null @@ -1,24 +0,0 @@ -{ - "config": { - "error": { - "cannot_connect": "Failed to connect", - "invalid_host": "Invalid hostname or IP address" - }, - "step": { - "user": { - "data": { - "host": "Host", - "port": "Port" - }, - "description": "Please enter your server connection details", - "title": "Connect" - } - } - }, - "issues": { - "deprecated_yaml": { - "description": "Configuring Snapcast using YAML is being removed.\n\nYour existing YAML configuration has been imported into the UI automatically.\n\nRemove the Snapcast YAML configuration from your configuration.yaml file and restart Home Assistant to fix this issue.", - "title": "The Snapcast YAML configuration is being removed" - } - } -} \ No newline at end of file From b9dbc5b0d722833089b0b27fcb48516f8367848e Mon Sep 17 00:00:00 2001 From: luar123 <49960470+luar123@users.noreply.github.com> Date: Thu, 23 Mar 2023 22:53:53 +0100 Subject: [PATCH 08/20] Apply suggestions from code review Co-authored-by: epenet <6771947+epenet@users.noreply.github.com> --- tests/components/snapcast/test_init.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/components/snapcast/test_init.py b/tests/components/snapcast/test_init.py index 99d15adbdcdc15..b6010cff182cd2 100644 --- a/tests/components/snapcast/test_init.py +++ b/tests/components/snapcast/test_init.py @@ -27,7 +27,7 @@ async def test_success(hass: HomeAssistant) -> None: with patch( "snapcast.control.create_server", return_value=mock_connection, - ) as mock_setup_entry: + ) as mock_create_server: result = await hass.config_entries.flow.async_configure( result["flow_id"], TEST_CONNECTION ) @@ -38,7 +38,7 @@ async def test_success(hass: HomeAssistant) -> None: result["title"] == f"{TEST_CONNECTION[CONF_HOST]}:{TEST_CONNECTION[CONF_PORT]}" ) assert result["data"] == TEST_CONNECTION - assert len(mock_setup_entry.mock_calls) == 2 + assert len(mock_create_server.mock_calls) == 2 async def test_unknown_error(hass: HomeAssistant) -> None: @@ -88,7 +88,7 @@ async def test_import(hass: HomeAssistant) -> None: with patch( "snapcast.control.create_server", return_value=mock_connection, - ) as mock_setup_entry: + ) as mock_create_server: result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, @@ -101,4 +101,4 @@ async def test_import(hass: HomeAssistant) -> None: result["title"] == f"{TEST_CONNECTION[CONF_HOST]}:{TEST_CONNECTION[CONF_PORT]}" ) assert result["data"] == TEST_CONNECTION - assert len(mock_setup_entry.mock_calls) == 2 + assert len(mock_create_server.mock_calls) == 2 From b9a57d2be182b949a7415a2d04bc395669c09cc4 Mon Sep 17 00:00:00 2001 From: raul Date: Thu, 23 Mar 2023 23:01:16 +0100 Subject: [PATCH 09/20] Refactor config flow and terminate connection --- .../components/snapcast/config_flow.py | 52 +++++++------------ 1 file changed, 19 insertions(+), 33 deletions(-) diff --git a/homeassistant/components/snapcast/config_flow.py b/homeassistant/components/snapcast/config_flow.py index 26d3010fb36d1a..84e452f99c7935 100644 --- a/homeassistant/components/snapcast/config_flow.py +++ b/homeassistant/components/snapcast/config_flow.py @@ -30,40 +30,26 @@ class SnapcastConfigFlow(ConfigFlow, domain=DOMAIN): async def async_step_user(self, user_input=None) -> FlowResult: """Handle first step.""" - - def _show_form(errors=None): - return self.async_show_form( - step_id="user", - data_schema=SNAPCAST_SCHEMA, - errors=errors, - ) - - if not user_input: - return _show_form() - - host = user_input[CONF_HOST] - port = user_input[CONF_PORT] errors = {} - client = None - - # Attempt to create the server - make sure it's going to work - try: - client = await snapcast.control.create_server( - self.hass.loop, host, port, reconnect=False - ) - except socket.gaierror: - errors["base"] = "invalid_host" - except OSError: - errors["base"] = "cannot_connect" - finally: - del client - - if errors: - return _show_form(errors=errors) - - return self.async_create_entry( - title=f"{host}:{port}", - data=user_input, + if user_input: + host = user_input[CONF_HOST] + port = user_input[CONF_PORT] + client = None + + # Attempt to create the server - make sure it's going to work + try: + client = await snapcast.control.create_server( + self.hass.loop, host, port, reconnect=False + ) + except socket.gaierror: + errors["base"] = "invalid_host" + except OSError: + errors["base"] = "cannot_connect" + else: + await client.stop() + return self.async_create_entry(title=f"{host}:{port}", data=user_input) + return self.async_show_form( + step_id="user", data_schema=SNAPCAST_SCHEMA, errors=errors ) async def async_step_import(self, import_config: dict[str, str]) -> FlowResult: From 7c974605b78c0c4f7575853be1cb409ea45bdb00 Mon Sep 17 00:00:00 2001 From: raul Date: Thu, 23 Mar 2023 23:05:25 +0100 Subject: [PATCH 10/20] Rename test_config_flow.py --- tests/components/snapcast/{test_init.py => test_config_flow.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/components/snapcast/{test_init.py => test_config_flow.py} (100%) diff --git a/tests/components/snapcast/test_init.py b/tests/components/snapcast/test_config_flow.py similarity index 100% rename from tests/components/snapcast/test_init.py rename to tests/components/snapcast/test_config_flow.py From ec67248e4eeea5db08dca4621cc305e81de21fb4 Mon Sep 17 00:00:00 2001 From: raul Date: Thu, 23 Mar 2023 23:35:57 +0100 Subject: [PATCH 11/20] Fix tests --- .../components/snapcast/media_player.py | 2 +- tests/components/snapcast/conftest.py | 14 +++++++++++ tests/components/snapcast/test_config_flow.py | 24 ++++++++++++------- 3 files changed, 30 insertions(+), 10 deletions(-) create mode 100644 tests/components/snapcast/conftest.py diff --git a/homeassistant/components/snapcast/media_player.py b/homeassistant/components/snapcast/media_player.py index d1f38a02df385a..2b60df919c9df9 100644 --- a/homeassistant/components/snapcast/media_player.py +++ b/homeassistant/components/snapcast/media_player.py @@ -96,7 +96,7 @@ async def async_setup_platform( hass, DOMAIN, "deprecated_yaml", - breaks_in_ha_version="2022.12.0", + breaks_in_ha_version="2023.6.0", is_fixable=False, severity=IssueSeverity.WARNING, translation_key="deprecated_yaml", diff --git a/tests/components/snapcast/conftest.py b/tests/components/snapcast/conftest.py new file mode 100644 index 00000000000000..a292b5d6abd2f4 --- /dev/null +++ b/tests/components/snapcast/conftest.py @@ -0,0 +1,14 @@ +"""Test the snapcast config flow.""" +from collections.abc import Generator +from unittest.mock import AsyncMock, patch + +import pytest + + +@pytest.fixture +def mock_setup_entry() -> Generator[AsyncMock, None, None]: + """Override async_setup_entry.""" + with patch( + "homeassistant.components.snapcast.async_setup_entry", return_value=True + ) as mock_setup_entry: + yield mock_setup_entry diff --git a/tests/components/snapcast/test_config_flow.py b/tests/components/snapcast/test_config_flow.py index b6010cff182cd2..57aabfe597c416 100644 --- a/tests/components/snapcast/test_config_flow.py +++ b/tests/components/snapcast/test_config_flow.py @@ -1,25 +1,30 @@ """Test the Snapcast module.""" import socket -from unittest.mock import patch +from unittest.mock import AsyncMock, patch + +import pytest from homeassistant import config_entries, setup from homeassistant.components.snapcast.const import DOMAIN from homeassistant.const import CONF_HOST, CONF_PORT from homeassistant.core import HomeAssistant +from homeassistant.data_entry_flow import FlowResultType from . import create_mock_snapcast TEST_CONNECTION = {CONF_HOST: "snapserver.test", CONF_PORT: 1705} +pytestmark = pytest.mark.usefixtures("mock_setup_entry") + -async def test_success(hass: HomeAssistant) -> None: +async def test_success(hass: HomeAssistant, mock_setup_entry: AsyncMock) -> None: """Test successful connection.""" await setup.async_setup_component(hass, "persistent_notification", {}) result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} ) - assert result["type"] == "form" + assert result["type"] == FlowResultType.FORM assert not result["errors"] mock_connection = create_mock_snapcast() @@ -38,7 +43,8 @@ async def test_success(hass: HomeAssistant) -> None: result["title"] == f"{TEST_CONNECTION[CONF_HOST]}:{TEST_CONNECTION[CONF_PORT]}" ) assert result["data"] == TEST_CONNECTION - assert len(mock_create_server.mock_calls) == 2 + assert len(mock_create_server.mock_calls) == 1 + assert len(mock_setup_entry.mock_calls) == 1 async def test_unknown_error(hass: HomeAssistant) -> None: @@ -47,7 +53,7 @@ async def test_unknown_error(hass: HomeAssistant) -> None: result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} ) - assert result["type"] == "form" + assert result["type"] == FlowResultType.FORM assert not result["errors"] with patch("snapcast.control.create_server", side_effect=socket.gaierror): @@ -57,7 +63,7 @@ async def test_unknown_error(hass: HomeAssistant) -> None: ) await hass.async_block_till_done() - assert result["type"] == "form" + assert result["type"] == FlowResultType.FORM assert result["errors"] == {"base": "invalid_host"} @@ -67,7 +73,7 @@ async def test_connection_error(hass: HomeAssistant) -> None: result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} ) - assert result["type"] == "form" + assert result["type"] == FlowResultType.FORM assert not result["errors"] with patch("snapcast.control.create_server", side_effect=ConnectionRefusedError): @@ -77,7 +83,7 @@ async def test_connection_error(hass: HomeAssistant) -> None: ) await hass.async_block_till_done() - assert result["type"] == "form" + assert result["type"] == FlowResultType.FORM assert result["errors"] == {"base": "cannot_connect"} @@ -101,4 +107,4 @@ async def test_import(hass: HomeAssistant) -> None: result["title"] == f"{TEST_CONNECTION[CONF_HOST]}:{TEST_CONNECTION[CONF_PORT]}" ) assert result["data"] == TEST_CONNECTION - assert len(mock_create_server.mock_calls) == 2 + assert len(mock_create_server.mock_calls) == 1 From 50580e937267a51ca95278aae214748a1944e8c3 Mon Sep 17 00:00:00 2001 From: raul Date: Thu, 23 Mar 2023 23:44:07 +0100 Subject: [PATCH 12/20] Minor fixes --- homeassistant/components/snapcast/config_flow.py | 1 - tests/components/snapcast/test_config_flow.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/snapcast/config_flow.py b/homeassistant/components/snapcast/config_flow.py index 84e452f99c7935..968b4a99459c5e 100644 --- a/homeassistant/components/snapcast/config_flow.py +++ b/homeassistant/components/snapcast/config_flow.py @@ -34,7 +34,6 @@ async def async_step_user(self, user_input=None) -> FlowResult: if user_input: host = user_input[CONF_HOST] port = user_input[CONF_PORT] - client = None # Attempt to create the server - make sure it's going to work try: diff --git a/tests/components/snapcast/test_config_flow.py b/tests/components/snapcast/test_config_flow.py index 57aabfe597c416..08edb87dd04364 100644 --- a/tests/components/snapcast/test_config_flow.py +++ b/tests/components/snapcast/test_config_flow.py @@ -38,7 +38,7 @@ async def test_success(hass: HomeAssistant, mock_setup_entry: AsyncMock) -> None ) await hass.async_block_till_done() - assert result["type"] == "create_entry" + assert result["type"] == FlowResultType.CREATE_ENTRY assert ( result["title"] == f"{TEST_CONNECTION[CONF_HOST]}:{TEST_CONNECTION[CONF_PORT]}" ) @@ -102,7 +102,7 @@ async def test_import(hass: HomeAssistant) -> None: ) await hass.async_block_till_done() - assert result["type"] == "create_entry" + assert result["type"] == FlowResultType.CREATE_ENTRY assert ( result["title"] == f"{TEST_CONNECTION[CONF_HOST]}:{TEST_CONNECTION[CONF_PORT]}" ) From d520968f87d344bb8ca00be2b0af327162ea8aef Mon Sep 17 00:00:00 2001 From: raul Date: Fri, 24 Mar 2023 21:36:14 +0100 Subject: [PATCH 13/20] Make mock_create_server a fixture --- tests/components/snapcast/__init__.py | 9 ---- tests/components/snapcast/conftest.py | 9 ++++ tests/components/snapcast/test_config_flow.py | 43 +++++++------------ 3 files changed, 25 insertions(+), 36 deletions(-) diff --git a/tests/components/snapcast/__init__.py b/tests/components/snapcast/__init__.py index bd4dabbdcd2490..a325bd41bd7d66 100644 --- a/tests/components/snapcast/__init__.py +++ b/tests/components/snapcast/__init__.py @@ -1,10 +1 @@ """Tests for the Snapcast integration.""" - -from unittest.mock import AsyncMock - - -def create_mock_snapcast() -> AsyncMock: - """Create mock snapcast connection.""" - mock_connection = AsyncMock() - mock_connection.start = AsyncMock(return_value=None) - return mock_connection diff --git a/tests/components/snapcast/conftest.py b/tests/components/snapcast/conftest.py index a292b5d6abd2f4..00d031192d819d 100644 --- a/tests/components/snapcast/conftest.py +++ b/tests/components/snapcast/conftest.py @@ -12,3 +12,12 @@ def mock_setup_entry() -> Generator[AsyncMock, None, None]: "homeassistant.components.snapcast.async_setup_entry", return_value=True ) as mock_setup_entry: yield mock_setup_entry + + +@pytest.fixture +def mock_create_server() -> Generator[AsyncMock, None, None]: + """Create mock snapcast connection.""" + mock_connection = AsyncMock() + mock_connection.start = AsyncMock(return_value=None) + with patch("snapcast.control.create_server", return_value=mock_connection): + yield mock_connection diff --git a/tests/components/snapcast/test_config_flow.py b/tests/components/snapcast/test_config_flow.py index 08edb87dd04364..ecb487770eaf90 100644 --- a/tests/components/snapcast/test_config_flow.py +++ b/tests/components/snapcast/test_config_flow.py @@ -11,14 +11,14 @@ from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResultType -from . import create_mock_snapcast - TEST_CONNECTION = {CONF_HOST: "snapserver.test", CONF_PORT: 1705} -pytestmark = pytest.mark.usefixtures("mock_setup_entry") +pytestmark = pytest.mark.usefixtures("mock_setup_entry", "mock_create_server") -async def test_success(hass: HomeAssistant, mock_setup_entry: AsyncMock) -> None: +async def test_success( + hass: HomeAssistant, mock_setup_entry: AsyncMock, mock_create_server: AsyncMock +) -> None: """Test successful connection.""" await setup.async_setup_component(hass, "persistent_notification", {}) result = await hass.config_entries.flow.async_init( @@ -27,16 +27,10 @@ async def test_success(hass: HomeAssistant, mock_setup_entry: AsyncMock) -> None assert result["type"] == FlowResultType.FORM assert not result["errors"] - mock_connection = create_mock_snapcast() - - with patch( - "snapcast.control.create_server", - return_value=mock_connection, - ) as mock_create_server: - result = await hass.config_entries.flow.async_configure( - result["flow_id"], TEST_CONNECTION - ) - await hass.async_block_till_done() + result = await hass.config_entries.flow.async_configure( + result["flow_id"], TEST_CONNECTION + ) + await hass.async_block_till_done() assert result["type"] == FlowResultType.CREATE_ENTRY assert ( @@ -87,20 +81,15 @@ async def test_connection_error(hass: HomeAssistant) -> None: assert result["errors"] == {"base": "cannot_connect"} -async def test_import(hass: HomeAssistant) -> None: +async def test_import(hass: HomeAssistant, mock_create_server: AsyncMock) -> None: """Test successful import.""" - mock_connection = create_mock_snapcast() - - with patch( - "snapcast.control.create_server", - return_value=mock_connection, - ) as mock_create_server: - result = await hass.config_entries.flow.async_init( - DOMAIN, - context={"source": config_entries.SOURCE_IMPORT}, - data=TEST_CONNECTION, - ) - await hass.async_block_till_done() + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_IMPORT}, + data=TEST_CONNECTION, + ) + await hass.async_block_till_done() assert result["type"] == FlowResultType.CREATE_ENTRY assert ( From 5ac9af4c48cdd029dc1d4fb1ac8b5fad48fbaecd Mon Sep 17 00:00:00 2001 From: raul Date: Fri, 24 Mar 2023 21:45:01 +0100 Subject: [PATCH 14/20] Combine tests --- tests/components/snapcast/test_config_flow.py | 53 +++++++------------ 1 file changed, 18 insertions(+), 35 deletions(-) diff --git a/tests/components/snapcast/test_config_flow.py b/tests/components/snapcast/test_config_flow.py index ecb487770eaf90..105de38e80c19a 100644 --- a/tests/components/snapcast/test_config_flow.py +++ b/tests/components/snapcast/test_config_flow.py @@ -16,33 +16,10 @@ pytestmark = pytest.mark.usefixtures("mock_setup_entry", "mock_create_server") -async def test_success( +async def test_form( hass: HomeAssistant, mock_setup_entry: AsyncMock, mock_create_server: AsyncMock ) -> None: - """Test successful connection.""" - await setup.async_setup_component(hass, "persistent_notification", {}) - result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": config_entries.SOURCE_USER} - ) - assert result["type"] == FlowResultType.FORM - assert not result["errors"] - - result = await hass.config_entries.flow.async_configure( - result["flow_id"], TEST_CONNECTION - ) - await hass.async_block_till_done() - - assert result["type"] == FlowResultType.CREATE_ENTRY - assert ( - result["title"] == f"{TEST_CONNECTION[CONF_HOST]}:{TEST_CONNECTION[CONF_PORT]}" - ) - assert result["data"] == TEST_CONNECTION - assert len(mock_create_server.mock_calls) == 1 - assert len(mock_setup_entry.mock_calls) == 1 - - -async def test_unknown_error(hass: HomeAssistant) -> None: - """Test what happens when there is no server to connect.""" + """Test we get the form and handle errors and successful connection.""" await setup.async_setup_component(hass, "persistent_notification", {}) result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} @@ -50,6 +27,7 @@ async def test_unknown_error(hass: HomeAssistant) -> None: assert result["type"] == FlowResultType.FORM assert not result["errors"] + # test invalid host error with patch("snapcast.control.create_server", side_effect=socket.gaierror): result = await hass.config_entries.flow.async_configure( result["flow_id"], @@ -60,16 +38,7 @@ async def test_unknown_error(hass: HomeAssistant) -> None: assert result["type"] == FlowResultType.FORM assert result["errors"] == {"base": "invalid_host"} - -async def test_connection_error(hass: HomeAssistant) -> None: - """Test what happens when there is no server to connect.""" - await setup.async_setup_component(hass, "persistent_notification", {}) - result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": config_entries.SOURCE_USER} - ) - assert result["type"] == FlowResultType.FORM - assert not result["errors"] - + # test connection error with patch("snapcast.control.create_server", side_effect=ConnectionRefusedError): result = await hass.config_entries.flow.async_configure( result["flow_id"], @@ -80,6 +49,20 @@ async def test_connection_error(hass: HomeAssistant) -> None: assert result["type"] == FlowResultType.FORM assert result["errors"] == {"base": "cannot_connect"} + # test success + result = await hass.config_entries.flow.async_configure( + result["flow_id"], TEST_CONNECTION + ) + await hass.async_block_till_done() + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert ( + result["title"] == f"{TEST_CONNECTION[CONF_HOST]}:{TEST_CONNECTION[CONF_PORT]}" + ) + assert result["data"] == TEST_CONNECTION + assert len(mock_create_server.mock_calls) == 1 + assert len(mock_setup_entry.mock_calls) == 1 + async def test_import(hass: HomeAssistant, mock_create_server: AsyncMock) -> None: """Test successful import.""" From cb62c3f6f2dd8951a40a8ec22aaaa25ecc215854 Mon Sep 17 00:00:00 2001 From: raul Date: Sat, 25 Mar 2023 18:52:38 +0100 Subject: [PATCH 15/20] Abort if entry already exists --- .../components/snapcast/config_flow.py | 1 + .../components/snapcast/strings.json | 3 ++ tests/components/snapcast/test_config_flow.py | 31 +++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/homeassistant/components/snapcast/config_flow.py b/homeassistant/components/snapcast/config_flow.py index 968b4a99459c5e..19dfd8690df3b5 100644 --- a/homeassistant/components/snapcast/config_flow.py +++ b/homeassistant/components/snapcast/config_flow.py @@ -32,6 +32,7 @@ async def async_step_user(self, user_input=None) -> FlowResult: """Handle first step.""" errors = {} if user_input: + self._async_abort_entries_match(user_input) host = user_input[CONF_HOST] port = user_input[CONF_PORT] diff --git a/homeassistant/components/snapcast/strings.json b/homeassistant/components/snapcast/strings.json index 463285879114cb..0087b70d8204e0 100644 --- a/homeassistant/components/snapcast/strings.json +++ b/homeassistant/components/snapcast/strings.json @@ -10,6 +10,9 @@ "title": "Connect" } }, + "abort": { + "already_configured": "[%key:common::config_flow::abort::already_configured_device%]" + }, "error": { "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", "invalid_host": "[%key:common::config_flow::error::invalid_host%]" diff --git a/tests/components/snapcast/test_config_flow.py b/tests/components/snapcast/test_config_flow.py index 105de38e80c19a..a8b37dff568072 100644 --- a/tests/components/snapcast/test_config_flow.py +++ b/tests/components/snapcast/test_config_flow.py @@ -11,6 +11,8 @@ from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResultType +from tests.common import MockConfigEntry + TEST_CONNECTION = {CONF_HOST: "snapserver.test", CONF_PORT: 1705} pytestmark = pytest.mark.usefixtures("mock_setup_entry", "mock_create_server") @@ -64,6 +66,35 @@ async def test_form( assert len(mock_setup_entry.mock_calls) == 1 +async def test_abort( + hass: HomeAssistant, mock_setup_entry: AsyncMock, mock_create_server: AsyncMock +) -> None: + """Test config flow abort if device is already configured.""" + entry = MockConfigEntry( + domain=DOMAIN, + title=f"{TEST_CONNECTION[CONF_HOST]}:{TEST_CONNECTION[CONF_PORT]}", + data=TEST_CONNECTION, + ) + entry.add_to_hass(hass) + + await setup.async_setup_component(hass, "persistent_notification", {}) + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] == FlowResultType.FORM + assert not result["errors"] + + with patch("snapcast.control.create_server", side_effect=socket.gaierror): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + TEST_CONNECTION, + ) + await hass.async_block_till_done() + + assert result["type"] == FlowResultType.ABORT + assert result["reason"] == "already_configured" + + async def test_import(hass: HomeAssistant, mock_create_server: AsyncMock) -> None: """Test successful import.""" From 69a902f4ad070a8ecb20a84ab8d24b25555a63d8 Mon Sep 17 00:00:00 2001 From: luar123 <49960470+luar123@users.noreply.github.com> Date: Tue, 28 Mar 2023 19:14:17 +0200 Subject: [PATCH 16/20] Apply suggestions from code review Co-authored-by: epenet <6771947+epenet@users.noreply.github.com> --- .../components/snapcast/media_player.py | 16 +++++++--------- tests/components/snapcast/test_config_flow.py | 7 +++---- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/homeassistant/components/snapcast/media_player.py b/homeassistant/components/snapcast/media_player.py index 2b60df919c9df9..175dac5834c694 100644 --- a/homeassistant/components/snapcast/media_player.py +++ b/homeassistant/components/snapcast/media_player.py @@ -65,7 +65,7 @@ async def async_setup_entry( async_add_entities: AddEntitiesCallback, ) -> None: """Set up the snapcast config entry.""" - the_server: Snapserver = hass.data[DOMAIN][config_entry.entry_id].server + snapcast_data: HomeAssistantSnapcastData = hass.data[DOMAIN][config_entry.entry_id] register_services() @@ -73,15 +73,13 @@ async def async_setup_entry( port = config_entry.data[CONF_PORT] hpid = f"{host}:{port}" - groups: list[MediaPlayerEntity] = [ - SnapcastGroupDevice(group, hpid) for group in the_server.groups + snapcast_data.groups: list[MediaPlayerEntity] = [ + SnapcastGroupDevice(group, hpid) for group in snapcast_data.server.groups ] - clients: list[MediaPlayerEntity] = [ + snapcast_data.clients: list[MediaPlayerEntity] = [ SnapcastClientDevice(client, hpid, config_entry.entry_id) - for client in the_server.clients + for client in snapcast_data.server.clients ] - hass.data[DOMAIN][config_entry.entry_id].clients = clients - hass.data[DOMAIN][config_entry.entry_id].groups = groups async_add_entities(clients + groups) @@ -241,7 +239,7 @@ def __init__(self, client, uid_part, entry_id): """Initialize the Snapcast client device.""" self._client = client self._uid = f"{CLIENT_PREFIX}{uid_part}_{self._client.identifier}" - self.entry_id = entry_id + self._entry_id = entry_id async def async_added_to_hass(self) -> None: """Subscribe to client events.""" @@ -332,7 +330,7 @@ async def async_join(self, master): """Join the group of the master player.""" master_entity = next( entity - for entity in self.hass.data[DOMAIN][self.entry_id].clients + for entity in self.hass.data[DOMAIN][self._entry_id].clients if entity.entity_id == master ) if not isinstance(master_entity, SnapcastClientDevice): diff --git a/tests/components/snapcast/test_config_flow.py b/tests/components/snapcast/test_config_flow.py index a8b37dff568072..8e69f18009885c 100644 --- a/tests/components/snapcast/test_config_flow.py +++ b/tests/components/snapcast/test_config_flow.py @@ -35,7 +35,7 @@ async def test_form( result["flow_id"], TEST_CONNECTION, ) - await hass.async_block_till_done() + await hass.async_block_till_done() assert result["type"] == FlowResultType.FORM assert result["errors"] == {"base": "invalid_host"} @@ -46,7 +46,7 @@ async def test_form( result["flow_id"], TEST_CONNECTION, ) - await hass.async_block_till_done() + await hass.async_block_till_done() assert result["type"] == FlowResultType.FORM assert result["errors"] == {"base": "cannot_connect"} @@ -77,7 +77,6 @@ async def test_abort( ) entry.add_to_hass(hass) - await setup.async_setup_component(hass, "persistent_notification", {}) result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} ) @@ -89,7 +88,7 @@ async def test_abort( result["flow_id"], TEST_CONNECTION, ) - await hass.async_block_till_done() + await hass.async_block_till_done() assert result["type"] == FlowResultType.ABORT assert result["reason"] == "already_configured" From 2d3ad786d8421434b97f9eacc106667d53fd102f Mon Sep 17 00:00:00 2001 From: raul Date: Tue, 28 Mar 2023 20:59:33 +0200 Subject: [PATCH 17/20] Move HomeAssistantSnapcast to own file. Clean-up last commit --- .coveragerc | 1 + homeassistant/components/snapcast/__init__.py | 14 ++------------ homeassistant/components/snapcast/media_player.py | 11 ++++++----- homeassistant/components/snapcast/server.py | 15 +++++++++++++++ 4 files changed, 24 insertions(+), 17 deletions(-) create mode 100644 homeassistant/components/snapcast/server.py diff --git a/.coveragerc b/.coveragerc index 8fa21279e89ed1..4bc095e34655d4 100644 --- a/.coveragerc +++ b/.coveragerc @@ -1104,6 +1104,7 @@ omit = homeassistant/components/smtp/notify.py homeassistant/components/snapcast/__init__.py homeassistant/components/snapcast/media_player.py + homeassistant/components/snapcast/server.py homeassistant/components/snmp/device_tracker.py homeassistant/components/snmp/sensor.py homeassistant/components/snmp/switch.py diff --git a/homeassistant/components/snapcast/__init__.py b/homeassistant/components/snapcast/__init__.py index aa9fd839faf01c..c4259c4457a84d 100644 --- a/homeassistant/components/snapcast/__init__.py +++ b/homeassistant/components/snapcast/__init__.py @@ -1,30 +1,20 @@ """Snapcast Integration.""" -from dataclasses import dataclass import logging import socket import snapcast.control -from homeassistant.components.media_player import MediaPlayerEntity from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_HOST, CONF_PORT from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady from .const import DOMAIN, PLATFORMS +from .server import HomeAssistantSnapcast _LOGGER = logging.getLogger(__name__) -@dataclass -class HomeAssistantSnapcastData: - """Snapcast data stored in the Home Assistant data object.""" - - server: snapcast.control.Snapserver - clients: list[MediaPlayerEntity] - groups: list[MediaPlayerEntity] - - async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up Snapcast from a config entry.""" host = entry.data[CONF_HOST] @@ -38,7 +28,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: f"Could not connect to Snapcast server at {host}:{port}" ) from ex - hass.data.setdefault(DOMAIN, {})[entry.entry_id] = HomeAssistantSnapcastData( + hass.data.setdefault(DOMAIN, {})[entry.entry_id] = HomeAssistantSnapcast( server, [], [] ) diff --git a/homeassistant/components/snapcast/media_player.py b/homeassistant/components/snapcast/media_player.py index 175dac5834c694..6f965155bba4df 100644 --- a/homeassistant/components/snapcast/media_player.py +++ b/homeassistant/components/snapcast/media_player.py @@ -3,7 +3,7 @@ import logging -from snapcast.control.server import CONTROL_PORT, Snapserver +from snapcast.control.server import CONTROL_PORT import voluptuous as vol from homeassistant.components.media_player import ( @@ -34,6 +34,7 @@ SERVICE_SNAPSHOT, SERVICE_UNJOIN, ) +from .server import HomeAssistantSnapcast _LOGGER = logging.getLogger(__name__) @@ -65,7 +66,7 @@ async def async_setup_entry( async_add_entities: AddEntitiesCallback, ) -> None: """Set up the snapcast config entry.""" - snapcast_data: HomeAssistantSnapcastData = hass.data[DOMAIN][config_entry.entry_id] + snapcast_data: HomeAssistantSnapcast = hass.data[DOMAIN][config_entry.entry_id] register_services() @@ -73,14 +74,14 @@ async def async_setup_entry( port = config_entry.data[CONF_PORT] hpid = f"{host}:{port}" - snapcast_data.groups: list[MediaPlayerEntity] = [ + snapcast_data.groups = [ SnapcastGroupDevice(group, hpid) for group in snapcast_data.server.groups ] - snapcast_data.clients: list[MediaPlayerEntity] = [ + snapcast_data.clients = [ SnapcastClientDevice(client, hpid, config_entry.entry_id) for client in snapcast_data.server.clients ] - async_add_entities(clients + groups) + async_add_entities(snapcast_data.clients + snapcast_data.groups) async def async_setup_platform( diff --git a/homeassistant/components/snapcast/server.py b/homeassistant/components/snapcast/server.py new file mode 100644 index 00000000000000..a8aefd55c00f35 --- /dev/null +++ b/homeassistant/components/snapcast/server.py @@ -0,0 +1,15 @@ +"""Snapcast Integration.""" +from dataclasses import dataclass + +from snapcast.control import Snapserver + +from homeassistant.components.media_player import MediaPlayerEntity + + +@dataclass +class HomeAssistantSnapcast: + """Snapcast data stored in the Home Assistant data object.""" + + server: Snapserver + clients: list[MediaPlayerEntity] + groups: list[MediaPlayerEntity] From 7df4d7ac16eb6030db05b03906dd7455abea70be Mon Sep 17 00:00:00 2001 From: raul Date: Tue, 28 Mar 2023 21:32:37 +0200 Subject: [PATCH 18/20] Split import flow from user flow. Fix tests. --- homeassistant/components/snapcast/__init__.py | 3 +-- homeassistant/components/snapcast/config_flow.py | 8 +++++++- tests/components/snapcast/test_config_flow.py | 7 +++++-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/homeassistant/components/snapcast/__init__.py b/homeassistant/components/snapcast/__init__.py index c4259c4457a84d..66cda9d5b584be 100644 --- a/homeassistant/components/snapcast/__init__.py +++ b/homeassistant/components/snapcast/__init__.py @@ -1,6 +1,5 @@ """Snapcast Integration.""" import logging -import socket import snapcast.control @@ -23,7 +22,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: server = await snapcast.control.create_server( hass.loop, host, port, reconnect=True ) - except socket.gaierror as ex: + except OSError as ex: raise ConfigEntryNotReady( f"Could not connect to Snapcast server at {host}:{port}" ) from ex diff --git a/homeassistant/components/snapcast/config_flow.py b/homeassistant/components/snapcast/config_flow.py index 19dfd8690df3b5..29bf84afba287f 100644 --- a/homeassistant/components/snapcast/config_flow.py +++ b/homeassistant/components/snapcast/config_flow.py @@ -54,4 +54,10 @@ async def async_step_user(self, user_input=None) -> FlowResult: async def async_step_import(self, import_config: dict[str, str]) -> FlowResult: """Import a config entry from configuration.yaml.""" - return await self.async_step_user(import_config) + self._async_abort_entries_match( + { + CONF_HOST: (host := import_config[CONF_HOST]), + CONF_PORT: (port := import_config[CONF_PORT]), + } + ) + return self.async_create_entry(title=f"{host}:{port}", data=import_config) diff --git a/tests/components/snapcast/test_config_flow.py b/tests/components/snapcast/test_config_flow.py index 8e69f18009885c..2909f74f3f46a5 100644 --- a/tests/components/snapcast/test_config_flow.py +++ b/tests/components/snapcast/test_config_flow.py @@ -27,6 +27,7 @@ async def test_form( DOMAIN, context={"source": config_entries.SOURCE_USER} ) assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "user" assert not result["errors"] # test invalid host error @@ -38,6 +39,7 @@ async def test_form( await hass.async_block_till_done() assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "user" assert result["errors"] == {"base": "invalid_host"} # test connection error @@ -49,6 +51,7 @@ async def test_form( await hass.async_block_till_done() assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "user" assert result["errors"] == {"base": "cannot_connect"} # test success @@ -81,6 +84,7 @@ async def test_abort( DOMAIN, context={"source": config_entries.SOURCE_USER} ) assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "user" assert not result["errors"] with patch("snapcast.control.create_server", side_effect=socket.gaierror): @@ -94,7 +98,7 @@ async def test_abort( assert result["reason"] == "already_configured" -async def test_import(hass: HomeAssistant, mock_create_server: AsyncMock) -> None: +async def test_import(hass: HomeAssistant) -> None: """Test successful import.""" result = await hass.config_entries.flow.async_init( @@ -109,4 +113,3 @@ async def test_import(hass: HomeAssistant, mock_create_server: AsyncMock) -> Non result["title"] == f"{TEST_CONNECTION[CONF_HOST]}:{TEST_CONNECTION[CONF_PORT]}" ) assert result["data"] == TEST_CONNECTION - assert len(mock_create_server.mock_calls) == 1 From a1ec7369d3be0c9505d7dd3e7c2412d25ea54f2d Mon Sep 17 00:00:00 2001 From: raul Date: Wed, 29 Mar 2023 19:18:20 +0200 Subject: [PATCH 19/20] Use explicit asserts. Add default values to dataclass --- homeassistant/components/snapcast/__init__.py | 4 +--- homeassistant/components/snapcast/server.py | 6 +++--- tests/components/snapcast/test_config_flow.py | 13 ++++--------- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/homeassistant/components/snapcast/__init__.py b/homeassistant/components/snapcast/__init__.py index 66cda9d5b584be..309669a8496826 100644 --- a/homeassistant/components/snapcast/__init__.py +++ b/homeassistant/components/snapcast/__init__.py @@ -27,9 +27,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: f"Could not connect to Snapcast server at {host}:{port}" ) from ex - hass.data.setdefault(DOMAIN, {})[entry.entry_id] = HomeAssistantSnapcast( - server, [], [] - ) + hass.data.setdefault(DOMAIN, {})[entry.entry_id] = HomeAssistantSnapcast(server) await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) diff --git a/homeassistant/components/snapcast/server.py b/homeassistant/components/snapcast/server.py index a8aefd55c00f35..507ad6393a2e42 100644 --- a/homeassistant/components/snapcast/server.py +++ b/homeassistant/components/snapcast/server.py @@ -1,5 +1,5 @@ """Snapcast Integration.""" -from dataclasses import dataclass +from dataclasses import dataclass, field from snapcast.control import Snapserver @@ -11,5 +11,5 @@ class HomeAssistantSnapcast: """Snapcast data stored in the Home Assistant data object.""" server: Snapserver - clients: list[MediaPlayerEntity] - groups: list[MediaPlayerEntity] + clients: list[MediaPlayerEntity] = field(default_factory=list) + groups: list[MediaPlayerEntity] = field(default_factory=list) diff --git a/tests/components/snapcast/test_config_flow.py b/tests/components/snapcast/test_config_flow.py index 2909f74f3f46a5..cfd0ba449d1d17 100644 --- a/tests/components/snapcast/test_config_flow.py +++ b/tests/components/snapcast/test_config_flow.py @@ -61,10 +61,8 @@ async def test_form( await hass.async_block_till_done() assert result["type"] == FlowResultType.CREATE_ENTRY - assert ( - result["title"] == f"{TEST_CONNECTION[CONF_HOST]}:{TEST_CONNECTION[CONF_PORT]}" - ) - assert result["data"] == TEST_CONNECTION + assert result["title"] == "snapserver.test:1705" + assert result["data"] == {CONF_HOST: "snapserver.test", CONF_PORT: 1705} assert len(mock_create_server.mock_calls) == 1 assert len(mock_setup_entry.mock_calls) == 1 @@ -75,7 +73,6 @@ async def test_abort( """Test config flow abort if device is already configured.""" entry = MockConfigEntry( domain=DOMAIN, - title=f"{TEST_CONNECTION[CONF_HOST]}:{TEST_CONNECTION[CONF_PORT]}", data=TEST_CONNECTION, ) entry.add_to_hass(hass) @@ -109,7 +106,5 @@ async def test_import(hass: HomeAssistant) -> None: await hass.async_block_till_done() assert result["type"] == FlowResultType.CREATE_ENTRY - assert ( - result["title"] == f"{TEST_CONNECTION[CONF_HOST]}:{TEST_CONNECTION[CONF_PORT]}" - ) - assert result["data"] == TEST_CONNECTION + assert result["title"] == "snapserver.test:1705" + assert result["data"] == {CONF_HOST: "snapserver.test", CONF_PORT: 1705} From 8304c1410297ad2eec11793ca8d4c3a88b83d652 Mon Sep 17 00:00:00 2001 From: raul Date: Wed, 29 Mar 2023 20:48:44 +0200 Subject: [PATCH 20/20] Change entry title to Snapcast --- homeassistant/components/snapcast/config_flow.py | 10 +++++----- homeassistant/components/snapcast/const.py | 1 + tests/components/snapcast/test_config_flow.py | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/snapcast/config_flow.py b/homeassistant/components/snapcast/config_flow.py index 29bf84afba287f..896d3f8b5a8ace 100644 --- a/homeassistant/components/snapcast/config_flow.py +++ b/homeassistant/components/snapcast/config_flow.py @@ -13,7 +13,7 @@ from homeassistant.const import CONF_HOST, CONF_PORT from homeassistant.data_entry_flow import FlowResult -from .const import DOMAIN +from .const import DEFAULT_TITLE, DOMAIN _LOGGER = logging.getLogger(__name__) @@ -47,7 +47,7 @@ async def async_step_user(self, user_input=None) -> FlowResult: errors["base"] = "cannot_connect" else: await client.stop() - return self.async_create_entry(title=f"{host}:{port}", data=user_input) + return self.async_create_entry(title=DEFAULT_TITLE, data=user_input) return self.async_show_form( step_id="user", data_schema=SNAPCAST_SCHEMA, errors=errors ) @@ -56,8 +56,8 @@ async def async_step_import(self, import_config: dict[str, str]) -> FlowResult: """Import a config entry from configuration.yaml.""" self._async_abort_entries_match( { - CONF_HOST: (host := import_config[CONF_HOST]), - CONF_PORT: (port := import_config[CONF_PORT]), + CONF_HOST: (import_config[CONF_HOST]), + CONF_PORT: (import_config[CONF_PORT]), } ) - return self.async_create_entry(title=f"{host}:{port}", data=import_config) + return self.async_create_entry(title=DEFAULT_TITLE, data=import_config) diff --git a/homeassistant/components/snapcast/const.py b/homeassistant/components/snapcast/const.py index b0033c0c7c7168..ded57e6fb037ea 100644 --- a/homeassistant/components/snapcast/const.py +++ b/homeassistant/components/snapcast/const.py @@ -18,3 +18,4 @@ ATTR_LATENCY = "latency" DOMAIN = "snapcast" +DEFAULT_TITLE = "Snapcast" diff --git a/tests/components/snapcast/test_config_flow.py b/tests/components/snapcast/test_config_flow.py index cfd0ba449d1d17..b6ff43503a6328 100644 --- a/tests/components/snapcast/test_config_flow.py +++ b/tests/components/snapcast/test_config_flow.py @@ -61,7 +61,7 @@ async def test_form( await hass.async_block_till_done() assert result["type"] == FlowResultType.CREATE_ENTRY - assert result["title"] == "snapserver.test:1705" + assert result["title"] == "Snapcast" assert result["data"] == {CONF_HOST: "snapserver.test", CONF_PORT: 1705} assert len(mock_create_server.mock_calls) == 1 assert len(mock_setup_entry.mock_calls) == 1 @@ -106,5 +106,5 @@ async def test_import(hass: HomeAssistant) -> None: await hass.async_block_till_done() assert result["type"] == FlowResultType.CREATE_ENTRY - assert result["title"] == "snapserver.test:1705" + assert result["title"] == "Snapcast" assert result["data"] == {CONF_HOST: "snapserver.test", CONF_PORT: 1705}