Add unique id to esphome config flow (#34753)

homeassistant/components/esphome/config_flow.py

@@ -5,18 +5,21 @@ from typing import Optional
 from aioesphomeapi import APIClient, APIConnectionError
 import voluptuous as vol
 
-from homeassistant import config_entries, core
+from homeassistant.config_entries import CONN_CLASS_LOCAL_PUSH, ConfigFlow
-from homeassistant.helpers.typing import ConfigType
+from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PASSWORD, CONF_PORT
+from homeassistant.core import callback
+from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType
 
 from .entry_data import DATA_KEY, RuntimeEntryData
 
+DOMAIN = "esphome"
 
-@config_entries.HANDLERS.register("esphome")
+
-class EsphomeFlowHandler(config_entries.ConfigFlow):
+class EsphomeFlowHandler(ConfigFlow, domain=DOMAIN):
     """Handle a esphome config flow."""
 
     VERSION = 1
-    CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_PUSH
+    CONNECTION_CLASS = CONN_CLASS_LOCAL_PUSH
 
     def __init__(self):
         """Initialize flow."""
@@ -32,8 +35,8 @@ class EsphomeFlowHandler(config_entries.ConfigFlow):
             return await self._async_authenticate_or_add(user_input)
 
         fields = OrderedDict()
-        fields[vol.Required("host", default=self._host or vol.UNDEFINED)] = str
+        fields[vol.Required(CONF_HOST, default=self._host or vol.UNDEFINED)] = str
-        fields[vol.Optional("port", default=self._port or 6053)] = int
+        fields[vol.Optional(CONF_PORT, default=self._port or 6053)] = int
 
         errors = {}
         if error is not None:
@@ -46,19 +49,19 @@ class EsphomeFlowHandler(config_entries.ConfigFlow):
     @property
     def _name(self):
         # pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167
-        return self.context.get("name")
+        return self.context.get(CONF_NAME)
 
     @_name.setter
     def _name(self, value):
         # pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167
-        self.context["name"] = value
+        self.context[CONF_NAME] = value
         self.context["title_placeholders"] = {"name": self._name}
 
     def _set_user_input(self, user_input):
         if user_input is None:
             return
-        self._host = user_input["host"]
+        self._host = user_input[CONF_HOST]
-        self._port = user_input["port"]
+        self._port = user_input[CONF_PORT]
 
     async def _async_authenticate_or_add(self, user_input):
         self._set_user_input(user_input)
@@ -81,56 +84,69 @@ class EsphomeFlowHandler(config_entries.ConfigFlow):
             step_id="discovery_confirm", description_placeholders={"name": self._name}
         )
 
-    async def async_step_zeroconf(self, user_input: ConfigType):
+    async def async_step_zeroconf(self, discovery_info: DiscoveryInfoType):
         """Handle zeroconf discovery."""
         # Hostname is format: livingroom.local.
-        local_name = user_input["hostname"][:-1]
+        local_name = discovery_info["hostname"][:-1]
         node_name = local_name[: -len(".local")]
-        address = user_input["properties"].get("address", local_name)
+        address = discovery_info["properties"].get("address", local_name)
 
         # Check if already configured
+        await self.async_set_unique_id(node_name)
+        self._abort_if_unique_id_configured(
+            updates={CONF_HOST: discovery_info[CONF_HOST]}
+        )
+
         for entry in self._async_current_entries():
             already_configured = False
-            if entry.data["host"] == address:
+
-                # Is this address already configured?
+            if (
+                entry.data[CONF_HOST] == address
+                or entry.data[CONF_HOST] == discovery_info[CONF_HOST]
+            ):
+                # Is this address or IP address already configured?
                 already_configured = True
             elif entry.entry_id in self.hass.data.get(DATA_KEY, {}):
                 # Does a config entry with this name already exist?
                 data: RuntimeEntryData = self.hass.data[DATA_KEY][entry.entry_id]
+
                 # Node names are unique in the network
                 if data.device_info is not None:
                     already_configured = data.device_info.name == node_name
 
             if already_configured:
+                # Backwards compat, we update old entries
+                if not entry.unique_id:
+                    self.hass.config_entries.async_update_entry(
+                        entry,
+                        data={**entry.data, CONF_HOST: discovery_info[CONF_HOST]},
+                        unique_id=node_name,
+                    )
+
                 return self.async_abort(reason="already_configured")
 
-        self._host = address
+        self._host = discovery_info[CONF_HOST]
-        self._port = user_input["port"]
+        self._port = discovery_info[CONF_PORT]
         self._name = node_name
 
-        # Check if flow for this device already in progress
-        for flow in self._async_in_progress():
-            if flow["context"].get("name") == node_name:
-                return self.async_abort(reason="already_configured")
-
         return await self.async_step_discovery_confirm()
 
-    @core.callback
+    @callback
     def _async_get_entry(self):
         return self.async_create_entry(
             title=self._name,
             data={
-                "host": self._host,
+                CONF_HOST: self._host,
-                "port": self._port,
+                CONF_PORT: self._port,
                 # The API uses protobuf, so empty string denotes absence
-                "password": self._password or "",
+                CONF_PASSWORD: self._password or "",
             },
         )
 
     async def async_step_authenticate(self, user_input=None, error=None):
         """Handle getting password for authentication."""
         if user_input is not None:
-            self._password = user_input["password"]
+            self._password = user_input[CONF_PASSWORD]
             error = await self.try_login()
             if error:
                 return await self.async_step_authenticate(error=error)

homeassistant/components/esphome/strings.json

@@ -1,6 +1,9 @@
 {
   "config": {
-    "abort": { "already_configured": "ESP is already configured" },
+    "abort": {
+      "already_configured": "ESP is already configured",
+      "already_in_progress": "ESP configuration is already in progress"
+    },
     "error": {
       "resolve_error": "Can't resolve address of the ESP. If this error persists, please set a static IP address: https://esphomelib.com/esphomeyaml/components/wifi.html#manual-ips",
       "connection_error": "Can't connect to ESP. Please make sure your YAML file contains an 'api:' line.",

homeassistant/components/esphome/translations/en.json

@@ -1,7 +1,8 @@
 {
     "config": {
         "abort": {
-            "already_configured": "ESP is already configured"
+            "already_configured": "ESP is already configured",
+            "already_in_progress": "ESP configuration is already in progress"
         },
         "error": {
             "connection_error": "Can't connect to ESP. Please make sure your YAML file contains an 'api:' line.",

tests/components/esphome/test_config_flow.py

@@ -3,7 +3,13 @@ from collections import namedtuple
 
 import pytest
 
-from homeassistant.components.esphome import DATA_KEY, config_flow
+from homeassistant.components.esphome import DATA_KEY
+from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_PORT
+from homeassistant.data_entry_flow import (
+    RESULT_TYPE_ABORT,
+    RESULT_TYPE_CREATE_ENTRY,
+    RESULT_TYPE_FORM,
+)
 
 from tests.async_mock import AsyncMock, MagicMock, patch
 from tests.common import MockConfigEntry
@@ -40,26 +46,27 @@ def mock_api_connection_error():
         yield mock_error
 
 
-def _setup_flow_handler(hass):
-    flow = config_flow.EsphomeFlowHandler()
-    flow.hass = hass
-    flow.context = {}
-    return flow
-
-
 async def test_user_connection_works(hass, mock_client):
     """Test we can finish a config flow."""
-    flow = _setup_flow_handler(hass)
+    result = await hass.config_entries.flow.async_init(
-    result = await flow.async_step_user(user_input=None)
+        "esphome", context={"source": "user"}, data=None,
-    assert result["type"] == "form"
+    )
+
+    assert result["type"] == RESULT_TYPE_FORM
+    assert result["step_id"] == "user"
 
     mock_client.device_info = AsyncMock(return_value=MockDeviceInfo(False, "test"))
 
-    result = await flow.async_step_user(user_input={"host": "127.0.0.1", "port": 80})
+    result = await hass.config_entries.flow.async_init(
+        "esphome",
+        context={"source": "user"},
+        data={CONF_HOST: "127.0.0.1", CONF_PORT: 80},
+    )
 
-    assert result["type"] == "create_entry"
+    assert result["type"] == RESULT_TYPE_CREATE_ENTRY
-    assert result["data"] == {"host": "127.0.0.1", "port": 80, "password": ""}
+    assert result["data"] == {CONF_HOST: "127.0.0.1", CONF_PORT: 80, CONF_PASSWORD: ""}
     assert result["title"] == "test"
+
     assert len(mock_client.connect.mock_calls) == 1
     assert len(mock_client.device_info.mock_calls) == 1
     assert len(mock_client.disconnect.mock_calls) == 1
@@ -70,8 +77,6 @@ async def test_user_connection_works(hass, mock_client):
 
 async def test_user_resolve_error(hass, mock_api_connection_error, mock_client):
     """Test user step with IP resolve error."""
-    flow = _setup_flow_handler(hass)
-    await flow.async_step_user(user_input=None)
 
     class MockResolveError(mock_api_connection_error):
         """Create an exception with a specific error message."""
@@ -85,13 +90,16 @@ async def test_user_resolve_error(hass, mock_api_connection_error, mock_client):
         new_callable=lambda: MockResolveError,
     ) as exc:
         mock_client.device_info.side_effect = exc
-        result = await flow.async_step_user(
+        result = await hass.config_entries.flow.async_init(
-            user_input={"host": "127.0.0.1", "port": 6053}
+            "esphome",
+            context={"source": "user"},
+            data={CONF_HOST: "127.0.0.1", CONF_PORT: 6053},
         )
 
-    assert result["type"] == "form"
+    assert result["type"] == RESULT_TYPE_FORM
     assert result["step_id"] == "user"
     assert result["errors"] == {"base": "resolve_error"}
+
     assert len(mock_client.connect.mock_calls) == 1
     assert len(mock_client.device_info.mock_calls) == 1
     assert len(mock_client.disconnect.mock_calls) == 1
@@ -99,16 +107,18 @@ async def test_user_resolve_error(hass, mock_api_connection_error, mock_client):
 
 async def test_user_connection_error(hass, mock_api_connection_error, mock_client):
     """Test user step with connection error."""
-    flow = _setup_flow_handler(hass)
-    await flow.async_step_user(user_input=None)
-
     mock_client.device_info.side_effect = mock_api_connection_error
 
-    result = await flow.async_step_user(user_input={"host": "127.0.0.1", "port": 6053})
+    result = await hass.config_entries.flow.async_init(
+        "esphome",
+        context={"source": "user"},
+        data={CONF_HOST: "127.0.0.1", CONF_PORT: 6053},
+    )
 
-    assert result["type"] == "form"
+    assert result["type"] == RESULT_TYPE_FORM
     assert result["step_id"] == "user"
     assert result["errors"] == {"base": "connection_error"}
+
     assert len(mock_client.connect.mock_calls) == 1
     assert len(mock_client.device_info.mock_calls) == 1
     assert len(mock_client.disconnect.mock_calls) == 1
@@ -116,125 +126,159 @@ async def test_user_connection_error(hass, mock_api_connection_error, mock_clien
 
 async def test_user_with_password(hass, mock_client):
     """Test user step with password."""
-    flow = _setup_flow_handler(hass)
-    await flow.async_step_user(user_input=None)
-
     mock_client.device_info = AsyncMock(return_value=MockDeviceInfo(True, "test"))
 
-    result = await flow.async_step_user(user_input={"host": "127.0.0.1", "port": 6053})
+    result = await hass.config_entries.flow.async_init(
+        "esphome",
+        context={"source": "user"},
+        data={CONF_HOST: "127.0.0.1", CONF_PORT: 6053},
+    )
 
-    assert result["type"] == "form"
+    assert result["type"] == RESULT_TYPE_FORM
     assert result["step_id"] == "authenticate"
 
-    result = await flow.async_step_authenticate(user_input={"password": "password1"})
+    result = await hass.config_entries.flow.async_configure(
+        result["flow_id"], user_input={CONF_PASSWORD: "password1"}
+    )
 
-    assert result["type"] == "create_entry"
+    assert result["type"] == RESULT_TYPE_CREATE_ENTRY
     assert result["data"] == {
-        "host": "127.0.0.1",
+        CONF_HOST: "127.0.0.1",
-        "port": 6053,
+        CONF_PORT: 6053,
-        "password": "password1",
+        CONF_PASSWORD: "password1",
     }
     assert mock_client.password == "password1"
 
 
 async def test_user_invalid_password(hass, mock_api_connection_error, mock_client):
     """Test user step with invalid password."""
-    flow = _setup_flow_handler(hass)
-    await flow.async_step_user(user_input=None)
-
     mock_client.device_info = AsyncMock(return_value=MockDeviceInfo(True, "test"))
 
-    await flow.async_step_user(user_input={"host": "127.0.0.1", "port": 6053})
+    result = await hass.config_entries.flow.async_init(
-    mock_client.connect.side_effect = mock_api_connection_error
+        "esphome",
-    result = await flow.async_step_authenticate(user_input={"password": "invalid"})
+        context={"source": "user"},
+        data={CONF_HOST: "127.0.0.1", CONF_PORT: 6053},
+    )
 
-    assert result["type"] == "form"
+    assert result["type"] == RESULT_TYPE_FORM
+    assert result["step_id"] == "authenticate"
+
+    mock_client.connect.side_effect = mock_api_connection_error
+
+    result = await hass.config_entries.flow.async_configure(
+        result["flow_id"], user_input={CONF_PASSWORD: "invalid"}
+    )
+
+    assert result["type"] == RESULT_TYPE_FORM
     assert result["step_id"] == "authenticate"
     assert result["errors"] == {"base": "invalid_password"}
 
 
 async def test_discovery_initiation(hass, mock_client):
     """Test discovery importing works."""
-    flow = _setup_flow_handler(hass)
+    mock_client.device_info = AsyncMock(return_value=MockDeviceInfo(False, "test8266"))
+
     service_info = {
         "host": "192.168.43.183",
         "port": 6053,
         "hostname": "test8266.local.",
         "properties": {},
     }
+    flow = await hass.config_entries.flow.async_init(
+        "esphome", context={"source": "zeroconf"}, data=service_info
+    )
 
-    mock_client.device_info = AsyncMock(return_value=MockDeviceInfo(False, "test8266"))
+    result = await hass.config_entries.flow.async_configure(
+        flow["flow_id"], user_input={}
+    )
 
-    result = await flow.async_step_zeroconf(user_input=service_info)
+    assert result["type"] == RESULT_TYPE_CREATE_ENTRY
-    assert result["type"] == "form"
-    assert result["step_id"] == "discovery_confirm"
-    assert result["description_placeholders"]["name"] == "test8266"
-    assert flow.context["title_placeholders"]["name"] == "test8266"
-
-    result = await flow.async_step_discovery_confirm(user_input={})
-    assert result["type"] == "create_entry"
     assert result["title"] == "test8266"
-    assert result["data"]["host"] == "test8266.local"
+    assert result["data"][CONF_HOST] == "192.168.43.183"
-    assert result["data"]["port"] == 6053
+    assert result["data"][CONF_PORT] == 6053
+
+    assert result["result"]
+    assert result["result"].unique_id == "test8266"
 
 
 async def test_discovery_already_configured_hostname(hass, mock_client):
     """Test discovery aborts if already configured via hostname."""
-    MockConfigEntry(
+    entry = MockConfigEntry(
-        domain="esphome", data={"host": "test8266.local", "port": 6053, "password": ""}
+        domain="esphome",
-    ).add_to_hass(hass)
+        data={CONF_HOST: "test8266.local", CONF_PORT: 6053, CONF_PASSWORD: ""},
+    )
+
+    entry.add_to_hass(hass)
 
-    flow = _setup_flow_handler(hass)
     service_info = {
         "host": "192.168.43.183",
         "port": 6053,
         "hostname": "test8266.local.",
         "properties": {},
     }
-    result = await flow.async_step_zeroconf(user_input=service_info)
+    result = await hass.config_entries.flow.async_init(
-    assert result["type"] == "abort"
+        "esphome", context={"source": "zeroconf"}, data=service_info
+    )
+
+    assert result["type"] == RESULT_TYPE_ABORT
     assert result["reason"] == "already_configured"
 
+    assert entry.unique_id == "test8266"
+
 
 async def test_discovery_already_configured_ip(hass, mock_client):
     """Test discovery aborts if already configured via static IP."""
-    MockConfigEntry(
+    entry = MockConfigEntry(
-        domain="esphome", data={"host": "192.168.43.183", "port": 6053, "password": ""}
+        domain="esphome",
-    ).add_to_hass(hass)
+        data={CONF_HOST: "192.168.43.183", CONF_PORT: 6053, CONF_PASSWORD: ""},
+    )
+
+    entry.add_to_hass(hass)
 
-    flow = _setup_flow_handler(hass)
     service_info = {
         "host": "192.168.43.183",
         "port": 6053,
         "hostname": "test8266.local.",
         "properties": {"address": "192.168.43.183"},
     }
-    result = await flow.async_step_zeroconf(user_input=service_info)
+    result = await hass.config_entries.flow.async_init(
-    assert result["type"] == "abort"
+        "esphome", context={"source": "zeroconf"}, data=service_info
+    )
+
+    assert result["type"] == RESULT_TYPE_ABORT
     assert result["reason"] == "already_configured"
 
+    assert entry.unique_id == "test8266"
+
 
 async def test_discovery_already_configured_name(hass, mock_client):
     """Test discovery aborts if already configured via name."""
     entry = MockConfigEntry(
-        domain="esphome", data={"host": "192.168.43.183", "port": 6053, "password": ""}
+        domain="esphome",
+        data={CONF_HOST: "192.168.43.183", CONF_PORT: 6053, CONF_PASSWORD: ""},
     )
     entry.add_to_hass(hass)
+
     mock_entry_data = MagicMock()
     mock_entry_data.device_info.name = "test8266"
     hass.data[DATA_KEY] = {entry.entry_id: mock_entry_data}
 
-    flow = _setup_flow_handler(hass)
     service_info = {
-        "host": "192.168.43.183",
+        "host": "192.168.43.184",
         "port": 6053,
         "hostname": "test8266.local.",
         "properties": {"address": "test8266.local"},
     }
-    result = await flow.async_step_zeroconf(user_input=service_info)
+    result = await hass.config_entries.flow.async_init(
-    assert result["type"] == "abort"
+        "esphome", context={"source": "zeroconf"}, data=service_info
+    )
+
+    assert result["type"] == RESULT_TYPE_ABORT
     assert result["reason"] == "already_configured"
 
+    assert entry.unique_id == "test8266"
+    assert entry.data[CONF_HOST] == "192.168.43.184"
+
 
 async def test_discovery_duplicate_data(hass, mock_client):
     """Test discovery aborts if same mDNS packet arrives."""
@@ -250,11 +294,36 @@ async def test_discovery_duplicate_data(hass, mock_client):
     result = await hass.config_entries.flow.async_init(
         "esphome", data=service_info, context={"source": "zeroconf"}
     )
-    assert result["type"] == "form"
+    assert result["type"] == RESULT_TYPE_FORM
     assert result["step_id"] == "discovery_confirm"
 
     result = await hass.config_entries.flow.async_init(
         "esphome", data=service_info, context={"source": "zeroconf"}
     )
-    assert result["type"] == "abort"
+    assert result["type"] == RESULT_TYPE_ABORT
+    assert result["reason"] == "already_in_progress"
+
+
+async def test_discovery_updates_unique_id(hass, mock_client):
+    """Test a duplicate discovery host aborts and updates existing entry."""
+    entry = MockConfigEntry(
+        domain="esphome",
+        data={CONF_HOST: "192.168.43.183", CONF_PORT: 6053, CONF_PASSWORD: ""},
+    )
+
+    entry.add_to_hass(hass)
+
+    service_info = {
+        "host": "192.168.43.183",
+        "port": 6053,
+        "hostname": "test8266.local.",
+        "properties": {"address": "test8266.local"},
+    }
+    result = await hass.config_entries.flow.async_init(
+        "esphome", context={"source": "zeroconf"}, data=service_info
+    )
+
+    assert result["type"] == RESULT_TYPE_ABORT
     assert result["reason"] == "already_configured"
+
+    assert entry.unique_id == "test8266"