From d8cbff65f18433c205dc4e6a3bf8238ff1d9f2ca Mon Sep 17 00:00:00 2001 From: Duco Sebel <74970928+DCSBL@users.noreply.github.com> Date: Thu, 29 Dec 2022 19:35:24 +0100 Subject: [PATCH] Fix code quality issues for HomeWizard (#84134) * Remove unused constant * Reuse fetch check for retrieving device information * Remove else block * Patch integration setup in test * use isinstance to detect return type, instead of tuple * Raise exception when recoverable error has been triggered to make code cleaner * Use error code to split message and localization * Actually log things --- .../components/homewizard/config_flow.py | 60 +++++++++++-------- .../components/homewizard/coordinator.py | 6 +- .../components/homewizard/test_config_flow.py | 3 + 3 files changed, 40 insertions(+), 29 deletions(-) diff --git a/homeassistant/components/homewizard/config_flow.py b/homeassistant/components/homewizard/config_flow.py index bdd84c0d07e..60fa4b2451e 100644 --- a/homeassistant/components/homewizard/config_flow.py +++ b/homeassistant/components/homewizard/config_flow.py @@ -7,12 +7,14 @@ from typing import Any, cast from homewizard_energy import HomeWizardEnergy from homewizard_energy.errors import DisabledError, RequestError, UnsupportedError +from homewizard_energy.models import Device from voluptuous import Required, Schema from homeassistant import config_entries from homeassistant.components import persistent_notification, zeroconf from homeassistant.const import CONF_IP_ADDRESS from homeassistant.data_entry_flow import AbortFlow, FlowResult +from homeassistant.exceptions import HomeAssistantError from .const import ( CONF_API_ENABLED, @@ -73,19 +75,17 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): errors=None, ) - error = await self._async_try_connect(user_input[CONF_IP_ADDRESS]) - if error is not None: + # Fetch device information + try: + device_info = await self._async_try_connect(user_input[CONF_IP_ADDRESS]) + except RecoverableError as ex: + _LOGGER.error(ex) return self.async_show_form( step_id="user", data_schema=data_schema, - errors={"base": error}, + errors={"base": ex.error_code}, ) - # Fetch device information - api = HomeWizardEnergy(user_input[CONF_IP_ADDRESS]) - device_info = await api.device() - await api.close() - # Sets unique ID and aborts if it is already exists await self._async_set_and_check_unique_id( { @@ -153,12 +153,13 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Confirm discovery.""" if user_input is not None: - # Check connection - error = await self._async_try_connect(str(self.config[CONF_IP_ADDRESS])) - if error is not None: + try: + await self._async_try_connect(str(self.config[CONF_IP_ADDRESS])) + except RecoverableError as ex: + _LOGGER.error(ex) return self.async_show_form( step_id="discovery_confirm", - errors={"base": error}, + errors={"base": ex.error_code}, ) return self.async_create_entry( @@ -197,11 +198,13 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): if user_input is not None: assert self.entry is not None - error = await self._async_try_connect(self.entry.data[CONF_IP_ADDRESS]) - if error is not None: + try: + await self._async_try_connect(self.entry.data[CONF_IP_ADDRESS]) + except RecoverableError as ex: + _LOGGER.error(ex) return self.async_show_form( step_id="reauth_confirm", - errors={"base": error}, + errors={"base": ex.error_code}, ) await self.hass.config_entries.async_reload(self.entry.entry_id) @@ -212,7 +215,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): ) @staticmethod - async def _async_try_connect(ip_address: str) -> str | None: + async def _async_try_connect(ip_address: str) -> Device: """Try to connect.""" _LOGGER.debug("config_flow _async_try_connect") @@ -222,19 +225,21 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): energy_api = HomeWizardEnergy(ip_address) try: - await energy_api.device() + return await energy_api.device() - except DisabledError: - _LOGGER.error("API disabled, API must be enabled in the app") - return "api_not_enabled" + except DisabledError as ex: + raise RecoverableError( + "API disabled, API must be enabled in the app", "api_not_enabled" + ) from ex except UnsupportedError as ex: _LOGGER.error("API version unsuppored") raise AbortFlow("unsupported_api_version") from ex except RequestError as ex: - _LOGGER.exception(ex) - return "network_error" + raise RecoverableError( + "Device unreachable or unexpected response", "network_error" + ) from ex except Exception as ex: _LOGGER.exception(ex) @@ -243,8 +248,6 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): finally: await energy_api.close() - return None - async def _async_set_and_check_unique_id(self, entry_info: dict[str, Any]) -> None: """Validate if entry exists.""" @@ -256,3 +259,12 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): self._abort_if_unique_id_configured( updates={CONF_IP_ADDRESS: entry_info[CONF_IP_ADDRESS]} ) + + +class RecoverableError(HomeAssistantError): + """Raised when a connection has been failed but can be retried.""" + + def __init__(self, message: str, error_code: str) -> None: + """Init RecoverableError.""" + super().__init__(message) + self.error_code = error_code diff --git a/homeassistant/components/homewizard/coordinator.py b/homeassistant/components/homewizard/coordinator.py index d8c20a6cc92..df4d99e23ef 100644 --- a/homeassistant/components/homewizard/coordinator.py +++ b/homeassistant/components/homewizard/coordinator.py @@ -1,7 +1,6 @@ """Update coordinator for HomeWizard.""" from __future__ import annotations -from datetime import timedelta import logging from homewizard_energy import HomeWizardEnergy @@ -16,8 +15,6 @@ from .const import DOMAIN, UPDATE_INTERVAL, DeviceResponseEntry _LOGGER = logging.getLogger(__name__) -MAX_UPDATE_INTERVAL = timedelta(minutes=30) - class HWEnergyDeviceUpdateCoordinator(DataUpdateCoordinator[DeviceResponseEntry]): """Gather data for the energy device.""" @@ -73,7 +70,6 @@ class HWEnergyDeviceUpdateCoordinator(DataUpdateCoordinator[DeviceResponseEntry] raise UpdateFailed(ex) from ex - else: - self.api_disabled = False + self.api_disabled = False return data diff --git a/tests/components/homewizard/test_config_flow.py b/tests/components/homewizard/test_config_flow.py index 453a29c74f8..8170632e840 100644 --- a/tests/components/homewizard/test_config_flow.py +++ b/tests/components/homewizard/test_config_flow.py @@ -385,6 +385,9 @@ async def test_reauth_flow(hass, aioclient_mock): device = get_mock_device() with patch( + "homeassistant.components.homewizard.async_setup_entry", + return_value=True, + ), patch( "homeassistant.components.homewizard.config_flow.HomeWizardEnergy", return_value=device, ):