Fix ESPHome discovered when already exists (#24187)

* Fix ESPHome discovered when already exists

* Update .coveragerc

.coveragerc

@@ -172,6 +172,7 @@ omit =
     homeassistant/components/esphome/camera.py
     homeassistant/components/esphome/climate.py
     homeassistant/components/esphome/cover.py
+    homeassistant/components/esphome/entry_data.py
     homeassistant/components/esphome/fan.py
     homeassistant/components/esphome/light.py
     homeassistant/components/esphome/sensor.py

homeassistant/components/esphome/__init__.py

@@ -2,12 +2,11 @@
 import asyncio
 import logging
 import math
-from typing import Any, Callable, Dict, List, Optional, Tuple
+from typing import Any, Callable, Dict, List, Optional
 
 from aioesphomeapi import (
-    COMPONENT_TYPE_TO_INFO, APIClient, APIConnectionError, DeviceInfo,
+    APIClient, APIConnectionError, DeviceInfo, EntityInfo, EntityState,
-    EntityInfo, EntityState, ServiceCall, UserService, UserServiceArgType)
+    ServiceCall, UserService, UserServiceArgType)
-import attr
 import voluptuous as vol
 
 from homeassistant import const
@@ -19,8 +18,7 @@ from homeassistant.exceptions import TemplateError
 from homeassistant.helpers import template
 import homeassistant.helpers.config_validation as cv
 import homeassistant.helpers.device_registry as dr
-from homeassistant.helpers.dispatcher import (
+from homeassistant.helpers.dispatcher import async_dispatcher_connect
-    async_dispatcher_connect, async_dispatcher_send)
 from homeassistant.helpers.entity import Entity
 from homeassistant.helpers.event import async_track_state_change
 from homeassistant.helpers.json import JSONEncoder
@@ -30,16 +28,14 @@ from homeassistant.helpers.typing import ConfigType, HomeAssistantType
 
 # Import config flow so that it's added to the registry
 from .config_flow import EsphomeFlowHandler  # noqa
+from .entry_data import (
+    DATA_KEY, DISPATCHER_ON_DEVICE_UPDATE, DISPATCHER_ON_LIST,
+    DISPATCHER_ON_STATE, DISPATCHER_REMOVE_ENTITY, DISPATCHER_UPDATE_ENTITY,
+    RuntimeEntryData)
 
 DOMAIN = 'esphome'
 _LOGGER = logging.getLogger(__name__)
 
-DISPATCHER_UPDATE_ENTITY = 'esphome_{entry_id}_update_{component_key}_{key}'
-DISPATCHER_REMOVE_ENTITY = 'esphome_{entry_id}_remove_{component_key}_{key}'
-DISPATCHER_ON_LIST = 'esphome_{entry_id}_on_list'
-DISPATCHER_ON_DEVICE_UPDATE = 'esphome_{entry_id}_on_device_update'
-DISPATCHER_ON_STATE = 'esphome_{entry_id}_on_state'
-
 STORAGE_KEY = 'esphome.{}'
 STORAGE_VERSION = 1
 
@@ -59,95 +55,6 @@ HA_COMPONENTS = [
 CONFIG_SCHEMA = vol.Schema({}, extra=vol.ALLOW_EXTRA)
 
 
-@attr.s
-class RuntimeEntryData:
-    """Store runtime data for esphome config entries."""
-
-    entry_id = attr.ib(type=str)
-    client = attr.ib(type='APIClient')
-    store = attr.ib(type=Store)
-    reconnect_task = attr.ib(type=Optional[asyncio.Task], default=None)
-    state = attr.ib(type=Dict[str, Dict[str, Any]], factory=dict)
-    info = attr.ib(type=Dict[str, Dict[str, Any]], factory=dict)
-    services = attr.ib(type=Dict[int, 'UserService'], factory=dict)
-    available = attr.ib(type=bool, default=False)
-    device_info = attr.ib(type='DeviceInfo', default=None)
-    cleanup_callbacks = attr.ib(type=List[Callable[[], None]], factory=list)
-    disconnect_callbacks = attr.ib(type=List[Callable[[], None]], factory=list)
-
-    def async_update_entity(self, hass: HomeAssistantType, component_key: str,
-                            key: int) -> None:
-        """Schedule the update of an entity."""
-        signal = DISPATCHER_UPDATE_ENTITY.format(
-            entry_id=self.entry_id, component_key=component_key, key=key)
-        async_dispatcher_send(hass, signal)
-
-    def async_remove_entity(self, hass: HomeAssistantType, component_key: str,
-                            key: int) -> None:
-        """Schedule the removal of an entity."""
-        signal = DISPATCHER_REMOVE_ENTITY.format(
-            entry_id=self.entry_id, component_key=component_key, key=key)
-        async_dispatcher_send(hass, signal)
-
-    def async_update_static_infos(self, hass: HomeAssistantType,
-                                  infos: 'List[EntityInfo]') -> None:
-        """Distribute an update of static infos to all platforms."""
-        signal = DISPATCHER_ON_LIST.format(entry_id=self.entry_id)
-        async_dispatcher_send(hass, signal, infos)
-
-    def async_update_state(self, hass: HomeAssistantType,
-                           state: 'EntityState') -> None:
-        """Distribute an update of state information to all platforms."""
-        signal = DISPATCHER_ON_STATE.format(entry_id=self.entry_id)
-        async_dispatcher_send(hass, signal, state)
-
-    def async_update_device_state(self, hass: HomeAssistantType) -> None:
-        """Distribute an update of a core device state like availability."""
-        signal = DISPATCHER_ON_DEVICE_UPDATE.format(entry_id=self.entry_id)
-        async_dispatcher_send(hass, signal)
-
-    async def async_load_from_store(self) -> Tuple[List['EntityInfo'],
-                                                   List['UserService']]:
-        """Load the retained data from store and return de-serialized data."""
-        restored = await self.store.async_load()
-        if restored is None:
-            return [], []
-
-        self.device_info = _attr_obj_from_dict(DeviceInfo,
-                                               **restored.pop('device_info'))
-        infos = []
-        for comp_type, restored_infos in restored.items():
-            if comp_type not in COMPONENT_TYPE_TO_INFO:
-                continue
-            for info in restored_infos:
-                cls = COMPONENT_TYPE_TO_INFO[comp_type]
-                infos.append(_attr_obj_from_dict(cls, **info))
-        services = []
-        for service in restored.get('services', []):
-            services.append(UserService.from_dict(service))
-        return infos, services
-
-    async def async_save_to_store(self) -> None:
-        """Generate dynamic data to store and save it to the filesystem."""
-        store_data = {
-            'device_info': attr.asdict(self.device_info),
-            'services': []
-        }
-
-        for comp_type, infos in self.info.items():
-            store_data[comp_type] = [attr.asdict(info)
-                                     for info in infos.values()]
-        for service in self.services.values():
-            store_data['services'].append(service.to_dict())
-
-        await self.store.async_save(store_data)
-
-
-def _attr_obj_from_dict(cls, **kwargs):
-    return cls(**{key: kwargs[key] for key in attr.fields_dict(cls)
-                  if key in kwargs})
-
-
 async def async_setup(hass: HomeAssistantType, config: ConfigType) -> bool:
     """Stub to allow setting up this component.
 
@@ -159,7 +66,7 @@ async def async_setup(hass: HomeAssistantType, config: ConfigType) -> bool:
 async def async_setup_entry(hass: HomeAssistantType,
                             entry: ConfigEntry) -> bool:
     """Set up the esphome component."""
-    hass.data.setdefault(DOMAIN, {})
+    hass.data.setdefault(DATA_KEY, {})
 
     host = entry.data[CONF_HOST]
     port = entry.data[CONF_PORT]
@@ -171,7 +78,7 @@ async def async_setup_entry(hass: HomeAssistantType,
     # Store client in per-config-entry hass.data
     store = Store(hass, STORAGE_VERSION, STORAGE_KEY.format(entry.entry_id),
                   encoder=JSONEncoder)
-    entry_data = hass.data[DOMAIN][entry.entry_id] = RuntimeEntryData(
+    entry_data = hass.data[DATA_KEY][entry.entry_id] = RuntimeEntryData(
         client=cli,
         entry_id=entry.entry_id,
         store=store,
@@ -186,12 +93,12 @@ async def async_setup_entry(hass: HomeAssistantType,
     )
 
     @callback
-    def async_on_state(state: 'EntityState') -> None:
+    def async_on_state(state: EntityState) -> None:
         """Send dispatcher updates when a new state is received."""
         entry_data.async_update_state(hass, state)
 
     @callback
-    def async_on_service_call(service: 'ServiceCall') -> None:
+    def async_on_service_call(service: ServiceCall) -> None:
         """Call service when user automation in ESPHome config is triggered."""
         domain, service_name = service.service.split('.', 1)
         service_data = service.data
@@ -253,26 +160,6 @@ async def async_setup_entry(hass: HomeAssistantType,
     try_connect = await _setup_auto_reconnect_logic(hass, cli, entry, host,
                                                     on_login)
 
-    # This is a bit of a hack: We schedule complete_setup into the
-    # event loop and return immediately (return True)
-    #
-    # Usually, we should avoid that so that HA can track which components
-    # have been started successfully and which failed to be set up.
-    # That doesn't work here for two reasons:
-    #  - We have our own re-connect logic
-    #  - Before we do the first try_connect() call, we need to make sure
-    #    all dispatcher event listeners have been connected, so
-    #    async_forward_entry_setup needs to be awaited. However, if we
-    #    would await async_forward_entry_setup() in async_setup_entry(),
-    #    we would end up with a deadlock.
-    #
-    # Solution is: complete the setup outside of the async_setup_entry()
-    # function. HA will wait until the first connection attempt is made
-    # before starting up (as it should), but if the first connection attempt
-    # fails we will schedule all next re-connect attempts outside of the
-    # tracked tasks (hass.loop.create_task). This way HA won't stall startup
-    # forever until a connection is successful.
-
     async def complete_setup() -> None:
         """Complete the config entry setup."""
         tasks = []
@@ -285,17 +172,16 @@ async def async_setup_entry(hass: HomeAssistantType,
         entry_data.async_update_static_infos(hass, infos)
         await _setup_services(hass, entry_data, services)
 
-        # If first connect fails, the next re-connect will be scheduled
+        # Create connection attempt outside of HA's tracked task in order
-        # outside of _pending_task, in order not to delay HA startup
+        # not to delay startup.
-        # indefinitely
+        hass.loop.create_task(try_connect(is_disconnect=False))
-        await try_connect(is_disconnect=False)
 
     hass.async_create_task(complete_setup())
     return True
 
 
 async def _setup_auto_reconnect_logic(hass: HomeAssistantType,
-                                      cli: 'APIClient',
+                                      cli: APIClient,
                                       entry: ConfigEntry, host: str, on_login):
     """Set up the re-connect logic for the API client."""
     async def try_connect(tries: int = 0, is_disconnect: bool = True) -> None:
@@ -351,7 +237,7 @@ async def _setup_auto_reconnect_logic(hass: HomeAssistantType,
 
 async def _async_setup_device_registry(hass: HomeAssistantType,
                                        entry: ConfigEntry,
-                                       device_info: 'DeviceInfo'):
+                                       device_info: DeviceInfo):
     """Set up device registry feature for a particular config entry."""
     sw_version = device_info.esphome_core_version
     if device_info.compilation_time:
@@ -371,7 +257,7 @@ async def _async_setup_device_registry(hass: HomeAssistantType,
 
 async def _register_service(hass: HomeAssistantType,
                             entry_data: RuntimeEntryData,
-                            service: 'UserService'):
+                            service: UserService):
     service_name = '{}_{}'.format(entry_data.device_info.name, service.name)
     schema = {}
     for arg in service.args:
@@ -391,7 +277,7 @@ async def _register_service(hass: HomeAssistantType,
 
 async def _setup_services(hass: HomeAssistantType,
                           entry_data: RuntimeEntryData,
-                          services: List['UserService']):
+                          services: List[UserService]):
     old_services = entry_data.services.copy()
     to_unregister = []
     to_register = []
@@ -424,7 +310,7 @@ async def _setup_services(hass: HomeAssistantType,
 async def _cleanup_instance(hass: HomeAssistantType,
                             entry: ConfigEntry) -> None:
     """Cleanup the esphome client if it exists."""
-    data = hass.data[DOMAIN].pop(entry.entry_id)  # type: RuntimeEntryData
+    data = hass.data[DATA_KEY].pop(entry.entry_id)  # type: RuntimeEntryData
     if data.reconnect_task is not None:
         data.reconnect_task.cancel()
     for disconnect_cb in data.disconnect_callbacks:
@@ -467,7 +353,7 @@ async def platform_async_setup_entry(hass: HomeAssistantType,
     entry_data.state[component_key] = {}
 
     @callback
-    def async_list_entities(infos: List['EntityInfo']):
+    def async_list_entities(infos: List[EntityInfo]):
         """Update entities of this platform when entities are listed."""
         old_infos = entry_data.info[component_key]
         new_infos = {}
@@ -498,7 +384,7 @@ async def platform_async_setup_entry(hass: HomeAssistantType,
     )
 
     @callback
-    def async_entity_state(state: 'EntityState'):
+    def async_entity_state(state: EntityState):
         """Notify the appropriate entity of an updated state."""
         if not isinstance(state, state_type):
             return
@@ -519,6 +405,7 @@ def esphome_state_property(func):
     """
     @property
     def _wrapper(self):
+        # pylint: disable=protected-access
         if self._state is None:
             return None
         val = func(self)
@@ -603,22 +490,22 @@ class EsphomeEntity(Entity):
 
     @property
     def _entry_data(self) -> RuntimeEntryData:
-        return self.hass.data[DOMAIN][self._entry_id]
+        return self.hass.data[DATA_KEY][self._entry_id]
 
     @property
-    def _static_info(self) -> 'EntityInfo':
+    def _static_info(self) -> EntityInfo:
         return self._entry_data.info[self._component_key][self._key]
 
     @property
-    def _device_info(self) -> 'DeviceInfo':
+    def _device_info(self) -> DeviceInfo:
         return self._entry_data.device_info
 
     @property
-    def _client(self) -> 'APIClient':
+    def _client(self) -> APIClient:
         return self._entry_data.client
 
     @property
-    def _state(self) -> 'Optional[EntityState]':
+    def _state(self) -> Optional[EntityState]:
         try:
             return self._entry_data.state[self._component_key][self._key]
         except KeyError:

homeassistant/components/esphome/config_flow.py

@@ -7,6 +7,8 @@ import voluptuous as vol
 from homeassistant import config_entries
 from homeassistant.helpers import ConfigType
 
+from .entry_data import DATA_KEY, RuntimeEntryData
+
 
 @config_entries.HANDLERS.register('esphome')
 class EsphomeFlowHandler(config_entries.ConfigFlow):
@@ -76,10 +78,25 @@ class EsphomeFlowHandler(config_entries.ConfigFlow):
 
     async def async_step_zeroconf(self, user_input: ConfigType):
         """Handle zeroconf discovery."""
-        address = user_input['properties'].get(
+        # Hostname is format: livingroom.local.
-            'address', user_input['hostname'][:-1])
+        local_name = user_input['hostname'][:-1]
+        node_name = local_name[:-len('.local')]
+        address = user_input['properties'].get('address', local_name)
+
+        # Check if already configured
         for entry in self._async_current_entries():
+            already_configured = False
             if entry.data['host'] == address:
+                # Is this 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 = self.hass.data[DATA_KEY][
+                    entry.entry_id]  # type: RuntimeEntryData
+                # Node names are unique in the network
+                already_configured = data.device_info.name == node_name
+
+            if already_configured:
                 return self.async_abort(
                     reason='already_configured'
                 )

homeassistant/components/esphome/entry_data.py

@@ -0,0 +1,107 @@
+"""Runtime entry data for ESPHome stored in hass.data."""
+import asyncio
+from typing import Any, Callable, Dict, List, Optional, Tuple
+
+from aioesphomeapi import (
+    COMPONENT_TYPE_TO_INFO, DeviceInfo, EntityInfo, EntityState, UserService)
+import attr
+
+from homeassistant.helpers.dispatcher import async_dispatcher_send
+from homeassistant.helpers.storage import Store
+from homeassistant.helpers.typing import HomeAssistantType
+
+DATA_KEY = 'esphome'
+DISPATCHER_UPDATE_ENTITY = 'esphome_{entry_id}_update_{component_key}_{key}'
+DISPATCHER_REMOVE_ENTITY = 'esphome_{entry_id}_remove_{component_key}_{key}'
+DISPATCHER_ON_LIST = 'esphome_{entry_id}_on_list'
+DISPATCHER_ON_DEVICE_UPDATE = 'esphome_{entry_id}_on_device_update'
+DISPATCHER_ON_STATE = 'esphome_{entry_id}_on_state'
+
+
+@attr.s
+class RuntimeEntryData:
+    """Store runtime data for esphome config entries."""
+
+    entry_id = attr.ib(type=str)
+    client = attr.ib(type='APIClient')
+    store = attr.ib(type=Store)
+    reconnect_task = attr.ib(type=Optional[asyncio.Task], default=None)
+    state = attr.ib(type=Dict[str, Dict[str, Any]], factory=dict)
+    info = attr.ib(type=Dict[str, Dict[str, Any]], factory=dict)
+    services = attr.ib(type=Dict[int, 'UserService'], factory=dict)
+    available = attr.ib(type=bool, default=False)
+    device_info = attr.ib(type=DeviceInfo, default=None)
+    cleanup_callbacks = attr.ib(type=List[Callable[[], None]], factory=list)
+    disconnect_callbacks = attr.ib(type=List[Callable[[], None]], factory=list)
+
+    def async_update_entity(self, hass: HomeAssistantType, component_key: str,
+                            key: int) -> None:
+        """Schedule the update of an entity."""
+        signal = DISPATCHER_UPDATE_ENTITY.format(
+            entry_id=self.entry_id, component_key=component_key, key=key)
+        async_dispatcher_send(hass, signal)
+
+    def async_remove_entity(self, hass: HomeAssistantType, component_key: str,
+                            key: int) -> None:
+        """Schedule the removal of an entity."""
+        signal = DISPATCHER_REMOVE_ENTITY.format(
+            entry_id=self.entry_id, component_key=component_key, key=key)
+        async_dispatcher_send(hass, signal)
+
+    def async_update_static_infos(self, hass: HomeAssistantType,
+                                  infos: List[EntityInfo]) -> None:
+        """Distribute an update of static infos to all platforms."""
+        signal = DISPATCHER_ON_LIST.format(entry_id=self.entry_id)
+        async_dispatcher_send(hass, signal, infos)
+
+    def async_update_state(self, hass: HomeAssistantType,
+                           state: EntityState) -> None:
+        """Distribute an update of state information to all platforms."""
+        signal = DISPATCHER_ON_STATE.format(entry_id=self.entry_id)
+        async_dispatcher_send(hass, signal, state)
+
+    def async_update_device_state(self, hass: HomeAssistantType) -> None:
+        """Distribute an update of a core device state like availability."""
+        signal = DISPATCHER_ON_DEVICE_UPDATE.format(entry_id=self.entry_id)
+        async_dispatcher_send(hass, signal)
+
+    async def async_load_from_store(self) -> Tuple[List[EntityInfo],
+                                                   List[UserService]]:
+        """Load the retained data from store and return de-serialized data."""
+        restored = await self.store.async_load()
+        if restored is None:
+            return [], []
+
+        self.device_info = _attr_obj_from_dict(DeviceInfo,
+                                               **restored.pop('device_info'))
+        infos = []
+        for comp_type, restored_infos in restored.items():
+            if comp_type not in COMPONENT_TYPE_TO_INFO:
+                continue
+            for info in restored_infos:
+                cls = COMPONENT_TYPE_TO_INFO[comp_type]
+                infos.append(_attr_obj_from_dict(cls, **info))
+        services = []
+        for service in restored.get('services', []):
+            services.append(UserService.from_dict(service))
+        return infos, services
+
+    async def async_save_to_store(self) -> None:
+        """Generate dynamic data to store and save it to the filesystem."""
+        store_data = {
+            'device_info': attr.asdict(self.device_info),
+            'services': []
+        }
+
+        for comp_type, infos in self.info.items():
+            store_data[comp_type] = [attr.asdict(info)
+                                     for info in infos.values()]
+        for service in self.services.values():
+            store_data['services'].append(service.to_dict())
+
+        await self.store.async_save(store_data)
+
+
+def _attr_obj_from_dict(cls, **kwargs):
+    return cls(**{key: kwargs[key] for key in attr.fields_dict(cls)
+                  if key in kwargs})

tests/components/esphome/test_config_flow.py

@@ -4,7 +4,7 @@ from unittest.mock import MagicMock, patch
 
 import pytest
 
-from homeassistant.components.esphome import config_flow
+from homeassistant.components.esphome import config_flow, DATA_KEY
 from tests.common import mock_coro, MockConfigEntry
 
 MockDeviceInfo = namedtuple("DeviceInfo", ["uses_password", "name"])
@@ -254,3 +254,30 @@ async def test_discovery_already_configured_ip(hass, mock_client):
     result = await flow.async_step_zeroconf(user_input=service_info)
     assert result['type'] == 'abort'
     assert result['reason'] == 'already_configured'
+
+
+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': ''}
+    )
+    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',
+        'port': 6053,
+        'hostname': 'test8266.local.',
+        'properties': {
+            "address": "test8266.local"
+        }
+    }
+    result = await flow.async_step_zeroconf(user_input=service_info)
+    assert result['type'] == 'abort'
+    assert result['reason'] == 'already_configured'