diff --git a/homeassistant/components/esphome/config_flow.py b/homeassistant/components/esphome/config_flow.py index 17d3ed5f659..cb9b7958efa 100644 --- a/homeassistant/components/esphome/config_flow.py +++ b/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.helpers.typing import ConfigType +from homeassistant.config_entries import CONN_CLASS_LOCAL_PUSH, ConfigFlow +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.Optional("port", default=self._port or 6053)] = int + fields[vol.Required(CONF_HOST, default=self._host or vol.UNDEFINED)] = str + 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._port = user_input["port"] + self._host = user_input[CONF_HOST] + 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._port = user_input["port"] + self._host = discovery_info[CONF_HOST] + 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, - "port": self._port, + CONF_HOST: self._host, + 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) diff --git a/homeassistant/components/esphome/strings.json b/homeassistant/components/esphome/strings.json index 0c2f0fee8d9..e5eeb150d89 100644 --- a/homeassistant/components/esphome/strings.json +++ b/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.", diff --git a/homeassistant/components/esphome/translations/en.json b/homeassistant/components/esphome/translations/en.json index b52dbc5a3f1..3f695e6f183 100644 --- a/homeassistant/components/esphome/translations/en.json +++ b/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.", @@ -31,4 +32,4 @@ } } } -} \ No newline at end of file +} diff --git a/tests/components/esphome/test_config_flow.py b/tests/components/esphome/test_config_flow.py index b55621226fa..50aaa989028 100644 --- a/tests/components/esphome/test_config_flow.py +++ b/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 flow.async_step_user(user_input=None) - assert result["type"] == "form" + result = await hass.config_entries.flow.async_init( + "esphome", context={"source": "user"}, data=None, + ) + + 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["data"] == {"host": "127.0.0.1", "port": 80, "password": ""} + assert result["type"] == RESULT_TYPE_CREATE_ENTRY + 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( - 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": "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", - "port": 6053, - "password": "password1", + CONF_HOST: "127.0.0.1", + CONF_PORT: 6053, + 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}) - mock_client.connect.side_effect = mock_api_connection_error - result = await flow.async_step_authenticate(user_input={"password": "invalid"}) + 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" + + 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"] == "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["type"] == RESULT_TYPE_CREATE_ENTRY assert result["title"] == "test8266" - assert result["data"]["host"] == "test8266.local" - assert result["data"]["port"] == 6053 + assert result["data"][CONF_HOST] == "192.168.43.183" + 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( - domain="esphome", data={"host": "test8266.local", "port": 6053, "password": ""} - ).add_to_hass(hass) + entry = MockConfigEntry( + domain="esphome", + 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) - assert result["type"] == "abort" + 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" + async def test_discovery_already_configured_ip(hass, mock_client): """Test discovery aborts if already configured via static IP.""" - MockConfigEntry( - domain="esphome", data={"host": "192.168.43.183", "port": 6053, "password": ""} - ).add_to_hass(hass) + entry = MockConfigEntry( + domain="esphome", + 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) - assert result["type"] == "abort" + 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" + 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) - assert result["type"] == "abort" + 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" + 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"