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
This commit is contained in:
Duco Sebel 2022-12-29 19:35:24 +01:00 committed by GitHub
parent 570824100c
commit d8cbff65f1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 40 additions and 29 deletions

View File

@ -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

View File

@ -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

View File

@ -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,
):