Improve HomeWizard request issue reporting (#82366)

* Trigger reauth flow when HomeWizard API was disabled

* Add tests for reauth flow

* Fix typo in test

* Add parallel updates constant

* Improve error message when device in unreachable during config

* Set quality scale

* Remove quality scale

* Throw error instead of abort when setup fails

* Adjust test for new setup behaviour

* Trigger reauth flow when API is disabled and continue retrying

* Reload entry and raise AuthFailed during init

* Abort running config flow

* Listen for coordinator updates to trigger reload

* Use build-in backoff system

* Fix failing test

* Test reauth flow is active after disable-api init

* Test reauth flow removal
pull/84203/head
Duco Sebel 2022-12-16 16:53:54 +01:00 committed by GitHub
parent f5a8ce4aca
commit b41d0be952
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 243 additions and 66 deletions

View File

@ -1,7 +1,7 @@
"""The Homewizard integration.""" """The Homewizard integration."""
import logging import logging
from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_REAUTH, ConfigEntry
from homeassistant.const import CONF_IP_ADDRESS from homeassistant.const import CONF_IP_ADDRESS
from homeassistant.core import HomeAssistant from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.exceptions import ConfigEntryNotReady
@ -64,13 +64,28 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
hass.async_create_task(hass.config_entries.async_remove(old_config_entry_id)) hass.async_create_task(hass.config_entries.async_remove(old_config_entry_id))
# Create coordinator # Create coordinator
coordinator = Coordinator(hass, entry.data[CONF_IP_ADDRESS]) coordinator = Coordinator(hass, entry.entry_id, entry.data[CONF_IP_ADDRESS])
try: try:
await coordinator.async_config_entry_first_refresh() await coordinator.async_config_entry_first_refresh()
except ConfigEntryNotReady: except ConfigEntryNotReady:
await coordinator.api.close() await coordinator.api.close()
if coordinator.api_disabled:
entry.async_start_reauth(hass)
raise raise
# Abort reauth config flow if active
for progress_flow in hass.config_entries.flow.async_progress_by_handler(DOMAIN):
if progress_flow["context"].get("source") == SOURCE_REAUTH:
hass.config_entries.flow.async_abort(progress_flow["flow_id"])
# Setup entry
hass.data.setdefault(DOMAIN, {})
hass.data[DOMAIN][entry.entry_id] = coordinator
# Register device # Register device
device_registry = dr.async_get(hass) device_registry = dr.async_get(hass)
device_registry.async_get_or_create( device_registry.async_get_or_create(
@ -83,9 +98,6 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
) )
# Finalize # Finalize
hass.data.setdefault(DOMAIN, {})
hass.data[DOMAIN][entry.entry_id] = coordinator
await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
return True return True
@ -98,7 +110,7 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS) unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
if unload_ok: if unload_ok:
config_data = hass.data[DOMAIN].pop(entry.entry_id) coordinator = hass.data[DOMAIN].pop(entry.entry_id)
await config_data.api.close() await coordinator.api.close()
return unload_ok return unload_ok

View File

@ -1,6 +1,7 @@
"""Config flow for Homewizard.""" """Config flow for Homewizard."""
from __future__ import annotations from __future__ import annotations
from collections.abc import Mapping
import logging import logging
from typing import Any, cast from typing import Any, cast
@ -33,6 +34,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
def __init__(self) -> None: def __init__(self) -> None:
"""Initialize the HomeWizard config flow.""" """Initialize the HomeWizard config flow."""
self.config: dict[str, str | int] = {} self.config: dict[str, str | int] = {}
self.entry: config_entries.ConfigEntry | None = None
async def async_step_import(self, import_config: dict[str, Any]) -> FlowResult: async def async_step_import(self, import_config: dict[str, Any]) -> FlowResult:
"""Handle a flow initiated by older `homewizard_energy` component.""" """Handle a flow initiated by older `homewizard_energy` component."""
@ -57,27 +59,38 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
_LOGGER.debug("config_flow async_step_user") _LOGGER.debug("config_flow async_step_user")
data_schema = Schema(
{
Required(CONF_IP_ADDRESS): str,
}
)
if user_input is None: if user_input is None:
return self.async_show_form( return self.async_show_form(
step_id="user", step_id="user",
data_schema=Schema( data_schema=data_schema,
{
Required(CONF_IP_ADDRESS): str,
}
),
errors=None, errors=None,
) )
device_info = await self._async_try_connect_and_fetch( error = await self._async_try_connect(user_input[CONF_IP_ADDRESS])
user_input[CONF_IP_ADDRESS] if error is not None:
) return self.async_show_form(
step_id="user",
data_schema=data_schema,
errors={"base": error},
)
# 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 # Sets unique ID and aborts if it is already exists
await self._async_set_and_check_unique_id( await self._async_set_and_check_unique_id(
{ {
CONF_IP_ADDRESS: user_input[CONF_IP_ADDRESS], CONF_IP_ADDRESS: user_input[CONF_IP_ADDRESS],
CONF_PRODUCT_TYPE: device_info[CONF_PRODUCT_TYPE], CONF_PRODUCT_TYPE: device_info.product_type,
CONF_SERIAL: device_info[CONF_SERIAL], CONF_SERIAL: device_info.serial,
} }
) )
@ -90,7 +103,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
# Add entry # Add entry
return self.async_create_entry( return self.async_create_entry(
title=f"{device_info[CONF_PRODUCT_NAME]} ({device_info[CONF_SERIAL]})", title=f"{device_info.product_name} ({device_info.serial})",
data=data, data=data,
) )
@ -138,11 +151,14 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
) -> FlowResult: ) -> FlowResult:
"""Confirm discovery.""" """Confirm discovery."""
if user_input is not None: if user_input is not None:
if (self.config[CONF_API_ENABLED]) != "1":
raise AbortFlow(reason="api_not_enabled")
# Check connection # Check connection
await self._async_try_connect_and_fetch(str(self.config[CONF_IP_ADDRESS])) error = await self._async_try_connect(str(self.config[CONF_IP_ADDRESS]))
if error is not None:
return self.async_show_form(
step_id="discovery_confirm",
errors={"base": error},
)
return self.async_create_entry( return self.async_create_entry(
title=f"{self.config[CONF_PRODUCT_NAME]} ({self.config[CONF_SERIAL]})", title=f"{self.config[CONF_PRODUCT_NAME]} ({self.config[CONF_SERIAL]})",
@ -166,46 +182,67 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
}, },
) )
async def async_step_reauth(self, entry_data: Mapping[str, Any]) -> FlowResult:
"""Handle re-auth if API was disabled."""
self.entry = self.hass.config_entries.async_get_entry(self.context["entry_id"])
return await self.async_step_reauth_confirm()
async def async_step_reauth_confirm(
self, user_input: dict[str, Any] | None = None
) -> FlowResult:
"""Confirm reauth dialog."""
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:
return self.async_show_form(
step_id="reauth_confirm",
errors={"base": error},
)
await self.hass.config_entries.async_reload(self.entry.entry_id)
return self.async_abort(reason="reauth_successful")
return self.async_show_form(
step_id="reauth_confirm",
)
@staticmethod @staticmethod
async def _async_try_connect_and_fetch(ip_address: str) -> dict[str, Any]: async def _async_try_connect(ip_address: str) -> str | None:
"""Try to connect.""" """Try to connect."""
_LOGGER.debug("config_flow _async_try_connect_and_fetch") _LOGGER.debug("config_flow _async_try_connect")
# Make connection with device # Make connection with device
# This is to test the connection and to get info for unique_id # This is to test the connection and to get info for unique_id
energy_api = HomeWizardEnergy(ip_address) energy_api = HomeWizardEnergy(ip_address)
try: try:
device = await energy_api.device() await energy_api.device()
except DisabledError as ex: except DisabledError:
_LOGGER.error("API disabled, API must be enabled in the app") _LOGGER.error("API disabled, API must be enabled in the app")
raise AbortFlow("api_not_enabled") from ex return "api_not_enabled"
except UnsupportedError as ex: except UnsupportedError as ex:
_LOGGER.error("API version unsuppored") _LOGGER.error("API version unsuppored")
raise AbortFlow("unsupported_api_version") from ex raise AbortFlow("unsupported_api_version") from ex
except RequestError as ex: except RequestError as ex:
_LOGGER.error("Unexpected or no response") _LOGGER.exception(ex)
raise AbortFlow("unknown_error") from ex return "network_error"
except Exception as ex: except Exception as ex:
_LOGGER.exception( _LOGGER.exception(ex)
"Error connecting with Energy Device at %s",
ip_address,
)
raise AbortFlow("unknown_error") from ex raise AbortFlow("unknown_error") from ex
finally: finally:
await energy_api.close() await energy_api.close()
return { return None
CONF_PRODUCT_NAME: device.product_name,
CONF_PRODUCT_TYPE: device.product_type,
CONF_SERIAL: device.serial,
}
async def _async_set_and_check_unique_id(self, entry_info: dict[str, Any]) -> None: async def _async_set_and_check_unique_id(self, entry_info: dict[str, Any]) -> None:
"""Validate if entry exists.""" """Validate if entry exists."""

View File

@ -1,6 +1,7 @@
"""Update coordinator for HomeWizard.""" """Update coordinator for HomeWizard."""
from __future__ import annotations from __future__ import annotations
from datetime import timedelta
import logging import logging
from homewizard_energy import HomeWizardEnergy from homewizard_energy import HomeWizardEnergy
@ -15,20 +16,25 @@ from .const import DOMAIN, UPDATE_INTERVAL, DeviceResponseEntry
_LOGGER = logging.getLogger(__name__) _LOGGER = logging.getLogger(__name__)
MAX_UPDATE_INTERVAL = timedelta(minutes=30)
class HWEnergyDeviceUpdateCoordinator(DataUpdateCoordinator[DeviceResponseEntry]): class HWEnergyDeviceUpdateCoordinator(DataUpdateCoordinator[DeviceResponseEntry]):
"""Gather data for the energy device.""" """Gather data for the energy device."""
api: HomeWizardEnergy api: HomeWizardEnergy
api_disabled: bool = False
def __init__( def __init__(
self, self,
hass: HomeAssistant, hass: HomeAssistant,
entry_id: str,
host: str, host: str,
) -> None: ) -> None:
"""Initialize Update Coordinator.""" """Initialize Update Coordinator."""
super().__init__(hass, _LOGGER, name=DOMAIN, update_interval=UPDATE_INTERVAL) super().__init__(hass, _LOGGER, name=DOMAIN, update_interval=UPDATE_INTERVAL)
self.entry_id = entry_id
self.api = HomeWizardEnergy(host, clientsession=async_get_clientsession(hass)) self.api = HomeWizardEnergy(host, clientsession=async_get_clientsession(hass))
@property @property
@ -55,9 +61,19 @@ class HWEnergyDeviceUpdateCoordinator(DataUpdateCoordinator[DeviceResponseEntry]
data["system"] = await self.api.system() data["system"] = await self.api.system()
except RequestError as ex: except RequestError as ex:
raise UpdateFailed("Device did not respond as expected") from ex raise UpdateFailed(ex) from ex
except DisabledError as ex: except DisabledError as ex:
raise UpdateFailed("API disabled, API must be enabled in the app") from ex if not self.api_disabled:
self.api_disabled = True
# Do not reload when performing first refresh
if self.data is not None:
await self.hass.config_entries.async_reload(self.entry_id)
raise UpdateFailed(ex) from ex
else:
self.api_disabled = False
return data return data

View File

@ -1,7 +1,6 @@
"""Creates Homewizard sensor entities.""" """Creates Homewizard sensor entities."""
from __future__ import annotations from __future__ import annotations
import logging
from typing import Final, cast from typing import Final, cast
from homeassistant.components.sensor import ( from homeassistant.components.sensor import (
@ -26,7 +25,7 @@ from homeassistant.helpers.update_coordinator import CoordinatorEntity
from .const import DOMAIN, DeviceResponseEntry from .const import DOMAIN, DeviceResponseEntry
from .coordinator import HWEnergyDeviceUpdateCoordinator from .coordinator import HWEnergyDeviceUpdateCoordinator
_LOGGER = logging.getLogger(__name__) PARALLEL_UPDATES = 1
SENSORS: Final[tuple[SensorEntityDescription, ...]] = ( SENSORS: Final[tuple[SensorEntityDescription, ...]] = (
SensorEntityDescription( SensorEntityDescription(

View File

@ -11,14 +11,21 @@
"discovery_confirm": { "discovery_confirm": {
"title": "Confirm", "title": "Confirm",
"description": "Do you want to set up {product_type} ({serial}) at {ip_address}?" "description": "Do you want to set up {product_type} ({serial}) at {ip_address}?"
},
"reauth_confirm": {
"description": "The local API is disabled. Go to the HomeWizard Energy app and enable the API in the device settings."
} }
}, },
"error": {
"api_not_enabled": "The API is not enabled. Enable API in the HomeWizard Energy App under settings",
"network_error": "Device unreachable, make sure that you have entered the correct IP address and that the device is available in your network"
},
"abort": { "abort": {
"already_configured": "[%key:common::config_flow::abort::already_configured_device%]", "already_configured": "[%key:common::config_flow::abort::already_configured_device%]",
"invalid_discovery_parameters": "Detected unsupported API version", "invalid_discovery_parameters": "Detected unsupported API version",
"api_not_enabled": "The API is not enabled. Enable API in the HomeWizard Energy App under settings",
"device_not_supported": "This device is not supported", "device_not_supported": "This device is not supported",
"unknown_error": "[%key:common::config_flow::error::unknown%]" "unknown_error": "[%key:common::config_flow::error::unknown%]",
"reauth_successful": "Enabeling API was successful"
} }
} }
} }

View File

@ -46,8 +46,7 @@ async def test_manual_flow_works(hass, aioclient_mock):
assert len(hass.config_entries.async_entries(DOMAIN)) == 1 assert len(hass.config_entries.async_entries(DOMAIN)) == 1
assert len(device.device.mock_calls) == 1 assert len(device.close.mock_calls) == len(device.device.mock_calls)
assert len(device.close.mock_calls) == 1
assert len(mock_setup_entry.mock_calls) == 1 assert len(mock_setup_entry.mock_calls) == 1
@ -142,8 +141,7 @@ async def test_config_flow_imports_entry(aioclient_mock, hass):
assert result["data"][CONF_IP_ADDRESS] == "1.2.3.4" assert result["data"][CONF_IP_ADDRESS] == "1.2.3.4"
assert len(hass.config_entries.async_entries(DOMAIN)) == 1 assert len(hass.config_entries.async_entries(DOMAIN)) == 1
assert len(device.device.mock_calls) == 1 assert len(device.device.mock_calls) == len(device.close.mock_calls)
assert len(device.close.mock_calls) == 1
assert len(mock_setup_entry.mock_calls) == 1 assert len(mock_setup_entry.mock_calls) == 1
@ -174,19 +172,25 @@ async def test_discovery_disabled_api(hass, aioclient_mock):
assert result["type"] == FlowResultType.FORM assert result["type"] == FlowResultType.FORM
def mock_initialize():
raise DisabledError
device = get_mock_device()
device.device.side_effect = mock_initialize
with patch( with patch(
"homeassistant.components.homewizard.async_setup_entry", "homeassistant.components.homewizard.async_setup_entry",
return_value=True, return_value=True,
), patch( ), patch(
"homeassistant.components.homewizard.config_flow.HomeWizardEnergy", "homeassistant.components.homewizard.config_flow.HomeWizardEnergy",
return_value=get_mock_device(), return_value=device,
): ):
result = await hass.config_entries.flow.async_configure( result = await hass.config_entries.flow.async_configure(
result["flow_id"], user_input={"ip_address": "192.168.43.183"} result["flow_id"], user_input={"ip_address": "192.168.43.183"}
) )
assert result["type"] == FlowResultType.ABORT assert result["type"] == FlowResultType.FORM
assert result["reason"] == "api_not_enabled" assert result["errors"] == {"base": "api_not_enabled"}
async def test_discovery_missing_data_in_service_info(hass, aioclient_mock): async def test_discovery_missing_data_in_service_info(hass, aioclient_mock):
@ -271,8 +275,8 @@ async def test_check_disabled_api(hass, aioclient_mock):
result["flow_id"], {CONF_IP_ADDRESS: "2.2.2.2"} result["flow_id"], {CONF_IP_ADDRESS: "2.2.2.2"}
) )
assert result["type"] == FlowResultType.ABORT assert result["type"] == FlowResultType.FORM
assert result["reason"] == "api_not_enabled" assert result["errors"] == {"base": "api_not_enabled"}
async def test_check_error_handling_api(hass, aioclient_mock): async def test_check_error_handling_api(hass, aioclient_mock):
@ -355,5 +359,71 @@ async def test_check_requesterror(hass, aioclient_mock):
result["flow_id"], {CONF_IP_ADDRESS: "2.2.2.2"} result["flow_id"], {CONF_IP_ADDRESS: "2.2.2.2"}
) )
assert result["type"] == FlowResultType.ABORT assert result["type"] == FlowResultType.FORM
assert result["reason"] == "unknown_error" assert result["errors"] == {"base": "network_error"}
async def test_reauth_flow(hass, aioclient_mock):
"""Test reauth flow while API is enabled."""
mock_entry = MockConfigEntry(
domain="homewizard_energy", data={CONF_IP_ADDRESS: "1.2.3.4"}
)
mock_entry.add_to_hass(hass)
result = await hass.config_entries.flow.async_init(
DOMAIN,
context={
"source": config_entries.SOURCE_REAUTH,
"entry_id": mock_entry.entry_id,
},
)
assert result["type"] == "form"
assert result["step_id"] == "reauth_confirm"
device = get_mock_device()
with patch(
"homeassistant.components.homewizard.config_flow.HomeWizardEnergy",
return_value=device,
):
result = await hass.config_entries.flow.async_configure(result["flow_id"], {})
assert result["type"] == FlowResultType.ABORT
assert result["reason"] == "reauth_successful"
async def test_reauth_error(hass, aioclient_mock):
"""Test reauth flow while API is still disabled."""
def mock_initialize():
raise DisabledError()
mock_entry = MockConfigEntry(
domain="homewizard_energy", data={CONF_IP_ADDRESS: "1.2.3.4"}
)
mock_entry.add_to_hass(hass)
result = await hass.config_entries.flow.async_init(
DOMAIN,
context={
"source": config_entries.SOURCE_REAUTH,
"entry_id": mock_entry.entry_id,
},
)
assert result["type"] == "form"
assert result["step_id"] == "reauth_confirm"
device = get_mock_device()
device.device.side_effect = mock_initialize
with patch(
"homeassistant.components.homewizard.config_flow.HomeWizardEnergy",
return_value=device,
):
result = await hass.config_entries.flow.async_configure(result["flow_id"], {})
assert result["type"] == FlowResultType.FORM
assert result["errors"] == {"base": "api_not_enabled"}

View File

@ -6,7 +6,7 @@ from homewizard_energy.errors import DisabledError, HomeWizardEnergyException
from homeassistant import config_entries from homeassistant import config_entries
from homeassistant.components.homewizard.const import DOMAIN from homeassistant.components.homewizard.const import DOMAIN
from homeassistant.config_entries import ConfigEntryState from homeassistant.config_entries import SOURCE_REAUTH, ConfigEntryState
from homeassistant.const import CONF_IP_ADDRESS from homeassistant.const import CONF_IP_ADDRESS
from homeassistant.helpers import entity_registry as er from homeassistant.helpers import entity_registry as er
@ -187,6 +187,52 @@ async def test_load_detect_api_disabled(aioclient_mock, hass):
assert entry.state is ConfigEntryState.SETUP_RETRY assert entry.state is ConfigEntryState.SETUP_RETRY
flows = hass.config_entries.flow.async_progress()
assert len(flows) == 1
flow = flows[0]
assert flow.get("step_id") == "reauth_confirm"
assert flow.get("handler") == DOMAIN
assert "context" in flow
assert flow["context"].get("source") == SOURCE_REAUTH
assert flow["context"].get("entry_id") == entry.entry_id
async def test_load_removes_reauth_flow(aioclient_mock, hass):
"""Test setup removes reauth flow when API is enabled."""
device = get_mock_device()
entry = MockConfigEntry(
domain=DOMAIN,
data={CONF_IP_ADDRESS: "1.1.1.1"},
unique_id=DOMAIN,
)
entry.add_to_hass(hass)
# Add reauth flow from 'previously' failed init
entry.async_start_reauth(hass)
await hass.async_block_till_done()
flows = hass.config_entries.flow.async_progress_by_handler(DOMAIN)
assert len(flows) == 1
# Initialize entry
with patch(
"homeassistant.components.homewizard.coordinator.HomeWizardEnergy",
return_value=device,
):
await hass.config_entries.async_setup(entry.entry_id)
await hass.async_block_till_done()
assert entry.state is ConfigEntryState.LOADED
# Flow should be removed
flows = hass.config_entries.flow.async_progress_by_handler(DOMAIN)
assert len(flows) == 0
async def test_load_handles_homewizardenergy_exception(aioclient_mock, hass): async def test_load_handles_homewizardenergy_exception(aioclient_mock, hass):
"""Test setup handles exception from API.""" """Test setup handles exception from API."""

View File

@ -774,13 +774,3 @@ async def test_api_disabled(hass, mock_config_entry_data, mock_config_entry):
).state ).state
== "unavailable" == "unavailable"
) )
api.data.side_effect = None
async_fire_time_changed(hass, utcnow + timedelta(seconds=10))
await hass.async_block_till_done()
assert (
hass.states.get(
"sensor.product_name_aabbccddeeff_total_power_import_t1"
).state
== "1234.123"
)