Address ruckus_unleashed late review (#99411)

CODEOWNERS

@@ -1064,8 +1064,8 @@ build.json @home-assistant/supervisor
 /tests/components/rss_feed_template/ @home-assistant/core
 /homeassistant/components/rtsp_to_webrtc/ @allenporter
 /tests/components/rtsp_to_webrtc/ @allenporter
-/homeassistant/components/ruckus_unleashed/ @gabe565 @lanrat
+/homeassistant/components/ruckus_unleashed/ @lanrat @ms264556 @gabe565
-/tests/components/ruckus_unleashed/ @gabe565 @lanrat
+/tests/components/ruckus_unleashed/ @lanrat @ms264556 @gabe565
 /homeassistant/components/ruuvi_gateway/ @akx
 /tests/components/ruuvi_gateway/ @akx
 /homeassistant/components/ruuvitag_ble/ @akx

homeassistant/components/ruckus_unleashed/__init__.py

@@ -2,7 +2,7 @@
 import logging
 
 from aioruckus import AjaxSession
-from aioruckus.exceptions import AuthenticationError
+from aioruckus.exceptions import AuthenticationError, SchemaError
 
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_USERNAME
@@ -31,16 +31,18 @@ _LOGGER = logging.getLogger(__package__)
 async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     """Set up Ruckus Unleashed from a config entry."""
 
-    try:
     ruckus = AjaxSession.async_create(
         entry.data[CONF_HOST],
         entry.data[CONF_USERNAME],
         entry.data[CONF_PASSWORD],
     )
+    try:
         await ruckus.login()
-    except (ConnectionRefusedError, ConnectionError) as conerr:
+    except (ConnectionError, SchemaError) as conerr:
+        await ruckus.close()
         raise ConfigEntryNotReady from conerr
     except AuthenticationError as autherr:
+        await ruckus.close()
         raise ConfigEntryAuthFailed from autherr
 
     coordinator = RuckusUnleashedDataUpdateCoordinator(hass, ruckus=ruckus)

homeassistant/components/ruckus_unleashed/config_flow.py

@@ -1,9 +1,10 @@
 """Config flow for Ruckus Unleashed integration."""
 from collections.abc import Mapping
+import logging
 from typing import Any
 
 from aioruckus import AjaxSession, SystemStat
-from aioruckus.exceptions import AuthenticationError
+from aioruckus.exceptions import AuthenticationError, SchemaError
 import voluptuous as vol
 
 from homeassistant import config_entries, core, exceptions
@@ -19,6 +20,8 @@ from .const import (
     KEY_SYS_TITLE,
 )
 
+_LOGGER = logging.getLogger(__package__)
+
 DATA_SCHEMA = vol.Schema(
     {
         vol.Required(CONF_HOST): str,
@@ -38,19 +41,20 @@ async def validate_input(hass: core.HomeAssistant, data):
         async with AjaxSession.async_create(
             data[CONF_HOST], data[CONF_USERNAME], data[CONF_PASSWORD]
         ) as ruckus:
-            system_info = await ruckus.api.get_system_info(
+            mesh_info = await ruckus.api.get_mesh_info()
-                SystemStat.SYSINFO,
+            system_info = await ruckus.api.get_system_info(SystemStat.SYSINFO)
-            )
+    except AuthenticationError as autherr:
-            mesh_name = (await ruckus.api.get_mesh_info())[API_MESH_NAME]
+        raise InvalidAuth from autherr
+    except (ConnectionError, SchemaError) as connerr:
+        raise CannotConnect from connerr
+
+    mesh_name = mesh_info[API_MESH_NAME]
     zd_serial = system_info[API_SYS_SYSINFO][API_SYS_SYSINFO_SERIAL]
+
     return {
         KEY_SYS_TITLE: mesh_name,
         KEY_SYS_SERIAL: zd_serial,
     }
-    except AuthenticationError as autherr:
-        raise InvalidAuth from autherr
-    except (ConnectionRefusedError, ConnectionError, KeyError) as connerr:
-        raise CannotConnect from connerr
 
 
 class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
@@ -58,6 +62,8 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
 
     VERSION = 1
 
+    _reauth_entry: config_entries.ConfigEntry | None = None
+
     async def async_step_user(
         self, user_input: dict[str, Any] | None = None
     ) -> FlowResult:
@@ -70,29 +76,39 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
                 errors["base"] = "cannot_connect"
             except InvalidAuth:
                 errors["base"] = "invalid_auth"
+            except Exception:  # pylint: disable=broad-except
+                _LOGGER.exception("Unexpected exception")
+                errors["base"] = "unknown"
             else:
+                if self._reauth_entry is None:
                     await self.async_set_unique_id(info[KEY_SYS_SERIAL])
                     self._abort_if_unique_id_configured()
                     return self.async_create_entry(
                         title=info[KEY_SYS_TITLE], data=user_input
                     )
+                if info[KEY_SYS_SERIAL] == self._reauth_entry.unique_id:
+                    self.hass.config_entries.async_update_entry(
+                        self._reauth_entry, data=user_input
+                    )
+                    self.hass.async_create_task(
+                        self.hass.config_entries.async_reload(
+                            self._reauth_entry.entry_id
+                        )
+                    )
+                    return self.async_abort(reason="reauth_successful")
+                errors["base"] = "invalid_host"
 
+        data_schema = self.add_suggested_values_to_schema(
+            DATA_SCHEMA, self._reauth_entry.data if self._reauth_entry else {}
+        )
         return self.async_show_form(
-            step_id="user", data_schema=DATA_SCHEMA, errors=errors
+            step_id="user", data_schema=data_schema, errors=errors
         )
 
     async def async_step_reauth(self, entry_data: Mapping[str, Any]) -> FlowResult:
         """Perform reauth upon an API authentication error."""
-        return await self.async_step_reauth_confirm()
+        self._reauth_entry = self.hass.config_entries.async_get_entry(
-
+            self.context["entry_id"]
-    async def async_step_reauth_confirm(
-        self, user_input: dict[str, Any] | None = None
-    ) -> FlowResult:
-        """Dialog that informs the user that reauth is required."""
-        if user_input is None:
-            return self.async_show_form(
-                step_id="reauth_confirm",
-                data_schema=DATA_SCHEMA,
         )
         return await self.async_step_user()
 

homeassistant/components/ruckus_unleashed/coordinator.py

@@ -3,9 +3,10 @@ from datetime import timedelta
 import logging
 
 from aioruckus import AjaxSession
-from aioruckus.exceptions import AuthenticationError
+from aioruckus.exceptions import AuthenticationError, SchemaError
 
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import ConfigEntryAuthFailed
 from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
 
 from .const import API_CLIENT_MAC, DOMAIN, KEY_SYS_CLIENTS, SCAN_INTERVAL
@@ -40,6 +41,6 @@ class RuckusUnleashedDataUpdateCoordinator(DataUpdateCoordinator):
         try:
             return {KEY_SYS_CLIENTS: await self._fetch_clients()}
         except AuthenticationError as autherror:
-            raise UpdateFailed(autherror) from autherror
+            raise ConfigEntryAuthFailed(autherror) from autherror
-        except (ConnectionRefusedError, ConnectionError) as conerr:
+        except (ConnectionError, SchemaError) as conerr:
             raise UpdateFailed(conerr) from conerr

homeassistant/components/ruckus_unleashed/device_tracker.py

@@ -103,20 +103,16 @@ class RuckusUnleashedDevice(CoordinatorEntity, ScannerEntity):
     @property
     def name(self) -> str:
         """Return the name."""
-        return (
+        if not self.is_connected:
-            self._name
+            return self._name
-            if not self.is_connected
+        return self.coordinator.data[KEY_SYS_CLIENTS][self._mac][API_CLIENT_HOSTNAME]
-            else self.coordinator.data[KEY_SYS_CLIENTS][self._mac][API_CLIENT_HOSTNAME]
-        )
 
     @property
-    def ip_address(self) -> str:
+    def ip_address(self) -> str | None:
         """Return the ip address."""
-        return (
+        if not self.is_connected:
-            self.coordinator.data[KEY_SYS_CLIENTS][self._mac][API_CLIENT_IP]
+            return None
-            if self.is_connected
+        return self.coordinator.data[KEY_SYS_CLIENTS][self._mac][API_CLIENT_IP]
-            else None
-        )
 
     @property
     def is_connected(self) -> bool:

homeassistant/components/ruckus_unleashed/manifest.json

@@ -1,11 +1,11 @@
 {
   "domain": "ruckus_unleashed",
   "name": "Ruckus Unleashed",
-  "codeowners": ["@gabe565", "@lanrat"],
+  "codeowners": ["@lanrat", "@ms264556", "@gabe565"],
   "config_flow": true,
   "documentation": "https://www.home-assistant.io/integrations/ruckus_unleashed",
   "integration_type": "hub",
   "iot_class": "local_polling",
   "loggers": ["aioruckus", "xmltodict"],
-  "requirements": ["aioruckus==0.31", "xmltodict==0.13.0"]
+  "requirements": ["aioruckus==0.34"]
 }

homeassistant/components/ruckus_unleashed/strings.json

@@ -12,10 +12,12 @@
     "error": {
       "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]",
       "invalid_auth": "[%key:common::config_flow::error::invalid_auth%]",
+      "invalid_host": "[%key:common::config_flow::error::invalid_host%]",
       "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%]"
     }
   }
 }

requirements_all.txt

@@ -334,7 +334,7 @@ aiorecollect==2023.09.0
 aioridwell==2023.07.0
 
 # homeassistant.components.ruckus_unleashed
-aioruckus==0.31
+aioruckus==0.34
 
 # homeassistant.components.ruuvi_gateway
 aioruuvigateway==0.1.0
@@ -2723,7 +2723,6 @@ xknxproject==3.2.0
 # homeassistant.components.bluesound
 # homeassistant.components.fritz
 # homeassistant.components.rest
-# homeassistant.components.ruckus_unleashed
 # homeassistant.components.startca
 # homeassistant.components.ted5000
 # homeassistant.components.zestimate

requirements_test_all.txt

@@ -309,7 +309,7 @@ aiorecollect==2023.09.0
 aioridwell==2023.07.0
 
 # homeassistant.components.ruckus_unleashed
-aioruckus==0.31
+aioruckus==0.34
 
 # homeassistant.components.ruuvi_gateway
 aioruuvigateway==0.1.0
@@ -2011,7 +2011,6 @@ xknxproject==3.2.0
 # homeassistant.components.bluesound
 # homeassistant.components.fritz
 # homeassistant.components.rest
-# homeassistant.components.ruckus_unleashed
 # homeassistant.components.startca
 # homeassistant.components.ted5000
 # homeassistant.components.zestimate

tests/components/ruckus_unleashed/test_config_flow.py

@@ -1,4 +1,5 @@
 """Test the Ruckus Unleashed config flow."""
+from copy import deepcopy
 from datetime import timedelta
 from unittest.mock import AsyncMock, patch
 
@@ -10,12 +11,22 @@ from aioruckus.const import (
 from aioruckus.exceptions import AuthenticationError
 
 from homeassistant import config_entries, data_entry_flow
-from homeassistant.components.ruckus_unleashed.const import DOMAIN
+from homeassistant.components.ruckus_unleashed.const import (
+    API_SYS_SYSINFO,
+    API_SYS_SYSINFO_SERIAL,
+    DOMAIN,
+)
 from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_USERNAME
 from homeassistant.core import HomeAssistant
 from homeassistant.util import utcnow
 
-from . import CONFIG, DEFAULT_TITLE, RuckusAjaxApiPatchContext, mock_config_entry
+from . import (
+    CONFIG,
+    DEFAULT_SYSTEM_INFO,
+    DEFAULT_TITLE,
+    RuckusAjaxApiPatchContext,
+    mock_config_entry,
+)
 
 from tests.common import async_fire_time_changed
 
@@ -25,7 +36,7 @@ async def test_form(hass: HomeAssistant) -> None:
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": config_entries.SOURCE_USER}
     )
-    assert result["type"] == "form"
+    assert result["type"] == data_entry_flow.FlowResultType.FORM
     assert result["errors"] == {}
 
     with RuckusAjaxApiPatchContext(), patch(
@@ -37,11 +48,11 @@ async def test_form(hass: HomeAssistant) -> None:
             CONFIG,
         )
         await hass.async_block_till_done()
+        assert len(mock_setup_entry.mock_calls) == 1
 
-        assert result2["type"] == "create_entry"
+    assert result2["type"] == data_entry_flow.FlowResultType.CREATE_ENTRY
     assert result2["title"] == DEFAULT_TITLE
     assert result2["data"] == CONFIG
-        assert len(mock_setup_entry.mock_calls) == 1
 
 
 async def test_form_invalid_auth(hass: HomeAssistant) -> None:
@@ -58,7 +69,7 @@ async def test_form_invalid_auth(hass: HomeAssistant) -> None:
             CONFIG,
         )
 
-    assert result2["type"] == "form"
+    assert result2["type"] == data_entry_flow.FlowResultType.FORM
     assert result2["errors"] == {"base": "invalid_auth"}
 
 
@@ -68,7 +79,13 @@ async def test_form_user_reauth(hass: HomeAssistant) -> None:
     entry.add_to_hass(hass)
 
     result = await hass.config_entries.flow.async_init(
-        DOMAIN, context={"source": config_entries.SOURCE_REAUTH}
+        DOMAIN,
+        context={
+            "source": config_entries.SOURCE_REAUTH,
+            "entry_id": entry.entry_id,
+            "unique_id": entry.unique_id,
+        },
+        data=entry.data,
     )
 
     flows = hass.config_entries.flow.async_progress()
@@ -76,20 +93,181 @@ async def test_form_user_reauth(hass: HomeAssistant) -> None:
     assert "flow_id" in flows[0]
 
     assert result["type"] == data_entry_flow.FlowResultType.FORM
-    assert result["step_id"] == "reauth_confirm"
+    assert result["step_id"] == "user"
+    assert result["errors"] == {}
 
+    with RuckusAjaxApiPatchContext():
         result2 = await hass.config_entries.flow.async_configure(
-        flows[0]["flow_id"],
+            result["flow_id"],
             user_input={
                 CONF_HOST: "1.2.3.4",
                 CONF_USERNAME: "new_name",
                 CONF_PASSWORD: "new_pass",
             },
         )
-
         await hass.async_block_till_done()
+
+    assert result2["type"] == data_entry_flow.FlowResultType.ABORT
+    assert result2["reason"] == "reauth_successful"
+
+
+async def test_form_user_reauth_different_unique_id(hass: HomeAssistant) -> None:
+    """Test reauth."""
+    entry = mock_config_entry()
+    entry.add_to_hass(hass)
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN,
+        context={
+            "source": config_entries.SOURCE_REAUTH,
+            "entry_id": entry.entry_id,
+            "unique_id": entry.unique_id,
+        },
+        data=entry.data,
+    )
+
+    flows = hass.config_entries.flow.async_progress()
+    assert len(flows) == 1
+    assert "flow_id" in flows[0]
+
+    assert result["type"] == data_entry_flow.FlowResultType.FORM
+    assert result["step_id"] == "user"
+    assert result["errors"] == {}
+
+    system_info = deepcopy(DEFAULT_SYSTEM_INFO)
+    system_info[API_SYS_SYSINFO][API_SYS_SYSINFO_SERIAL] = "000000000"
+    with RuckusAjaxApiPatchContext(system_info=system_info):
+        result2 = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            user_input={
+                CONF_HOST: "1.2.3.4",
+                CONF_USERNAME: "new_name",
+                CONF_PASSWORD: "new_pass",
+            },
+        )
+        await hass.async_block_till_done()
+
+    assert result2["type"] == data_entry_flow.FlowResultType.FORM
+    assert result2["errors"] == {"base": "invalid_host"}
+
+
+async def test_form_user_reauth_invalid_auth(hass: HomeAssistant) -> None:
+    """Test reauth."""
+    entry = mock_config_entry()
+    entry.add_to_hass(hass)
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN,
+        context={
+            "source": config_entries.SOURCE_REAUTH,
+            "entry_id": entry.entry_id,
+            "unique_id": entry.unique_id,
+        },
+        data=entry.data,
+    )
+
+    flows = hass.config_entries.flow.async_progress()
+    assert len(flows) == 1
+    assert "flow_id" in flows[0]
+
+    assert result["type"] == data_entry_flow.FlowResultType.FORM
+    assert result["step_id"] == "user"
+    assert result["errors"] == {}
+
+    with RuckusAjaxApiPatchContext(
+        login_mock=AsyncMock(side_effect=AuthenticationError(ERROR_LOGIN_INCORRECT))
+    ):
+        result2 = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            user_input={
+                CONF_HOST: "1.2.3.4",
+                CONF_USERNAME: "new_name",
+                CONF_PASSWORD: "new_pass",
+            },
+        )
+        await hass.async_block_till_done()
+
+    assert result2["type"] == data_entry_flow.FlowResultType.FORM
+    assert result2["errors"] == {"base": "invalid_auth"}
+
+
+async def test_form_user_reauth_cannot_connect(hass: HomeAssistant) -> None:
+    """Test reauth."""
+    entry = mock_config_entry()
+    entry.add_to_hass(hass)
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN,
+        context={
+            "source": config_entries.SOURCE_REAUTH,
+            "entry_id": entry.entry_id,
+            "unique_id": entry.unique_id,
+        },
+        data=entry.data,
+    )
+
+    flows = hass.config_entries.flow.async_progress()
+    assert len(flows) == 1
+    assert "flow_id" in flows[0]
+
+    assert result["type"] == data_entry_flow.FlowResultType.FORM
+    assert result["step_id"] == "user"
+    assert result["errors"] == {}
+
+    with RuckusAjaxApiPatchContext(
+        login_mock=AsyncMock(side_effect=ConnectionError(ERROR_CONNECT_TIMEOUT))
+    ):
+        result2 = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            user_input={
+                CONF_HOST: "1.2.3.4",
+                CONF_USERNAME: "new_name",
+                CONF_PASSWORD: "new_pass",
+            },
+        )
+        await hass.async_block_till_done()
+
+    assert result2["type"] == data_entry_flow.FlowResultType.FORM
+    assert result2["errors"] == {"base": "cannot_connect"}
+
+
+async def test_form_user_reauth_general_exception(hass: HomeAssistant) -> None:
+    """Test reauth."""
+    entry = mock_config_entry()
+    entry.add_to_hass(hass)
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN,
+        context={
+            "source": config_entries.SOURCE_REAUTH,
+            "entry_id": entry.entry_id,
+            "unique_id": entry.unique_id,
+        },
+        data=entry.data,
+    )
+
+    flows = hass.config_entries.flow.async_progress()
+    assert len(flows) == 1
+    assert "flow_id" in flows[0]
+
+    assert result["type"] == data_entry_flow.FlowResultType.FORM
+    assert result["step_id"] == "user"
+    assert result["errors"] == {}
+
+    with RuckusAjaxApiPatchContext(login_mock=AsyncMock(side_effect=Exception)):
+        result2 = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            user_input={
+                CONF_HOST: "1.2.3.4",
+                CONF_USERNAME: "new_name",
+                CONF_PASSWORD: "new_pass",
+            },
+        )
+        await hass.async_block_till_done()
+
     assert result2["type"] == data_entry_flow.FlowResultType.FORM
     assert result2["step_id"] == "user"
+    assert result2["errors"] == {"base": "unknown"}
 
 
 async def test_form_cannot_connect(hass: HomeAssistant) -> None:
@@ -106,10 +284,27 @@ async def test_form_cannot_connect(hass: HomeAssistant) -> None:
             CONFIG,
         )
 
-    assert result2["type"] == "form"
+    assert result2["type"] == data_entry_flow.FlowResultType.FORM
     assert result2["errors"] == {"base": "cannot_connect"}
 
 
+async def test_form_general_exception(hass: HomeAssistant) -> None:
+    """Test we handle cannot connect error."""
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN, context={"source": config_entries.SOURCE_USER}
+    )
+
+    with RuckusAjaxApiPatchContext(login_mock=AsyncMock(side_effect=Exception)):
+        result2 = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            CONFIG,
+        )
+
+    assert result2["type"] == data_entry_flow.FlowResultType.FORM
+    assert result2["step_id"] == "user"
+    assert result2["errors"] == {"base": "unknown"}
+
+
 async def test_form_unexpected_response(hass: HomeAssistant) -> None:
     """Test we handle unknown error."""
     result = await hass.config_entries.flow.async_init(
@@ -126,25 +321,7 @@ async def test_form_unexpected_response(hass: HomeAssistant) -> None:
             CONFIG,
         )
 
-    assert result2["type"] == "form"
+    assert result2["type"] == data_entry_flow.FlowResultType.FORM
-    assert result2["errors"] == {"base": "cannot_connect"}
-
-
-async def test_form_cannot_connect_unknown_serial(hass: HomeAssistant) -> None:
-    """Test we handle cannot connect error on invalid serial number."""
-    result = await hass.config_entries.flow.async_init(
-        DOMAIN, context={"source": config_entries.SOURCE_USER}
-    )
-    assert result["type"] == "form"
-    assert result["errors"] == {}
-
-    with RuckusAjaxApiPatchContext(system_info={}):
-        result2 = await hass.config_entries.flow.async_configure(
-            result["flow_id"],
-            CONFIG,
-        )
-
-    assert result2["type"] == "form"
     assert result2["errors"] == {"base": "cannot_connect"}
 
 
@@ -167,7 +344,7 @@ async def test_form_duplicate_error(hass: HomeAssistant) -> None:
         result = await hass.config_entries.flow.async_init(
             DOMAIN, context={"source": config_entries.SOURCE_USER}
         )
-        assert result["type"] == "form"
+        assert result["type"] == data_entry_flow.FlowResultType.FORM
         assert result["errors"] == {}
 
         result2 = await hass.config_entries.flow.async_configure(
@@ -175,5 +352,5 @@ async def test_form_duplicate_error(hass: HomeAssistant) -> None:
             CONFIG,
         )
 
-    assert result2["type"] == "abort"
+    assert result2["type"] == data_entry_flow.FlowResultType.ABORT
     assert result2["reason"] == "already_configured"