diff --git a/homeassistant/components/harmony/config_flow.py b/homeassistant/components/harmony/config_flow.py index ff7b47d6010..9d9c9dfb8e9 100644 --- a/homeassistant/components/harmony/config_flow.py +++ b/homeassistant/components/harmony/config_flow.py @@ -2,11 +2,9 @@ import logging from urllib.parse import urlparse -import aioharmony.exceptions as harmony_exceptions -from aioharmony.harmonyapi import HarmonyAPI import voluptuous as vol -from homeassistant import config_entries, core, exceptions +from homeassistant import config_entries, exceptions from homeassistant.components import ssdp from homeassistant.components.remote import ( ATTR_ACTIVITY, @@ -17,7 +15,11 @@ from homeassistant.const import CONF_HOST, CONF_NAME from homeassistant.core import callback from .const import DOMAIN, UNIQUE_ID -from .util import find_unique_id_for_remote +from .util import ( + find_best_name_for_remote, + find_unique_id_for_remote, + get_harmony_client_if_available, +) _LOGGER = logging.getLogger(__name__) @@ -26,43 +28,19 @@ DATA_SCHEMA = vol.Schema( ) -async def get_harmony_client_if_available(hass: core.HomeAssistant, ip_address): - """Connect to a harmony hub and fetch info.""" - harmony = HarmonyAPI(ip_address=ip_address) - - try: - if not await harmony.connect(): - await harmony.close() - return None - except harmony_exceptions.TimeOut: - return None - - await harmony.close() - - return harmony - - -async def validate_input(hass: core.HomeAssistant, data): +async def validate_input(data): """Validate the user input allows us to connect. Data has the keys from DATA_SCHEMA with values provided by the user. """ - harmony = await get_harmony_client_if_available(hass, data[CONF_HOST]) + harmony = await get_harmony_client_if_available(data[CONF_HOST]) if not harmony: raise CannotConnect - unique_id = find_unique_id_for_remote(harmony) - - # As a last resort we get the name from the harmony client - # in the event a name was not provided. harmony.name is - # usually the ip address but it can be an empty string. - if CONF_NAME not in data or data[CONF_NAME] is None or data[CONF_NAME] == "": - data[CONF_NAME] = harmony.name - return { - CONF_NAME: data[CONF_NAME], + CONF_NAME: find_best_name_for_remote(data, harmony), CONF_HOST: data[CONF_HOST], - UNIQUE_ID: unique_id, + UNIQUE_ID: find_unique_id_for_remote(harmony), } @@ -82,7 +60,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): if user_input is not None: try: - validated = await validate_input(self.hass, user_input) + validated = await validate_input(user_input) except CannotConnect: errors["base"] = "cannot_connect" except Exception: # pylint: disable=broad-except @@ -116,9 +94,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): CONF_NAME: friendly_name, } - harmony = await get_harmony_client_if_available( - self.hass, self.harmony_config[CONF_HOST] - ) + harmony = await get_harmony_client_if_available(parsed_url.hostname) if harmony: unique_id = find_unique_id_for_remote(harmony) @@ -150,9 +126,15 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): }, ) - async def async_step_import(self, user_input): + async def async_step_import(self, validated_input): """Handle import.""" - return await self.async_step_user(user_input) + await self.async_set_unique_id(validated_input[UNIQUE_ID]) + self._abort_if_unique_id_configured() + # Everything was validated in remote async_setup_platform + # all we do now is create. + return await self._async_create_entry_from_valid_input( + validated_input, validated_input + ) @staticmethod @callback diff --git a/homeassistant/components/harmony/remote.py b/homeassistant/components/harmony/remote.py index 024af9b1580..5af8d1eb65a 100644 --- a/homeassistant/components/harmony/remote.py +++ b/homeassistant/components/harmony/remote.py @@ -24,6 +24,7 @@ from homeassistant.components.remote import ( from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry from homeassistant.const import ATTR_ENTITY_ID, CONF_HOST, CONF_NAME from homeassistant.core import HomeAssistant +from homeassistant.exceptions import PlatformNotReady import homeassistant.helpers.config_validation as cv from homeassistant.helpers.dispatcher import async_dispatcher_connect @@ -33,6 +34,13 @@ from .const import ( HARMONY_OPTIONS_UPDATE, SERVICE_CHANGE_CHANNEL, SERVICE_SYNC, + UNIQUE_ID, +) +from .util import ( + find_best_name_for_remote, + find_matching_config_entries_for_host, + find_unique_id_for_remote, + get_harmony_client_if_available, ) _LOGGER = logging.getLogger(__name__) @@ -51,6 +59,7 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( extra=vol.ALLOW_EXTRA, ) + HARMONY_SYNC_SCHEMA = vol.Schema({vol.Optional(ATTR_ENTITY_ID): cv.entity_ids}) HARMONY_CHANGE_CHANNEL_SCHEMA = vol.Schema( @@ -68,9 +77,25 @@ async def async_setup_platform(hass, config, async_add_entities, discovery_info= # Now handled by ssdp in the config flow return + if find_matching_config_entries_for_host(hass, config[CONF_HOST]): + return + + # We do the validation to verify we can connect + # so we can raise PlatformNotReady to force + # a retry so we can avoid a scenario where the config + # entry cannot be created via import because hub + # is not yet ready. + harmony = await get_harmony_client_if_available(config[CONF_HOST]) + if not harmony: + raise PlatformNotReady + + validated_config = config.copy() + validated_config[UNIQUE_ID] = find_unique_id_for_remote(harmony) + validated_config[CONF_NAME] = find_best_name_for_remote(config, harmony) + hass.async_create_task( hass.config_entries.flow.async_init( - DOMAIN, context={"source": SOURCE_IMPORT}, data=config + DOMAIN, context={"source": SOURCE_IMPORT}, data=validated_config ) ) diff --git a/homeassistant/components/harmony/util.py b/homeassistant/components/harmony/util.py index 5f7e46510f9..69ed44cb7da 100644 --- a/homeassistant/components/harmony/util.py +++ b/homeassistant/components/harmony/util.py @@ -1,8 +1,13 @@ """The Logitech Harmony Hub integration utils.""" -from aioharmony.harmonyapi import HarmonyAPI as HarmonyClient +import aioharmony.exceptions as harmony_exceptions +from aioharmony.harmonyapi import HarmonyAPI + +from homeassistant.const import CONF_HOST, CONF_NAME + +from .const import DOMAIN -def find_unique_id_for_remote(harmony: HarmonyClient): +def find_unique_id_for_remote(harmony: HarmonyAPI): """Find the unique id for both websocket and xmpp clients.""" websocket_unique_id = harmony.hub_config.info.get("activeRemoteId") if websocket_unique_id is not None: @@ -10,3 +15,38 @@ def find_unique_id_for_remote(harmony: HarmonyClient): # fallback to the xmpp unique id if websocket is not available return harmony.config["global"]["timeStampHash"].split(";")[-1] + + +def find_best_name_for_remote(data: dict, harmony: HarmonyAPI): + """Find the best name from config or fallback to the remote.""" + # As a last resort we get the name from the harmony client + # in the event a name was not provided. harmony.name is + # usually the ip address but it can be an empty string. + if CONF_NAME not in data or data[CONF_NAME] is None or data[CONF_NAME] == "": + return harmony.name + + return data[CONF_NAME] + + +async def get_harmony_client_if_available(ip_address: str): + """Connect to a harmony hub and fetch info.""" + harmony = HarmonyAPI(ip_address=ip_address) + + try: + if not await harmony.connect(): + await harmony.close() + return None + except harmony_exceptions.TimeOut: + return None + + await harmony.close() + + return harmony + + +def find_matching_config_entries_for_host(hass, host): + """Search existing config entries for one matching the host.""" + for entry in hass.config_entries.async_entries(DOMAIN): + if entry.data[CONF_HOST] == host: + return entry + return None diff --git a/tests/components/harmony/test_config_flow.py b/tests/components/harmony/test_config_flow.py index 18c0825b6a1..30421756d22 100644 --- a/tests/components/harmony/test_config_flow.py +++ b/tests/components/harmony/test_config_flow.py @@ -25,8 +25,7 @@ async def test_user_form(hass): harmonyapi = _get_mock_harmonyapi(connect=True) with patch( - "homeassistant.components.harmony.config_flow.HarmonyAPI", - return_value=harmonyapi, + "homeassistant.components.harmony.util.HarmonyAPI", return_value=harmonyapi, ), patch( "homeassistant.components.harmony.async_setup", return_value=True ) as mock_setup, patch( @@ -53,8 +52,7 @@ async def test_form_import(hass): harmonyapi = _get_mock_harmonyapi(connect=True) with patch( - "homeassistant.components.harmony.config_flow.HarmonyAPI", - return_value=harmonyapi, + "homeassistant.components.harmony.util.HarmonyAPI", return_value=harmonyapi, ), patch( "homeassistant.components.harmony.async_setup", return_value=True ) as mock_setup, patch( @@ -68,9 +66,11 @@ async def test_form_import(hass): "name": "friend", "activity": "Watch TV", "delay_secs": 0.9, + "unique_id": "555234534543", }, ) + assert result["result"].unique_id == "555234534543" assert result["type"] == "create_entry" assert result["title"] == "friend" assert result["data"] == { @@ -94,8 +94,7 @@ async def test_form_ssdp(hass): harmonyapi = _get_mock_harmonyapi(connect=True) with patch( - "homeassistant.components.harmony.config_flow.HarmonyAPI", - return_value=harmonyapi, + "homeassistant.components.harmony.util.HarmonyAPI", return_value=harmonyapi, ): result = await hass.config_entries.flow.async_init( DOMAIN, @@ -114,8 +113,7 @@ async def test_form_ssdp(hass): } with patch( - "homeassistant.components.harmony.config_flow.HarmonyAPI", - return_value=harmonyapi, + "homeassistant.components.harmony.util.HarmonyAPI", return_value=harmonyapi, ), patch( "homeassistant.components.harmony.async_setup", return_value=True ) as mock_setup, patch( @@ -141,8 +139,7 @@ async def test_form_cannot_connect(hass): ) with patch( - "homeassistant.components.harmony.config_flow.HarmonyAPI", - side_effect=CannotConnect, + "homeassistant.components.harmony.util.HarmonyAPI", side_effect=CannotConnect, ): result2 = await hass.config_entries.flow.async_configure( result["flow_id"],