diff --git a/homeassistant/components/roon/__init__.py b/homeassistant/components/roon/__init__.py index 133598070c2..9e5c38f0211 100644 --- a/homeassistant/components/roon/__init__.py +++ b/homeassistant/components/roon/__init__.py @@ -4,14 +4,17 @@ from homeassistant.const import CONF_HOST from homeassistant.core import HomeAssistant from homeassistant.helpers import device_registry as dr -from .const import DOMAIN +from .const import CONF_ROON_NAME, DOMAIN from .server import RoonServer async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up a roonserver from a config entry.""" hass.data.setdefault(DOMAIN, {}) - host = entry.data[CONF_HOST] + + # fallback to using host for compatibility with older configs + name = entry.data.get(CONF_ROON_NAME, entry.data[CONF_HOST]) + roonserver = RoonServer(hass, entry) if not await roonserver.async_setup(): @@ -23,7 +26,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: config_entry_id=entry.entry_id, identifiers={(DOMAIN, entry.entry_id)}, manufacturer="Roonlabs", - name=host, + name=f"Roon Core ({name})", ) return True diff --git a/homeassistant/components/roon/config_flow.py b/homeassistant/components/roon/config_flow.py index 31391a0ff36..6ccf97155c4 100644 --- a/homeassistant/components/roon/config_flow.py +++ b/homeassistant/components/roon/config_flow.py @@ -6,11 +6,13 @@ from roonapi import RoonApi, RoonDiscovery import voluptuous as vol from homeassistant import config_entries, core, exceptions -from homeassistant.const import CONF_API_KEY, CONF_HOST +from homeassistant.const import CONF_API_KEY, CONF_HOST, CONF_PORT +import homeassistant.helpers.config_validation as cv from .const import ( AUTHENTICATE_TIMEOUT, CONF_ROON_ID, + CONF_ROON_NAME, DEFAULT_NAME, DOMAIN, ROON_APPINFO, @@ -18,7 +20,12 @@ from .const import ( _LOGGER = logging.getLogger(__name__) -DATA_SCHEMA = vol.Schema({vol.Required("host"): str}) +DATA_SCHEMA = vol.Schema( + { + vol.Required("host"): cv.string, + vol.Required("port", default=9330): cv.port, + } +) TIMEOUT = 120 @@ -45,7 +52,7 @@ class RoonHub: _LOGGER.debug("Servers = %s", servers) return servers - async def authenticate(self, host, servers): + async def authenticate(self, host, port, servers): """Authenticate with one or more roon servers.""" def stop_apis(apis): @@ -54,6 +61,7 @@ class RoonHub: token = None core_id = None + core_name = None secs = 0 if host is None: apis = [ @@ -61,7 +69,7 @@ class RoonHub: for server in servers ] else: - apis = [RoonApi(ROON_APPINFO, None, host, blocking_init=False)] + apis = [RoonApi(ROON_APPINFO, None, host, port, blocking_init=False)] while secs <= TIMEOUT: # Roon can discover multiple devices - not all of which are proper servers, so try and authenticate with them all. @@ -71,6 +79,7 @@ class RoonHub: secs += AUTHENTICATE_TIMEOUT if auth_api: core_id = auth_api[0].core_id + core_name = auth_api[0].core_name token = auth_api[0].token break @@ -78,7 +87,7 @@ class RoonHub: await self._hass.async_add_executor_job(stop_apis, apis) - return (token, core_id) + return (token, core_id, core_name) async def discover(hass): @@ -90,15 +99,21 @@ async def discover(hass): return servers -async def authenticate(hass: core.HomeAssistant, host, servers): +async def authenticate(hass: core.HomeAssistant, host, port, servers): """Connect and authenticate home assistant.""" hub = RoonHub(hass) - (token, core_id) = await hub.authenticate(host, servers) + (token, core_id, core_name) = await hub.authenticate(host, port, servers) if token is None: raise InvalidAuth - return {CONF_HOST: host, CONF_ROON_ID: core_id, CONF_API_KEY: token} + return { + CONF_HOST: host, + CONF_PORT: port, + CONF_ROON_ID: core_id, + CONF_ROON_NAME: core_name, + CONF_API_KEY: token, + } class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): @@ -109,33 +124,45 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): def __init__(self): """Initialize the Roon flow.""" self._host = None + self._port = None self._servers = [] async def async_step_user(self, user_input=None): - """Handle getting host details from the user.""" + """Get roon core details via discovery.""" - errors = {} self._servers = await discover(self.hass) # We discovered one or more roon - so skip to authentication if self._servers: return await self.async_step_link() + return await self.async_step_fallback() + + async def async_step_fallback(self, user_input=None): + """Get host and port details from the user.""" + errors = {} + if user_input is not None: self._host = user_input["host"] + self._port = user_input["port"] return await self.async_step_link() return self.async_show_form( - step_id="user", data_schema=DATA_SCHEMA, errors=errors + step_id="fallback", data_schema=DATA_SCHEMA, errors=errors ) async def async_step_link(self, user_input=None): """Handle linking and authenticting with the roon server.""" - errors = {} if user_input is not None: + # Do not authenticate if the host is already configured + self._async_abort_entries_match({CONF_HOST: self._host}) + try: - info = await authenticate(self.hass, self._host, self._servers) + info = await authenticate( + self.hass, self._host, self._port, self._servers + ) + except InvalidAuth: errors["base"] = "invalid_auth" except Exception: # pylint: disable=broad-except diff --git a/homeassistant/components/roon/const.py b/homeassistant/components/roon/const.py index 7c9cd6c4999..74cf6a38160 100644 --- a/homeassistant/components/roon/const.py +++ b/homeassistant/components/roon/const.py @@ -5,6 +5,7 @@ AUTHENTICATE_TIMEOUT = 5 DOMAIN = "roon" CONF_ROON_ID = "roon_server_id" +CONF_ROON_NAME = "roon_server_name" DATA_CONFIGS = "roon_configs" diff --git a/homeassistant/components/roon/manifest.json b/homeassistant/components/roon/manifest.json index a3b22a3c2cc..297bf5e9d7a 100644 --- a/homeassistant/components/roon/manifest.json +++ b/homeassistant/components/roon/manifest.json @@ -3,7 +3,7 @@ "name": "RoonLabs music player", "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/roon", - "requirements": ["roonapi==0.0.38"], + "requirements": ["roonapi==0.1.1"], "codeowners": ["@pavoni"], "iot_class": "local_push", "loggers": ["roonapi"] diff --git a/homeassistant/components/roon/server.py b/homeassistant/components/roon/server.py index 8301bf73fdf..d5e4cded08d 100644 --- a/homeassistant/components/roon/server.py +++ b/homeassistant/components/roon/server.py @@ -2,15 +2,16 @@ import asyncio import logging -from roonapi import RoonApi +from roonapi import RoonApi, RoonDiscovery -from homeassistant.const import CONF_API_KEY, CONF_HOST, Platform +from homeassistant.const import CONF_API_KEY, CONF_HOST, CONF_PORT, Platform from homeassistant.helpers.dispatcher import async_dispatcher_send from homeassistant.util.dt import utcnow from .const import CONF_ROON_ID, ROON_APPINFO _LOGGER = logging.getLogger(__name__) +INITIAL_SYNC_INTERVAL = 5 FULL_SYNC_INTERVAL = 30 PLATFORMS = [Platform.MEDIA_PLAYER] @@ -33,23 +34,38 @@ class RoonServer: async def async_setup(self, tries=0): """Set up a roon server based on config parameters.""" - hass = self.hass - # Host will be None for configs using discovery - host = self.config_entry.data[CONF_HOST] - token = self.config_entry.data[CONF_API_KEY] - # Default to None for compatibility with older configs - core_id = self.config_entry.data.get(CONF_ROON_ID) - _LOGGER.debug("async_setup: host=%s core_id=%s token=%s", host, core_id, token) - self.roonapi = RoonApi( - ROON_APPINFO, token, host, blocking_init=False, core_id=core_id - ) + def get_roon_host(): + host = self.config_entry.data.get(CONF_HOST) + port = self.config_entry.data.get(CONF_PORT) + if host: + _LOGGER.debug("static roon core host=%s port=%s", host, port) + return (host, port) + + discover = RoonDiscovery(core_id) + server = discover.first() + discover.stop() + _LOGGER.debug("dynamic roon core core_id=%s server=%s", core_id, server) + return (server[0], server[1]) + + def get_roon_api(): + token = self.config_entry.data[CONF_API_KEY] + (host, port) = get_roon_host() + return RoonApi(ROON_APPINFO, token, host, port, blocking_init=True) + + hass = self.hass + core_id = self.config_entry.data.get(CONF_ROON_ID) + + self.roonapi = await self.hass.async_add_executor_job(get_roon_api) + self.roonapi.register_state_callback( self.roonapi_state_callback, event_filter=["zones_changed"] ) # Default to 'host' for compatibility with older configs without core_id - self.roon_id = core_id if core_id is not None else host + self.roon_id = ( + core_id if core_id is not None else self.config_entry.data[CONF_HOST] + ) # initialize media_player platform hass.config_entries.async_setup_platforms(self.config_entry, PLATFORMS) @@ -98,13 +114,14 @@ class RoonServer: async def async_do_loop(self): """Background work loop.""" self._exit = False + await asyncio.sleep(INITIAL_SYNC_INTERVAL) while not self._exit: await self.async_update_players() - # await self.async_update_playlists() await asyncio.sleep(FULL_SYNC_INTERVAL) async def async_update_changed_players(self, changed_zones_ids): """Update the players which were reported as changed by the Roon API.""" + _LOGGER.debug("async_update_changed_players %s", changed_zones_ids) for zone_id in changed_zones_ids: if zone_id not in self.roonapi.zones: # device was removed ? @@ -127,6 +144,7 @@ class RoonServer: async def async_update_players(self): """Periodic full scan of all devices.""" zone_ids = self.roonapi.zones.keys() + _LOGGER.debug("async_update_players %s", zone_ids) await self.async_update_changed_players(zone_ids) # check for any removed devices all_devs = {} diff --git a/homeassistant/components/roon/strings.json b/homeassistant/components/roon/strings.json index 565a66a1320..ce5827e2c6c 100644 --- a/homeassistant/components/roon/strings.json +++ b/homeassistant/components/roon/strings.json @@ -1,10 +1,12 @@ { "config": { "step": { - "user": { - "description": "Could not discover Roon server, please enter your the Hostname or IP.", + "user": {}, + "fallback": { + "description": "Could not discover Roon server, please enter your Hostname and Port.", "data": { - "host": "[%key:common::config_flow::data::host%]" + "host": "[%key:common::config_flow::data::host%]", + "port": "[%key:common::config_flow::data::port%]" } }, "link": { diff --git a/requirements_all.txt b/requirements_all.txt index d39b7e129b2..0e414214df4 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -2076,7 +2076,7 @@ rokuecp==0.16.0 roombapy==1.6.5 # homeassistant.components.roon -roonapi==0.0.38 +roonapi==0.1.1 # homeassistant.components.rova rova==0.3.0 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index e75125ac7d3..34be9e90fa6 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -1351,7 +1351,7 @@ rokuecp==0.16.0 roombapy==1.6.5 # homeassistant.components.roon -roonapi==0.0.38 +roonapi==0.1.1 # homeassistant.components.rpi_power rpi-bad-power==0.1.0 diff --git a/tests/components/roon/test_config_flow.py b/tests/components/roon/test_config_flow.py index 686109f968e..7cc37bc73cd 100644 --- a/tests/components/roon/test_config_flow.py +++ b/tests/components/roon/test_config_flow.py @@ -1,9 +1,11 @@ """Test the roon config flow.""" from unittest.mock import patch -from homeassistant import config_entries +from homeassistant import config_entries, data_entry_flow from homeassistant.components.roon.const import DOMAIN +from tests.common import MockConfigEntry + class RoonApiMock: """Class to mock the Roon API for testing.""" @@ -18,8 +20,13 @@ class RoonApiMock: """Return the roon host.""" return "core_id" + @property + def core_name(self): + """Return the roon core name.""" + return "Roon Core" + def stop(self): - """Stop socket and discovery.""" + """Stop socket.""" return @@ -93,8 +100,10 @@ async def test_successful_discovery_and_auth(hass): assert result2["title"] == "Roon Labs Music Player" assert result2["data"] == { "host": None, + "port": None, "api_key": "good_token", "roon_server_id": "core_id", + "roon_server_name": "Roon Core", } @@ -119,11 +128,11 @@ async def test_unsuccessful_discovery_user_form_and_auth(hass): # Should show the form if server was not discovered assert result["type"] == "form" - assert result["step_id"] == "user" + assert result["step_id"] == "fallback" assert result["errors"] == {} await hass.config_entries.flow.async_configure( - result["flow_id"], {"host": "1.1.1.1"} + result["flow_id"], {"host": "1.1.1.1", "port": 9331} ) result2 = await hass.config_entries.flow.async_configure( result["flow_id"], user_input={} @@ -134,10 +143,52 @@ async def test_unsuccessful_discovery_user_form_and_auth(hass): assert result2["data"] == { "host": "1.1.1.1", "api_key": "good_token", + "port": 9331, + "api_key": "good_token", "roon_server_id": "core_id", + "roon_server_name": "Roon Core", } +async def test_duplicate_config(hass): + """Test user adding the host via the form for host that is already configured.""" + + CONFIG = {"host": "1.1.1.1"} + + MockConfigEntry(domain=DOMAIN, unique_id="0123456789", data=CONFIG).add_to_hass( + hass + ) + + with patch( + "homeassistant.components.roon.config_flow.RoonApi", + return_value=RoonApiMock(), + ), patch( + "homeassistant.components.roon.config_flow.RoonDiscovery", + return_value=RoonDiscoveryFailedMock(), + ): + + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + await hass.async_block_till_done() + + # Should show the form if server was not discovered + assert result["type"] == "form" + assert result["step_id"] == "fallback" + assert result["errors"] == {} + + await hass.config_entries.flow.async_configure( + result["flow_id"], {"host": "1.1.1.1", "port": 9331} + ) + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input={} + ) + await hass.async_block_till_done() + + assert result2["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result2["reason"] == "already_configured" + + async def test_successful_discovery_no_auth(hass): """Test successful discover, but failed auth."""