Ensure harmony hub is ready before importing (#33537)

If the harmony hub was not ready for connection or
was busy when importing from yaml, the import validation
would fail would not be retried.

To mitigate this scenario we now do the validation in
async_setup_platform which allows us to raise
PlatformNotReady so we can retry later.

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

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
         )
     )
 

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

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