Fix Epson config flow unique_id (#45434)

* switch Epson from HTTP to TCP communication

* fix tests

* add asyncio websession

* fix manifest

* fix config flow

* fix manifest

* fix logger warnings

* switch Epson from HTTP to TCP communication

* fix tests

* add asyncio websession

* fix config flow

* fix manifest

* fix logger warnings

* add already configured to import yaml

* remove neccessary projector on on config.yaml

* remove check import None

* reload integration if no unique_id

* async_migrate_entry

* add async_migrate_entry

* add init tests

* media player migration uid

* unifi config flow

* Update homeassistant/components/epson/media_player.py

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

* Update homeassistant/components/epson/media_player.py

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

* Apply suggestions from code review

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

* review

* remove unnecessary try except

* add import test

* Apply suggestions from code review

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

* revert PORT option from config yaml

* fix tests

* remove port from config flow

* fix CONFIG_SCHEMA

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

homeassistant/components/epson/__init__.py

@@ -2,44 +2,53 @@
 import logging
 
 from epson_projector import Projector
-from epson_projector.const import POWER, STATE_UNAVAILABLE as EPSON_STATE_UNAVAILABLE
+from epson_projector.const import (
+    PWR_OFF_STATE,
+    STATE_UNAVAILABLE as EPSON_STATE_UNAVAILABLE,
+)
 
 from homeassistant.components.media_player import DOMAIN as MEDIA_PLAYER_PLATFORM
 from homeassistant.config_entries import ConfigEntry
-from homeassistant.const import CONF_HOST, CONF_PORT
+from homeassistant.const import CONF_HOST
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
 
-from .const import DOMAIN
+from .const import DOMAIN, HTTP
-from .exceptions import CannotConnect
+from .exceptions import CannotConnect, PoweredOff
 
 PLATFORMS = [MEDIA_PLAYER_PLATFORM]
 
 _LOGGER = logging.getLogger(__name__)
 
 
-async def validate_projector(hass: HomeAssistant, host, port):
+async def validate_projector(
-    """Validate the given host and port allows us to connect."""
+    hass: HomeAssistant, host, check_power=True, check_powered_on=True
+):
+    """Validate the given projector host allows us to connect."""
     epson_proj = Projector(
         host=host,
         websession=async_get_clientsession(hass, verify_ssl=False),
-        port=port,
+        type=HTTP,
     )
-    _power = await epson_proj.get_property(POWER)
+    if check_power:
+        _power = await epson_proj.get_power()
         if not _power or _power == EPSON_STATE_UNAVAILABLE:
+            _LOGGER.debug("Cannot connect to projector")
             raise CannotConnect
+        if _power == PWR_OFF_STATE and check_powered_on:
+            _LOGGER.debug("Projector is off")
+            raise PoweredOff
     return epson_proj
 
 
 async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry):
     """Set up epson from a config entry."""
-    try:
     projector = await validate_projector(
-            hass, entry.data[CONF_HOST], entry.data[CONF_PORT]
+        hass=hass,
+        host=entry.data[CONF_HOST],
+        check_power=False,
+        check_powered_on=False,
     )
-    except CannotConnect:
-        _LOGGER.warning("Cannot connect to projector %s", entry.data[CONF_HOST])
-        return False
     hass.data.setdefault(DOMAIN, {})
     hass.data[DOMAIN][entry.entry_id] = projector
     hass.config_entries.async_setup_platforms(entry, PLATFORMS)

homeassistant/components/epson/config_flow.py

@@ -1,4 +1,6 @@
 """Config flow for epson integration."""
+import logging
+
 import voluptuous as vol
 
 from homeassistant import config_entries
@@ -6,16 +8,17 @@ from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT
 
 from . import validate_projector
 from .const import DOMAIN
-from .exceptions import CannotConnect
+from .exceptions import CannotConnect, PoweredOff
 
 DATA_SCHEMA = vol.Schema(
     {
         vol.Required(CONF_HOST): str,
         vol.Required(CONF_NAME, default=DOMAIN): str,
-        vol.Required(CONF_PORT, default=80): int,
     }
 )
 
+_LOGGER = logging.getLogger(__name__)
+
 
 class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
     """Handle a config flow for epson."""
@@ -24,19 +27,51 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
 
     async def async_step_import(self, import_config):
         """Import a config entry from configuration.yaml."""
-        return await self.async_step_user(import_config)
+        for entry in self._async_current_entries(include_ignore=True):
+            if import_config[CONF_HOST] == entry.data[CONF_HOST]:
+                return self.async_abort(reason="already_configured")
+        try:
+            projector = await validate_projector(
+                hass=self.hass,
+                host=import_config[CONF_HOST],
+                check_power=True,
+                check_powered_on=False,
+            )
+        except CannotConnect:
+            _LOGGER.warning("Cannot connect to projector")
+            return self.async_abort(reason="cannot_connect")
+
+        serial_no = await projector.get_serial_number()
+        await self.async_set_unique_id(serial_no)
+        self._abort_if_unique_id_configured()
+        import_config.pop(CONF_PORT, None)
+        return self.async_create_entry(
+            title=import_config.pop(CONF_NAME), data=import_config
+        )
 
     async def async_step_user(self, user_input=None):
         """Handle the initial step."""
         errors = {}
         if user_input is not None:
             try:
-                await validate_projector(
+                projector = await validate_projector(
-                    self.hass, user_input[CONF_HOST], user_input[CONF_PORT]
+                    hass=self.hass,
+                    host=user_input[CONF_HOST],
+                    check_power=True,
+                    check_powered_on=True,
                 )
             except CannotConnect:
                 errors["base"] = "cannot_connect"
+            except PoweredOff:
+                _LOGGER.warning(
+                    "You need to turn ON projector for initial configuration"
+                )
+                errors["base"] = "powered_off"
             else:
+                serial_no = await projector.get_serial_number()
+                await self.async_set_unique_id(serial_no)
+                self._abort_if_unique_id_configured()
+                user_input.pop(CONF_PORT, None)
                 return self.async_create_entry(
                     title=user_input.pop(CONF_NAME), data=user_input
                 )

homeassistant/components/epson/const.py

@@ -4,5 +4,5 @@ DOMAIN = "epson"
 SERVICE_SELECT_CMODE = "select_cmode"
 
 ATTR_CMODE = "cmode"
-
 DEFAULT_NAME = "EPSON Projector"
+HTTP = "http"

homeassistant/components/epson/exceptions.py

@@ -4,3 +4,7 @@ from homeassistant import exceptions
 
 class CannotConnect(exceptions.HomeAssistantError):
     """Error to indicate we cannot connect."""
+
+
+class PoweredOff(exceptions.HomeAssistantError):
+    """Error to indicate projector is off."""

homeassistant/components/epson/manifest.json

@@ -3,7 +3,7 @@
   "name": "Epson",
   "config_flow": true,
   "documentation": "https://www.home-assistant.io/integrations/epson",
-  "requirements": ["epson-projector==0.2.3"],
+  "requirements": ["epson-projector==0.4.2"],
   "codeowners": ["@pszafer"],
   "iot_class": "local_polling"
 }

homeassistant/components/epson/media_player.py

@@ -40,6 +40,7 @@ from homeassistant.config_entries import SOURCE_IMPORT
 from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT, STATE_OFF, STATE_ON
 from homeassistant.helpers import entity_platform
 import homeassistant.helpers.config_validation as cv
+from homeassistant.helpers.entity_registry import async_get as async_get_entity_registry
 
 from .const import ATTR_CMODE, DEFAULT_NAME, DOMAIN, SERVICE_SELECT_CMODE
 
@@ -66,10 +67,14 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend(
 
 async def async_setup_entry(hass, config_entry, async_add_entities):
     """Set up the Epson projector from a config entry."""
-    unique_id = config_entry.entry_id
+    entry_id = config_entry.entry_id
-    projector = hass.data[DOMAIN][unique_id]
+    unique_id = config_entry.unique_id
+    projector = hass.data[DOMAIN][entry_id]
     projector_entity = EpsonProjectorMediaPlayer(
-        projector, config_entry.title, unique_id
+        projector=projector,
+        name=config_entry.title,
+        unique_id=unique_id,
+        entry=config_entry,
     )
     async_add_entities([projector_entity], True)
     platform = entity_platform.async_get_current_platform()
@@ -92,10 +97,11 @@ async def async_setup_platform(hass, config, async_add_entities, discovery_info=
 class EpsonProjectorMediaPlayer(MediaPlayerEntity):
     """Representation of Epson Projector Device."""
 
-    def __init__(self, projector, name, unique_id):
+    def __init__(self, projector, name, unique_id, entry):
         """Initialize entity to control Epson projector."""
-        self._name = name
         self._projector = projector
+        self._entry = entry
+        self._name = name
         self._available = False
         self._cmode = None
         self._source_list = list(DEFAULT_SOURCES.values())
@@ -104,9 +110,28 @@ class EpsonProjectorMediaPlayer(MediaPlayerEntity):
         self._state = None
         self._unique_id = unique_id
 
+    async def set_unique_id(self):
+        """Set unique id for projector config entry."""
+        _LOGGER.debug("Setting unique_id for projector")
+        if self._unique_id:
+            return False
+        uid = await self._projector.get_serial_number()
+        if uid:
+            self.hass.config_entries.async_update_entry(self._entry, unique_id=uid)
+            registry = async_get_entity_registry(self.hass)
+            old_entity_id = registry.async_get_entity_id(
+                "media_player", DOMAIN, self._entry.entry_id
+            )
+            if old_entity_id is not None:
+                registry.async_update_entity(old_entity_id, new_unique_id=uid)
+            self.hass.async_create_task(
+                self.hass.config_entries.async_reload(self._entry.entry_id)
+            )
+            return True
+
     async def async_update(self):
         """Update state of device."""
-        power_state = await self._projector.get_property(POWER)
+        power_state = await self._projector.get_power()
         _LOGGER.debug("Projector status: %s", power_state)
         if not power_state or power_state == EPSON_STATE_UNAVAILABLE:
             self._available = False
@@ -114,6 +139,8 @@ class EpsonProjectorMediaPlayer(MediaPlayerEntity):
         self._available = True
         if power_state == EPSON_CODES[POWER]:
             self._state = STATE_ON
+            if await self.set_unique_id():
+                return
             self._source_list = list(DEFAULT_SOURCES.values())
             cmode = await self._projector.get_property(CMODE)
             self._cmode = CMODE_LIST.get(cmode, self._cmode)
@@ -127,6 +154,19 @@ class EpsonProjectorMediaPlayer(MediaPlayerEntity):
         else:
             self._state = STATE_OFF
 
+    @property
+    def device_info(self):
+        """Get attributes about the device."""
+        if not self._unique_id:
+            return None
+        return {
+            "identifiers": {(DOMAIN, self._unique_id)},
+            "manufacturer": "Epson",
+            "name": "Epson projector",
+            "model": "Epson",
+            "via_hub": (DOMAIN, self._unique_id),
+        }
+
     @property
     def name(self):
         """Return the name of the device."""

homeassistant/components/epson/strings.json

@@ -4,13 +4,13 @@
       "user": {
         "data": {
           "host": "[%key:common::config_flow::data::host%]",
-          "name": "[%key:common::config_flow::data::name%]",
+          "name": "[%key:common::config_flow::data::name%]"
-          "port": "[%key:common::config_flow::data::port%]"
         }
       }
     },
     "error": {
-      "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]"
+      "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]",
+      "powered_off": "Is projector turned on? You need to turn on projector for initial configuration."
     }
   }
 }

homeassistant/components/epson/translations/en.json

@@ -1,14 +1,14 @@
 {
     "config": {
         "error": {
-            "cannot_connect": "Failed to connect"
+            "cannot_connect": "Failed to connect",
+            "powered_off": "Is projector turned on? You need to turn on projector for initial configuration."
         },
         "step": {
             "user": {
                 "data": {
                     "host": "Host",
-                    "name": "Name",
+                    "name": "Name"
-                    "port": "Port"
                 }
             }
         }

requirements_all.txt

@@ -563,7 +563,7 @@ envoy_reader==0.18.4
 ephem==3.7.7.0
 
 # homeassistant.components.epson
-epson-projector==0.2.3
+epson-projector==0.4.2
 
 # homeassistant.components.epsonworkforce
 epsonprinter==0.0.9

requirements_test_all.txt

@@ -309,7 +309,7 @@ envoy_reader==0.18.4
 ephem==3.7.7.0
 
 # homeassistant.components.epson
-epson-projector==0.2.3
+epson-projector==0.4.2
 
 # homeassistant.components.faa_delays
 faadelays==0.0.7

tests/components/epson/test_config_flow.py

@@ -1,35 +1,40 @@
 """Test the epson config flow."""
 from unittest.mock import patch
 
+from epson_projector.const import PWR_OFF_STATE
+
 from homeassistant import config_entries, setup
 from homeassistant.components.epson.const import DOMAIN
-from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT, STATE_UNAVAILABLE
+from homeassistant.const import CONF_HOST, CONF_NAME, STATE_UNAVAILABLE
+
+from tests.common import MockConfigEntry
 
 
 async def test_form(hass):
     """Test we get the form."""
     await setup.async_setup_component(hass, "persistent_notification", {})
+    with patch("homeassistant.components.epson.Projector.get_power", return_value="01"):
         result = await hass.config_entries.flow.async_init(
             DOMAIN, context={"source": config_entries.SOURCE_USER}
         )
     assert result["type"] == "form"
     assert result["errors"] == {}
     assert result["step_id"] == config_entries.SOURCE_USER
-
     with patch(
-        "homeassistant.components.epson.Projector.get_property",
+        "homeassistant.components.epson.Projector.get_power",
-        return_value="04",
+        return_value="01",
     ), patch(
         "homeassistant.components.epson.async_setup_entry",
         return_value=True,
     ) as mock_setup_entry:
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
-            {CONF_HOST: "1.1.1.1", CONF_NAME: "test-epson", CONF_PORT: 80},
+            {CONF_HOST: "1.1.1.1", CONF_NAME: "test-epson"},
         )
+
     assert result2["type"] == "create_entry"
     assert result2["title"] == "test-epson"
-    assert result2["data"] == {CONF_HOST: "1.1.1.1", CONF_PORT: 80}
+    assert result2["data"] == {CONF_HOST: "1.1.1.1"}
     await hass.async_block_till_done()
     assert len(mock_setup_entry.mock_calls) == 1
 
@@ -41,21 +46,43 @@ async def test_form_cannot_connect(hass):
     )
 
     with patch(
-        "homeassistant.components.epson.Projector.get_property",
+        "homeassistant.components.epson.Projector.get_power",
         return_value=STATE_UNAVAILABLE,
     ):
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
-            {CONF_HOST: "1.1.1.1", CONF_NAME: "test-epson", CONF_PORT: 80},
+            {CONF_HOST: "1.1.1.1", CONF_NAME: "test-epson"},
         )
 
     assert result2["type"] == "form"
     assert result2["errors"] == {"base": "cannot_connect"}
 
 
+async def test_form_powered_off(hass):
+    """Test we handle powered off during initial configuration."""
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN, context={"source": config_entries.SOURCE_USER}
+    )
+
+    with patch(
+        "homeassistant.components.epson.Projector.get_power",
+        return_value=PWR_OFF_STATE,
+    ):
+        result2 = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            {CONF_HOST: "1.1.1.1", CONF_NAME: "test-epson"},
+        )
+
+    assert result2["type"] == "form"
+    assert result2["errors"] == {"base": "powered_off"}
+
+
 async def test_import(hass):
     """Test config.yaml import."""
     with patch(
+        "homeassistant.components.epson.Projector.get_power",
+        return_value="01",
+    ), patch(
         "homeassistant.components.epson.Projector.get_property",
         return_value="04",
     ), patch(
@@ -65,27 +92,53 @@ async def test_import(hass):
         result = await hass.config_entries.flow.async_init(
             DOMAIN,
             context={"source": config_entries.SOURCE_IMPORT},
-            data={CONF_HOST: "1.1.1.1", CONF_NAME: "test-epson", CONF_PORT: 80},
+            data={CONF_HOST: "1.1.1.1", CONF_NAME: "test-epson"},
         )
     assert result["type"] == "create_entry"
     assert result["title"] == "test-epson"
-        assert result["data"] == {CONF_HOST: "1.1.1.1", CONF_PORT: 80}
+    assert result["data"] == {CONF_HOST: "1.1.1.1"}
+
+
+async def test_already_imported(hass):
+    """Test config.yaml imported twice."""
+    MockConfigEntry(
+        domain=DOMAIN,
+        source=config_entries.SOURCE_IMPORT,
+        unique_id="bla",
+        title="test-epson",
+        data={CONF_HOST: "1.1.1.1"},
+    ).add_to_hass(hass)
+
+    with patch(
+        "homeassistant.components.epson.Projector.get_power",
+        return_value="01",
+    ), patch(
+        "homeassistant.components.epson.Projector.get_property",
+        return_value="04",
+    ), patch(
+        "homeassistant.components.epson.async_setup_entry",
+        return_value=True,
+    ):
+        result = await hass.config_entries.flow.async_init(
+            DOMAIN,
+            context={"source": config_entries.SOURCE_IMPORT},
+            data={CONF_HOST: "1.1.1.1", CONF_NAME: "test-epson"},
+        )
+    assert result["type"] == "abort"
+    assert result["reason"] == "already_configured"
 
 
 async def test_import_cannot_connect(hass):
-    """Test we handle cannot connect error with import."""
+    """Test we handle cannot connect error."""
-    result = await hass.config_entries.flow.async_init(
-        DOMAIN, context={"source": config_entries.SOURCE_IMPORT}
-    )
-
     with patch(
-        "homeassistant.components.epson.Projector.get_property",
+        "homeassistant.components.epson.Projector.get_power",
         return_value=STATE_UNAVAILABLE,
     ):
-        result2 = await hass.config_entries.flow.async_configure(
+        result = await hass.config_entries.flow.async_init(
-            result["flow_id"],
+            DOMAIN,
-            {CONF_HOST: "1.1.1.1", CONF_NAME: "test-epson", CONF_PORT: 80},
+            context={"source": config_entries.SOURCE_IMPORT},
+            data={CONF_HOST: "1.1.1.1", CONF_NAME: "test-epson"},
         )
 
-    assert result2["type"] == "form"
+    assert result["type"] == "abort"
-    assert result2["errors"] == {"base": "cannot_connect"}
+    assert result["reason"] == "cannot_connect"