Add reconfiguration flow to Plugwise (#132878)

Co-authored-by: Abílio Costa <abmantis@users.noreply.github.com>
Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>

homeassistant/components/plugwise/config_flow.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import logging
 from typing import Any, Self
 
 from plugwise import Smile
@@ -41,8 +42,16 @@ from .const import (
     ZEROCONF_MAP,
 )
 
+_LOGGER = logging.getLogger(__name__)
 
-def base_schema(discovery_info: ZeroconfServiceInfo | None) -> vol.Schema:
+SMILE_RECONF_SCHEMA = vol.Schema(
+    {
+        vol.Required(CONF_HOST): str,
+    }
+)
+
+
+def smile_user_schema(discovery_info: ZeroconfServiceInfo | None) -> vol.Schema:
     """Generate base schema for gateways."""
     schema = vol.Schema({vol.Required(CONF_PASSWORD): str})
 
@@ -50,6 +59,7 @@ def base_schema(discovery_info: ZeroconfServiceInfo | None) -> vol.Schema:
         schema = schema.extend(
             {
                 vol.Required(CONF_HOST): str,
+                # Port under investigation for removal (hence not added in #132878)
                 vol.Optional(CONF_PORT, default=DEFAULT_PORT): int,
                 vol.Required(CONF_USERNAME, default=SMILE): vol.In(
                     {SMILE: FLOW_SMILE, STRETCH: FLOW_STRETCH}
@@ -63,7 +73,7 @@ def base_schema(discovery_info: ZeroconfServiceInfo | None) -> vol.Schema:
 async def validate_input(hass: HomeAssistant, data: dict[str, Any]) -> Smile:
     """Validate whether the user input allows us to connect to the gateway.
 
-    Data has the keys from base_schema() with values provided by the user.
+    Data has the keys from the schema with values provided by the user.
     """
     websession = async_get_clientsession(hass, verify_ssl=False)
     api = Smile(
@@ -77,6 +87,32 @@ async def validate_input(hass: HomeAssistant, data: dict[str, Any]) -> Smile:
     return api
 
 
+async def verify_connection(
+    hass: HomeAssistant, user_input: dict[str, Any]
+) -> tuple[Smile | None, dict[str, str]]:
+    """Verify and return the gateway connection or an error."""
+    errors: dict[str, str] = {}
+
+    try:
+        return (await validate_input(hass, user_input), errors)
+    except ConnectionFailedError:
+        errors[CONF_BASE] = "cannot_connect"
+    except InvalidAuthentication:
+        errors[CONF_BASE] = "invalid_auth"
+    except InvalidSetupError:
+        errors[CONF_BASE] = "invalid_setup"
+    except (InvalidXMLError, ResponseError):
+        errors[CONF_BASE] = "response_error"
+    except UnsupportedDeviceError:
+        errors[CONF_BASE] = "unsupported"
+    except Exception:  # noqa: BLE001
+        _LOGGER.exception(
+            "Unknown exception while verifying connection with your Plugwise Smile"
+        )
+        errors[CONF_BASE] = "unknown"
+    return (None, errors)
+
+
 class PlugwiseConfigFlow(ConfigFlow, domain=DOMAIN):
     """Handle a config flow for Plugwise Smile."""
 
@@ -166,30 +202,56 @@ class PlugwiseConfigFlow(ConfigFlow, domain=DOMAIN):
                 user_input[CONF_PORT] = self.discovery_info.port
                 user_input[CONF_USERNAME] = self._username
 
-            try:
+            api, errors = await verify_connection(self.hass, user_input)
-                api = await validate_input(self.hass, user_input)
+            if api:
-            except ConnectionFailedError:
-                errors[CONF_BASE] = "cannot_connect"
-            except InvalidAuthentication:
-                errors[CONF_BASE] = "invalid_auth"
-            except InvalidSetupError:
-                errors[CONF_BASE] = "invalid_setup"
-            except (InvalidXMLError, ResponseError):
-                errors[CONF_BASE] = "response_error"
-            except UnsupportedDeviceError:
-                errors[CONF_BASE] = "unsupported"
-            except Exception:  # noqa: BLE001
-                errors[CONF_BASE] = "unknown"
-            else:
                 await self.async_set_unique_id(
-                    api.smile_hostname or api.gateway_id, raise_on_progress=False
+                    api.smile_hostname or api.gateway_id,
+                    raise_on_progress=False,
                 )
                 self._abort_if_unique_id_configured()
-
                 return self.async_create_entry(title=api.smile_name, data=user_input)
 
         return self.async_show_form(
             step_id=SOURCE_USER,
-            data_schema=base_schema(self.discovery_info),
+            data_schema=smile_user_schema(self.discovery_info),
+            errors=errors,
+        )
+
+    async def async_step_reconfigure(
+        self, user_input: dict[str, Any] | None = None
+    ) -> ConfigFlowResult:
+        """Handle reconfiguration of the integration."""
+        errors: dict[str, str] = {}
+
+        reconfigure_entry = self._get_reconfigure_entry()
+
+        if user_input:
+            # Keep current username and password
+            full_input = {
+                CONF_HOST: user_input.get(CONF_HOST),
+                CONF_PORT: reconfigure_entry.data.get(CONF_PORT),
+                CONF_USERNAME: reconfigure_entry.data.get(CONF_USERNAME),
+                CONF_PASSWORD: reconfigure_entry.data.get(CONF_PASSWORD),
+            }
+
+            api, errors = await verify_connection(self.hass, full_input)
+            if api:
+                await self.async_set_unique_id(
+                    api.smile_hostname or api.gateway_id,
+                    raise_on_progress=False,
+                )
+                self._abort_if_unique_id_mismatch(reason="not_the_same_smile")
+                return self.async_update_reload_and_abort(
+                    reconfigure_entry,
+                    data_updates=full_input,
+                )
+
+        return self.async_show_form(
+            step_id="reconfigure",
+            data_schema=self.add_suggested_values_to_schema(
+                data_schema=SMILE_RECONF_SCHEMA,
+                suggested_values=reconfigure_entry.data,
+            ),
+            description_placeholders={"title": reconfigure_entry.title},
             errors=errors,
         )

homeassistant/components/plugwise/quality_scale.yaml

@@ -52,9 +52,7 @@ rules:
   diagnostics: done
   exception-translations: done
   icon-translations: done
-  reconfiguration-flow:
+  reconfiguration-flow: done
-    status: todo
-    comment: This integration does not have any reconfiguration steps (yet) investigate how/why
   dynamic-devices: done
   discovery-update-info: done
   repair-issues:

homeassistant/components/plugwise/strings.json

@@ -1,12 +1,23 @@
 {
   "config": {
     "step": {
+      "reconfigure": {
+        "description": "Update configuration for {title}.",
+        "data": {
+          "host": "[%key:common::config_flow::data::ip%]",
+          "port": "[%key:common::config_flow::data::port%]"
+        },
+        "data_description": {
+          "host": "[%key:component::plugwise::config::step::user::data_description::host%]",
+          "port": "[%key:component::plugwise::config::step::user::data_description::port%]"
+        }
+      },
       "user": {
         "title": "Connect to the Smile",
         "description": "Please enter",
         "data": {
-          "password": "Smile ID",
           "host": "[%key:common::config_flow::data::ip%]",
+          "password": "Smile ID",
           "port": "[%key:common::config_flow::data::port%]",
           "username": "Smile Username"
         },
@@ -28,7 +39,9 @@
     },
     "abort": {
       "already_configured": "[%key:common::config_flow::abort::already_configured_service%]",
-      "anna_with_adam": "Both Anna and Adam detected. Add your Adam instead of your Anna"
+      "anna_with_adam": "Both Anna and Adam detected. Add your Adam instead of your Anna",
+      "not_the_same_smile": "The configured Smile ID does not match the Smile ID on the requested IP address.",
+      "reconfigure_successful": "[%key:common::config_flow::abort::reconfigure_successful%]"
     }
   },
   "entity": {

tests/components/plugwise/conftest.py

@@ -77,9 +77,15 @@ def mock_smile_adam() -> Generator[MagicMock]:
     """Create a Mock Adam environment for testing exceptions."""
     chosen_env = "m_adam_multiple_devices_per_zone"
 
-    with patch(
+    with (
+        patch(
             "homeassistant.components.plugwise.coordinator.Smile", autospec=True
-    ) as smile_mock:
+        ) as smile_mock,
+        patch(
+            "homeassistant.components.plugwise.config_flow.Smile",
+            new=smile_mock,
+        ),
+    ):
         smile = smile_mock.return_value
 
         smile.gateway_id = "fe799307f1624099878210aa0b9f1475"

tests/components/plugwise/test_config_flow.py

@@ -14,7 +14,7 @@ import pytest
 
 from homeassistant.components.plugwise.const import DEFAULT_PORT, DOMAIN
 from homeassistant.components.zeroconf import ZeroconfServiceInfo
-from homeassistant.config_entries import SOURCE_USER, SOURCE_ZEROCONF
+from homeassistant.config_entries import SOURCE_USER, SOURCE_ZEROCONF, ConfigFlowResult
 from homeassistant.const import (
     CONF_HOST,
     CONF_NAME,
@@ -35,7 +35,7 @@ TEST_PASSWORD = "test_password"
 TEST_PORT = 81
 TEST_USERNAME = "smile"
 TEST_USERNAME2 = "stretch"
-MOCK_SMILE_ID = "smile12345"
+TEST_SMILE_HOST = "smile12345"
 
 TEST_DISCOVERY = ZeroconfServiceInfo(
     ip_address=ip_address(TEST_HOST),
@@ -129,7 +129,7 @@ async def test_form(
     assert len(mock_setup_entry.mock_calls) == 1
     assert len(mock_smile_config_flow.connect.mock_calls) == 1
 
-    assert result2["result"].unique_id == MOCK_SMILE_ID
+    assert result2["result"].unique_id == TEST_SMILE_HOST
 
 
 @pytest.mark.parametrize(
@@ -175,7 +175,7 @@ async def test_zeroconf_flow(
     assert len(mock_setup_entry.mock_calls) == 1
     assert len(mock_smile_config_flow.connect.mock_calls) == 1
 
-    assert result2["result"].unique_id == MOCK_SMILE_ID
+    assert result2["result"].unique_id == TEST_SMILE_HOST
 
 
 async def test_zeroconf_flow_stretch(
@@ -274,7 +274,7 @@ async def test_flow_errors(
     side_effect: Exception,
     reason: str,
 ) -> None:
-    """Test we handle invalid auth."""
+    """Test we handle each exception error."""
     result = await hass.config_entries.flow.async_init(
         DOMAIN,
         context={CONF_SOURCE: SOURCE_USER},
@@ -285,6 +285,7 @@ async def test_flow_errors(
     assert "flow_id" in result
 
     mock_smile_config_flow.connect.side_effect = side_effect
+
     result2 = await hass.config_entries.flow.async_configure(
         result["flow_id"],
         user_input={CONF_HOST: TEST_HOST, CONF_PASSWORD: TEST_PASSWORD},
@@ -330,7 +331,7 @@ async def test_user_abort_existing_anna(
             CONF_USERNAME: TEST_USERNAME,
             CONF_PASSWORD: TEST_PASSWORD,
         },
-        unique_id=MOCK_SMILE_ID,
+        unique_id=TEST_SMILE_HOST,
     )
     entry.add_to_hass(hass)
 
@@ -435,3 +436,91 @@ async def test_zeroconf_abort_anna_with_adam(hass: HomeAssistant) -> None:
     flows_in_progress = hass.config_entries.flow._handler_progress_index[DOMAIN]
     assert len(flows_in_progress) == 1
     assert list(flows_in_progress)[0].product == "smile_open_therm"
+
+
+async def _start_reconfigure_flow(
+    hass: HomeAssistant,
+    mock_config_entry: MockConfigEntry,
+    host_ip: str,
+) -> ConfigFlowResult:
+    """Initialize a reconfigure flow."""
+    mock_config_entry.add_to_hass(hass)
+
+    reconfigure_result = await mock_config_entry.start_reconfigure_flow(hass)
+
+    assert reconfigure_result["type"] is FlowResultType.FORM
+    assert reconfigure_result["step_id"] == "reconfigure"
+
+    return await hass.config_entries.flow.async_configure(
+        reconfigure_result["flow_id"], {CONF_HOST: host_ip}
+    )
+
+
+async def test_reconfigure_flow(
+    hass: HomeAssistant,
+    mock_smile_adam: AsyncMock,
+    mock_setup_entry: AsyncMock,
+    mock_config_entry: MockConfigEntry,
+) -> None:
+    """Test reconfigure flow."""
+    result = await _start_reconfigure_flow(hass, mock_config_entry, TEST_HOST)
+
+    assert result["type"] is FlowResultType.ABORT
+    assert result["reason"] == "reconfigure_successful"
+
+    assert mock_config_entry.data.get(CONF_HOST) == TEST_HOST
+
+
+async def test_reconfigure_flow_smile_mismatch(
+    hass: HomeAssistant,
+    mock_smile_adam: AsyncMock,
+    mock_setup_entry: AsyncMock,
+    mock_config_entry: MockConfigEntry,
+) -> None:
+    """Test reconfigure flow aborts on other Smile ID."""
+    mock_smile_adam.smile_hostname = TEST_SMILE_HOST
+
+    result = await _start_reconfigure_flow(hass, mock_config_entry, TEST_HOST)
+
+    assert result["type"] is FlowResultType.ABORT
+    assert result["reason"] == "not_the_same_smile"
+
+
+@pytest.mark.parametrize(
+    ("side_effect", "reason"),
+    [
+        (ConnectionFailedError, "cannot_connect"),
+        (InvalidAuthentication, "invalid_auth"),
+        (InvalidSetupError, "invalid_setup"),
+        (InvalidXMLError, "response_error"),
+        (RuntimeError, "unknown"),
+        (UnsupportedDeviceError, "unsupported"),
+    ],
+)
+async def test_reconfigure_flow_connect_errors(
+    hass: HomeAssistant,
+    mock_smile_adam: AsyncMock,
+    mock_config_entry: MockConfigEntry,
+    side_effect: Exception,
+    reason: str,
+) -> None:
+    """Test we handle each reconfigure exception error and recover."""
+
+    mock_smile_adam.connect.side_effect = side_effect
+
+    result = await _start_reconfigure_flow(hass, mock_config_entry, TEST_HOST)
+
+    assert result.get("type") is FlowResultType.FORM
+    assert result.get("errors") == {"base": reason}
+    assert result.get("step_id") == "reconfigure"
+
+    mock_smile_adam.connect.side_effect = None
+
+    result2 = await hass.config_entries.flow.async_configure(
+        result["flow_id"], {CONF_HOST: TEST_HOST}
+    )
+
+    assert result2["type"] is FlowResultType.ABORT
+    assert result2["reason"] == "reconfigure_successful"
+
+    assert mock_config_entry.data.get(CONF_HOST) == TEST_HOST