Ensure harmony hub is ready before importing

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.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 @@
 )
 
 
-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 @@ async def async_step_user(self, user_input=None):
         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 @@ async def async_step_ssdp(self, discovery_info):
             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 @@ async def async_step_link(self, user_input=None):
             },
         )
 
-    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

homeassistant/components/harmony/remote.py

@@ -24,6 +24,7 @@
 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 @@
     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 @@
     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
         )
     )
 

homeassistant/components/harmony/util.py

@@ -1,12 +1,52 @@
 """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
 
-def find_unique_id_for_remote(harmony: HarmonyClient):
+from .const import DOMAIN
+
+
+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:
         return websocket_unique_id
 
     # 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

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"],