Elmax/sensors improvements (#74323)

* Address suggestions in PR #64090

* Remove redundant force-refresh at discovery time

* Improve re-auth UX

* Resolve linting warning

* Cover reauth case when no entry is in place

* Add missing reauth_successful message

* Use ad-hoc schema for reauth flow

* Align test cases with latest modifications

* Improve re-authentication flow, align tests

* Remove None check (as it can never happen)

* Remove panel_id from reauth dialog

* Remove panel_id from reauth dialog

* Simplify try-catch block

* Address suggestions in PR #64090

* Remove redundant force-refresh at discovery time

* Improve re-auth UX

* Resolve linting warning

* Cover reauth case when no entry is in place

* Add missing reauth_successful message

* Use ad-hoc schema for reauth flow

* Align test cases with latest modifications

* Improve re-authentication flow, align tests

* Remove None check (as it can never happen)

* Remove panel_id from reauth dialog

* Remove panel_id from reauth dialog

* Simplify try-catch block

* Improve type handling

* Remove translations

* Add unique-id attribute within context object

* Optimize local variable usage

homeassistant/components/elmax/binary_sensor.py

@@ -44,11 +44,14 @@ async def async_setup_entry(
                 coordinator=coordinator,
             )
             entities.append(entity)
-        async_add_entities(entities, True)
+
-        known_devices.update([e.unique_id for e in entities])
+        if entities:
+            async_add_entities(entities)
+            known_devices.update([e.unique_id for e in entities])
 
     # Register a listener for the discovery of new devices
-    coordinator.async_add_listener(_discover_new_devices)
+    remove_handle = coordinator.async_add_listener(_discover_new_devices)
+    config_entry.async_on_unload(remove_handle)
 
     # Immediately run a discovery, so we don't need to wait for the next update
     _discover_new_devices()

homeassistant/components/elmax/config_flow.py

@@ -32,6 +32,14 @@ LOGIN_FORM_SCHEMA = vol.Schema(
     }
 )
 
+REAUTH_FORM_SCHEMA = vol.Schema(
+    {
+        vol.Required(CONF_ELMAX_USERNAME): str,
+        vol.Required(CONF_ELMAX_PASSWORD): str,
+        vol.Required(CONF_ELMAX_PANEL_PIN): str,
+    }
+)
+
 
 def _store_panel_by_name(
     panel: PanelEntry, username: str, panel_names: dict[str, str]
@@ -56,8 +64,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
     _password: str
     _panels_schema: vol.Schema
     _panel_names: dict
-    _reauth_username: str | None
+    _entry: config_entries.ConfigEntry | None
-    _reauth_panelid: str | None
 
     async def async_step_user(
         self, user_input: dict[str, Any] | None = None
@@ -170,82 +177,64 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
 
     async def async_step_reauth(self, entry_data: Mapping[str, Any]) -> FlowResult:
         """Perform reauth upon an API authentication error."""
-        self._reauth_username = entry_data.get(CONF_ELMAX_USERNAME)
+        self._entry = self.hass.config_entries.async_get_entry(self.context["entry_id"])
-        self._reauth_panelid = entry_data.get(CONF_ELMAX_PANEL_ID)
         return await self.async_step_reauth_confirm()
 
-    async def async_step_reauth_confirm(self, user_input=None):
+    async def async_step_reauth_confirm(
+        self, user_input: dict[str, Any] | None = None
+    ) -> FlowResult:
         """Handle reauthorization flow."""
         errors = {}
         if user_input is not None:
-            panel_pin = user_input.get(CONF_ELMAX_PANEL_PIN)
+            username = user_input[CONF_ELMAX_USERNAME]
-            password = user_input.get(CONF_ELMAX_PASSWORD)
+            password = user_input[CONF_ELMAX_PASSWORD]
-            entry = await self.async_set_unique_id(self._reauth_panelid)
+            panel_pin = user_input[CONF_ELMAX_PANEL_PIN]
 
             # Handle authentication, make sure the panel we are re-authenticating against is listed among results
             # and verify its pin is correct.
+            assert self._entry is not None
             try:
                 # Test login.
-                client = await self._async_login(
+                client = await self._async_login(username=username, password=password)
-                    username=self._reauth_username, password=password
-                )
-
                 # Make sure the panel we are authenticating to is still available.
                 panels = [
                     p
                     for p in await client.list_control_panels()
-                    if p.hash == self._reauth_panelid
+                    if p.hash == self._entry.data[CONF_ELMAX_PANEL_ID]
                 ]
                 if len(panels) < 1:
                     raise NoOnlinePanelsError()
 
-                # Verify the pin is still valid.from
+                # Verify the pin is still valid.
                 await client.get_panel_status(
-                    control_panel_id=self._reauth_panelid, pin=panel_pin
+                    control_panel_id=self._entry.data[CONF_ELMAX_PANEL_ID],
+                    pin=panel_pin,
                 )
 
-                # If it is, proceed with configuration update.
-                self.hass.config_entries.async_update_entry(
-                    entry,
-                    data={
-                        CONF_ELMAX_PANEL_ID: self._reauth_panelid,
-                        CONF_ELMAX_PANEL_PIN: panel_pin,
-                        CONF_ELMAX_USERNAME: self._reauth_username,
-                        CONF_ELMAX_PASSWORD: password,
-                    },
-                )
-                await self.hass.config_entries.async_reload(entry.entry_id)
-                self._reauth_username = None
-                self._reauth_panelid = None
-                return self.async_abort(reason="reauth_successful")
-
             except ElmaxBadLoginError:
-                _LOGGER.error(
-                    "Wrong credentials or failed login while re-authenticating"
-                )
                 errors["base"] = "invalid_auth"
             except NoOnlinePanelsError:
-                _LOGGER.warning(
-                    "Panel ID %s is no longer associated to this user",
-                    self._reauth_panelid,
-                )
                 errors["base"] = "reauth_panel_disappeared"
             except ElmaxBadPinError:
                 errors["base"] = "invalid_pin"
 
-        # We want the user to re-authenticate only for the given panel id using the same login.
+            # If all went right, update the config entry
-        # We pin them to the UI, so the user realizes she must log in with the appropriate credentials
+            if not errors:
-        # for the that specific panel.
+                self.hass.config_entries.async_update_entry(
-        schema = vol.Schema(
+                    self._entry,
-            {
+                    data={
-                vol.Required(CONF_ELMAX_USERNAME): self._reauth_username,
+                        CONF_ELMAX_PANEL_ID: self._entry.data[CONF_ELMAX_PANEL_ID],
-                vol.Required(CONF_ELMAX_PASSWORD): str,
+                        CONF_ELMAX_PANEL_PIN: panel_pin,
-                vol.Required(CONF_ELMAX_PANEL_ID): self._reauth_panelid,
+                        CONF_ELMAX_USERNAME: username,
-                vol.Required(CONF_ELMAX_PANEL_PIN): str,
+                        CONF_ELMAX_PASSWORD: password,
-            }
+                    },
-        )
+                )
+                await self.hass.config_entries.async_reload(self._entry.entry_id)
+                return self.async_abort(reason="reauth_successful")
+
+        # Otherwise start over and show the relative error message
         return self.async_show_form(
-            step_id="reauth_confirm", data_schema=schema, errors=errors
+            step_id="reauth_confirm", data_schema=REAUTH_FORM_SCHEMA, errors=errors
         )
 
     @staticmethod

homeassistant/components/elmax/strings.json

@@ -15,6 +15,14 @@
           "panel_id": "Panel ID",
           "panel_pin": "PIN Code"
         }
+      },
+      "reauth_confirm": {
+        "description": "Please re-authenticate with the panel.",
+        "data": {
+          "password": "[%key:common::config_flow::data::password%]",
+          "username": "[%key:common::config_flow::data::username%]",
+          "panel_pin": "Panel Pin"
+        }
       }
     },
     "error": {
@@ -22,10 +30,12 @@
       "invalid_auth": "[%key:common::config_flow::error::invalid_auth%]",
       "network_error": "A network error occurred",
       "invalid_pin": "The provided pin is invalid",
+      "reauth_panel_disappeared": "The given panel is no longer associated to this user. Please log in using an account associated to this panel.",
       "unknown": "[%key:common::config_flow::error::unknown%]"
     },
     "abort": {
-      "already_configured": "[%key:common::config_flow::abort::already_configured_device%]"
+      "already_configured": "[%key:common::config_flow::abort::already_configured_device%]",
+      "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]"
     }
   }
 }

homeassistant/components/elmax/switch.py

@@ -36,19 +36,24 @@ async def async_setup_entry(
         # Otherwise, add all the entities we found
         entities = []
         for actuator in panel_status.actuators:
+            # Skip already handled devices
+            if actuator.endpoint_id in known_devices:
+                continue
             entity = ElmaxSwitch(
                 panel=coordinator.panel_entry,
                 elmax_device=actuator,
                 panel_version=panel_status.release,
                 coordinator=coordinator,
             )
-            if entity.unique_id not in known_devices:
+            entities.append(entity)
-                entities.append(entity)
+
-        async_add_entities(entities, True)
+        if entities:
-        known_devices.update([entity.unique_id for entity in entities])
+            async_add_entities(entities)
+            known_devices.update([entity.unique_id for entity in entities])
 
     # Register a listener for the discovery of new devices
-    coordinator.async_add_listener(_discover_new_devices)
+    remove_handle = coordinator.async_add_listener(_discover_new_devices)
+    config_entry.async_on_unload(remove_handle)
 
     # Immediately run a discovery, so we don't need to wait for the next update
     _discover_new_devices()

tests/components/elmax/test_config_flow.py

@@ -223,9 +223,25 @@ async def test_no_online_panel(hass: HomeAssistant) -> None:
 
 async def test_show_reauth(hass: HomeAssistant) -> None:
     """Test that the reauth form shows."""
+    entry = MockConfigEntry(
+        domain=DOMAIN,
+        data={
+            CONF_ELMAX_PANEL_ID: MOCK_PANEL_ID,
+            CONF_ELMAX_USERNAME: MOCK_USERNAME,
+            CONF_ELMAX_PASSWORD: MOCK_PASSWORD,
+            CONF_ELMAX_PANEL_PIN: MOCK_PANEL_PIN,
+        },
+        unique_id=MOCK_PANEL_ID,
+    )
+    entry.add_to_hass(hass)
+
     result = await hass.config_entries.flow.async_init(
         DOMAIN,
-        context={"source": SOURCE_REAUTH},
+        context={
+            "source": SOURCE_REAUTH,
+            "unique_id": entry.unique_id,
+            "entry_id": entry.entry_id,
+        },
         data={
             CONF_ELMAX_PANEL_ID: MOCK_PANEL_ID,
             CONF_ELMAX_PANEL_PIN: MOCK_PANEL_PIN,
@@ -239,7 +255,7 @@ async def test_show_reauth(hass: HomeAssistant) -> None:
 
 async def test_reauth_flow(hass: HomeAssistant) -> None:
     """Test that the reauth flow works."""
-    MockConfigEntry(
+    entry = MockConfigEntry(
         domain=DOMAIN,
         data={
             CONF_ELMAX_PANEL_ID: MOCK_PANEL_ID,
@@ -248,7 +264,8 @@ async def test_reauth_flow(hass: HomeAssistant) -> None:
             CONF_ELMAX_PANEL_PIN: MOCK_PANEL_PIN,
         },
         unique_id=MOCK_PANEL_ID,
-    ).add_to_hass(hass)
+    )
+    entry.add_to_hass(hass)
 
     # Trigger reauth
     with patch(
@@ -257,7 +274,11 @@ async def test_reauth_flow(hass: HomeAssistant) -> None:
     ):
         reauth_result = await hass.config_entries.flow.async_init(
             DOMAIN,
-            context={"source": SOURCE_REAUTH},
+            context={
+                "source": SOURCE_REAUTH,
+                "unique_id": entry.unique_id,
+                "entry_id": entry.entry_id,
+            },
             data={
                 CONF_ELMAX_PANEL_ID: MOCK_PANEL_ID,
                 CONF_ELMAX_PANEL_PIN: MOCK_PANEL_PIN,
@@ -268,7 +289,6 @@ async def test_reauth_flow(hass: HomeAssistant) -> None:
         result = await hass.config_entries.flow.async_configure(
             reauth_result["flow_id"],
             {
-                CONF_ELMAX_PANEL_ID: MOCK_PANEL_ID,
                 CONF_ELMAX_PANEL_PIN: MOCK_PANEL_PIN,
                 CONF_ELMAX_USERNAME: MOCK_USERNAME,
                 CONF_ELMAX_PASSWORD: MOCK_PASSWORD,
@@ -282,7 +302,7 @@ async def test_reauth_flow(hass: HomeAssistant) -> None:
 async def test_reauth_panel_disappeared(hass: HomeAssistant) -> None:
     """Test that the case where panel is no longer associated with the user."""
     # Simulate a first setup
-    MockConfigEntry(
+    entry = MockConfigEntry(
         domain=DOMAIN,
         data={
             CONF_ELMAX_PANEL_ID: MOCK_PANEL_ID,
@@ -291,7 +311,8 @@ async def test_reauth_panel_disappeared(hass: HomeAssistant) -> None:
             CONF_ELMAX_PANEL_PIN: MOCK_PANEL_PIN,
         },
         unique_id=MOCK_PANEL_ID,
-    ).add_to_hass(hass)
+    )
+    entry.add_to_hass(hass)
 
     # Trigger reauth
     with patch(
@@ -300,7 +321,11 @@ async def test_reauth_panel_disappeared(hass: HomeAssistant) -> None:
     ):
         reauth_result = await hass.config_entries.flow.async_init(
             DOMAIN,
-            context={"source": SOURCE_REAUTH},
+            context={
+                "source": SOURCE_REAUTH,
+                "unique_id": entry.unique_id,
+                "entry_id": entry.entry_id,
+            },
             data={
                 CONF_ELMAX_PANEL_ID: MOCK_PANEL_ID,
                 CONF_ELMAX_PANEL_PIN: MOCK_PANEL_PIN,
@@ -311,7 +336,6 @@ async def test_reauth_panel_disappeared(hass: HomeAssistant) -> None:
         result = await hass.config_entries.flow.async_configure(
             reauth_result["flow_id"],
             {
-                CONF_ELMAX_PANEL_ID: MOCK_PANEL_ID,
                 CONF_ELMAX_PANEL_PIN: MOCK_PANEL_PIN,
                 CONF_ELMAX_USERNAME: MOCK_USERNAME,
                 CONF_ELMAX_PASSWORD: MOCK_PASSWORD,
@@ -324,7 +348,7 @@ async def test_reauth_panel_disappeared(hass: HomeAssistant) -> None:
 
 async def test_reauth_invalid_pin(hass: HomeAssistant) -> None:
     """Test that the case where panel is no longer associated with the user."""
-    MockConfigEntry(
+    entry = MockConfigEntry(
         domain=DOMAIN,
         data={
             CONF_ELMAX_PANEL_ID: MOCK_PANEL_ID,
@@ -333,7 +357,8 @@ async def test_reauth_invalid_pin(hass: HomeAssistant) -> None:
             CONF_ELMAX_PANEL_PIN: MOCK_PANEL_PIN,
         },
         unique_id=MOCK_PANEL_ID,
-    ).add_to_hass(hass)
+    )
+    entry.add_to_hass(hass)
 
     # Trigger reauth
     with patch(
@@ -342,7 +367,11 @@ async def test_reauth_invalid_pin(hass: HomeAssistant) -> None:
     ):
         reauth_result = await hass.config_entries.flow.async_init(
             DOMAIN,
-            context={"source": SOURCE_REAUTH},
+            context={
+                "source": SOURCE_REAUTH,
+                "unique_id": entry.unique_id,
+                "entry_id": entry.entry_id,
+            },
             data={
                 CONF_ELMAX_PANEL_ID: MOCK_PANEL_ID,
                 CONF_ELMAX_PANEL_PIN: MOCK_PANEL_PIN,
@@ -353,7 +382,6 @@ async def test_reauth_invalid_pin(hass: HomeAssistant) -> None:
         result = await hass.config_entries.flow.async_configure(
             reauth_result["flow_id"],
             {
-                CONF_ELMAX_PANEL_ID: MOCK_PANEL_ID,
                 CONF_ELMAX_PANEL_PIN: MOCK_PANEL_PIN,
                 CONF_ELMAX_USERNAME: MOCK_USERNAME,
                 CONF_ELMAX_PASSWORD: MOCK_PASSWORD,
@@ -366,7 +394,7 @@ async def test_reauth_invalid_pin(hass: HomeAssistant) -> None:
 
 async def test_reauth_bad_login(hass: HomeAssistant) -> None:
     """Test bad login attempt at reauth time."""
-    MockConfigEntry(
+    entry = MockConfigEntry(
         domain=DOMAIN,
         data={
             CONF_ELMAX_PANEL_ID: MOCK_PANEL_ID,
@@ -375,7 +403,8 @@ async def test_reauth_bad_login(hass: HomeAssistant) -> None:
             CONF_ELMAX_PANEL_PIN: MOCK_PANEL_PIN,
         },
         unique_id=MOCK_PANEL_ID,
-    ).add_to_hass(hass)
+    )
+    entry.add_to_hass(hass)
 
     # Trigger reauth
     with patch(
@@ -384,7 +413,11 @@ async def test_reauth_bad_login(hass: HomeAssistant) -> None:
     ):
         reauth_result = await hass.config_entries.flow.async_init(
             DOMAIN,
-            context={"source": SOURCE_REAUTH},
+            context={
+                "source": SOURCE_REAUTH,
+                "unique_id": entry.unique_id,
+                "entry_id": entry.entry_id,
+            },
             data={
                 CONF_ELMAX_PANEL_ID: MOCK_PANEL_ID,
                 CONF_ELMAX_PANEL_PIN: MOCK_PANEL_PIN,
@@ -395,7 +428,6 @@ async def test_reauth_bad_login(hass: HomeAssistant) -> None:
         result = await hass.config_entries.flow.async_configure(
             reauth_result["flow_id"],
             {
-                CONF_ELMAX_PANEL_ID: MOCK_PANEL_ID,
                 CONF_ELMAX_PANEL_PIN: MOCK_PANEL_PIN,
                 CONF_ELMAX_USERNAME: MOCK_USERNAME,
                 CONF_ELMAX_PASSWORD: MOCK_PASSWORD,