Improve the code quality of the Discovergy integration (#94165)

* Remove option flow, refactor and improve the code quality after review in PR #54280

* Remove coordinator.py from coverage report

* Some minor improvements for unit tests

* Remove _LOGGER

* Use pytest.fixture and some more improvments

* Add back empty __init__

* Fix docstring

---------

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
This commit is contained in:
Jan-Philipp Benecke 2023-06-11 12:51:43 +02:00 committed by GitHub
parent a8dd2d520a
commit 7d0f5733c2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 210 additions and 267 deletions

View File

@ -204,6 +204,7 @@ omit =
homeassistant/components/discord/notify.py
homeassistant/components/discovergy/__init__.py
homeassistant/components/discovergy/sensor.py
homeassistant/components/discovergy/coordinator.py
homeassistant/components/dlib_face_detect/image_processing.py
homeassistant/components/dlib_face_identify/image_processing.py
homeassistant/components/dlink/data.py

View File

@ -1,37 +1,32 @@
"""The Discovergy integration."""
from __future__ import annotations
from dataclasses import dataclass, field
import logging
from dataclasses import dataclass
import pydiscovergy
from pydiscovergy.authentication import BasicAuth
import pydiscovergy.error as discovergyError
from pydiscovergy.models import Meter, Reading
from pydiscovergy.models import Meter
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import CONF_EMAIL, CONF_PASSWORD, Platform
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryNotReady
from homeassistant.helpers.httpx_client import get_async_client
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator
from .const import APP_NAME, DOMAIN
from .coordinator import DiscovergyUpdateCoordinator
PLATFORMS = [Platform.SENSOR]
_LOGGER = logging.getLogger(__name__)
@dataclass
class DiscovergyData:
"""Discovergy data class to share meters and api client."""
api_client: pydiscovergy.Discovergy = field(default_factory=lambda: None)
meters: list[Meter] = field(default_factory=lambda: [])
coordinators: dict[str, DataUpdateCoordinator[Reading]] = field(
default_factory=lambda: {}
)
api_client: pydiscovergy.Discovergy
meters: list[Meter]
coordinators: dict[str, DiscovergyUpdateCoordinator]
async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
@ -52,18 +47,30 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
)
try:
# try to get meters from api to check if access token is still valid and later use
# try to get meters from api to check if credentials are still valid and for later use
# if no exception is raised everything is fine to go
discovergy_data.meters = await discovergy_data.api_client.get_meters()
except discovergyError.InvalidLogin as err:
_LOGGER.debug("Invalid email or password: %s", err)
raise ConfigEntryAuthFailed("Invalid email or password") from err
except Exception as err: # pylint: disable=broad-except
_LOGGER.error("Unexpected error while communicating with API: %s", err)
raise ConfigEntryNotReady(
"Unexpected error while communicating with API"
"Unexpected error while while getting meters"
) from err
# Init coordinators for meters
for meter in discovergy_data.meters:
# Create coordinator for meter, set config entry and fetch initial data,
# so we have data when entities are added
coordinator = DiscovergyUpdateCoordinator(
hass=hass,
config_entry=entry,
meter=meter,
discovergy_client=discovergy_data.api_client,
)
await coordinator.async_config_entry_first_refresh()
discovergy_data.coordinators[meter.get_meter_id()] = coordinator
hass.data[DOMAIN][entry.entry_id] = discovergy_data
await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)

View File

@ -13,16 +13,10 @@ import voluptuous as vol
from homeassistant import config_entries
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import CONF_EMAIL, CONF_PASSWORD
from homeassistant.core import callback
from homeassistant.data_entry_flow import FlowResult
from homeassistant.helpers.httpx_client import get_async_client
from .const import (
APP_NAME,
CONF_TIME_BETWEEN_UPDATE,
DEFAULT_TIME_BETWEEN_UPDATE,
DOMAIN,
)
from .const import APP_NAME, DOMAIN
_LOGGER = logging.getLogger(__name__)
@ -75,10 +69,10 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
),
)
return await self._validate_and_save(dict(entry_data), step_id="reauth")
return await self._validate_and_save(entry_data, step_id="reauth")
async def _validate_and_save(
self, user_input: dict[str, Any] | None = None, step_id: str = "user"
self, user_input: Mapping[str, Any] | None = None, step_id: str = "user"
) -> FlowResult:
"""Validate user input and create config entry."""
errors = {}
@ -92,14 +86,12 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
httpx_client=get_async_client(self.hass),
authentication=BasicAuth(),
).get_meters()
result = {"title": user_input[CONF_EMAIL], "data": user_input}
except discovergyError.HTTPError:
errors["base"] = "cannot_connect"
except discovergyError.InvalidLogin:
errors["base"] = "invalid_auth"
except Exception: # pylint: disable=broad-except
_LOGGER.exception("Unexpected exception")
_LOGGER.exception("Unexpected error occurred while getting meters")
errors["base"] = "unknown"
else:
if self.existing_entry:
@ -116,11 +108,11 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
return self.async_abort(reason="reauth_successful")
# set unique id to title which is the account email
await self.async_set_unique_id(result["title"].lower())
await self.async_set_unique_id(user_input[CONF_EMAIL].lower())
self._abort_if_unique_id_configured()
return self.async_create_entry(
title=result["title"], data=result["data"]
title=user_input[CONF_EMAIL], data=user_input
)
return self.async_show_form(
@ -128,40 +120,3 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
data_schema=make_schema(),
errors=errors,
)
@staticmethod
@callback
def async_get_options_flow(
config_entry: config_entries.ConfigEntry,
) -> config_entries.OptionsFlow:
"""Create the options flow."""
return DiscovergyOptionsFlowHandler(config_entry)
class DiscovergyOptionsFlowHandler(config_entries.OptionsFlow):
"""Handle Discovergy options."""
def __init__(self, config_entry: config_entries.ConfigEntry) -> None:
"""Initialize options flow."""
self.config_entry = config_entry
async def async_step_init(
self, user_input: dict[str, Any] | None = None
) -> FlowResult:
"""Manage the options."""
if user_input is not None:
return self.async_create_entry(title="", data=user_input)
return self.async_show_form(
step_id="init",
data_schema=vol.Schema(
{
vol.Optional(
CONF_TIME_BETWEEN_UPDATE,
default=self.config_entry.options.get(
CONF_TIME_BETWEEN_UPDATE, DEFAULT_TIME_BETWEEN_UPDATE
),
): vol.All(vol.Coerce(int), vol.Range(min=1)),
}
),
)

View File

@ -4,5 +4,3 @@ from __future__ import annotations
DOMAIN = "discovergy"
MANUFACTURER = "Discovergy"
APP_NAME = "homeassistant"
CONF_TIME_BETWEEN_UPDATE = "time_between_update"
DEFAULT_TIME_BETWEEN_UPDATE = 30

View File

@ -0,0 +1,60 @@
"""DataUpdateCoordinator for the Discovergy integration."""
from __future__ import annotations
from datetime import timedelta
import logging
from pydiscovergy import Discovergy
from pydiscovergy.error import AccessTokenExpired, HTTPError
from pydiscovergy.models import Meter, Reading
from homeassistant.config_entries import ConfigEntry
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryAuthFailed
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
from .const import DOMAIN
_LOGGER = logging.getLogger(__name__)
class DiscovergyUpdateCoordinator(DataUpdateCoordinator[Reading]):
"""The Discovergy update coordinator."""
config_entry: ConfigEntry
discovergy_client: Discovergy
meter: Meter
def __init__(
self,
hass: HomeAssistant,
config_entry: ConfigEntry,
meter: Meter,
discovergy_client: Discovergy,
) -> None:
"""Initialize the Discovergy coordinator."""
self.config_entry = config_entry
self.meter = meter
self.discovergy_client = discovergy_client
super().__init__(
hass,
_LOGGER,
name=DOMAIN,
update_interval=timedelta(seconds=30),
)
async def _async_update_data(self) -> Reading:
"""Get last reading for meter."""
try:
return await self.discovergy_client.get_last_reading(
self.meter.get_meter_id()
)
except AccessTokenExpired as err:
raise ConfigEntryAuthFailed(
f"Auth expired while fetching last reading for meter {self.meter.get_meter_id()}"
) from err
except HTTPError as err:
raise UpdateFailed(
f"Error while fetching last reading for meter {self.meter.get_meter_id()}"
) from err

View File

@ -1,11 +1,7 @@
"""Discovergy sensor entity."""
from dataclasses import dataclass, field
from datetime import timedelta
import logging
from pydiscovergy import Discovergy
from pydiscovergy.error import AccessTokenExpired, HTTPError
from pydiscovergy.models import Meter, Reading
from pydiscovergy.models import Meter
from homeassistant.components.sensor import (
SensorDeviceClass,
@ -25,25 +21,14 @@ from homeassistant.const import (
UnitOfVolume,
)
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryAuthFailed
from homeassistant.helpers.entity_platform import AddEntitiesCallback
from homeassistant.helpers.typing import StateType
from homeassistant.helpers.update_coordinator import (
CoordinatorEntity,
DataUpdateCoordinator,
UpdateFailed,
)
from homeassistant.helpers.update_coordinator import CoordinatorEntity
from . import DiscovergyData
from .const import (
CONF_TIME_BETWEEN_UPDATE,
DEFAULT_TIME_BETWEEN_UPDATE,
DOMAIN,
MANUFACTURER,
)
from . import DiscovergyData, DiscovergyUpdateCoordinator
from .const import DOMAIN, MANUFACTURER
PARALLEL_UPDATES = 1
_LOGGER = logging.getLogger(__name__)
@dataclass
@ -160,63 +145,16 @@ ELECTRICITY_SENSORS: tuple[DiscovergySensorEntityDescription, ...] = (
)
def get_coordinator_for_meter(
hass: HomeAssistant,
meter: Meter,
discovergy_instance: Discovergy,
update_interval: timedelta,
) -> DataUpdateCoordinator[Reading]:
"""Create a new DataUpdateCoordinator for given meter."""
async def async_update_data() -> Reading:
"""Fetch data from API endpoint."""
try:
return await discovergy_instance.get_last_reading(meter.get_meter_id())
except AccessTokenExpired as err:
raise ConfigEntryAuthFailed(
"Got token expired while communicating with API"
) from err
except HTTPError as err:
raise UpdateFailed(f"Error communicating with API: {err}") from err
except Exception as err: # pylint: disable=broad-except
raise UpdateFailed(
f"Unexpected error while communicating with API: {err}"
) from err
coordinator = DataUpdateCoordinator(
hass,
_LOGGER,
name="sensor",
update_method=async_update_data,
update_interval=update_interval,
)
return coordinator
async def async_setup_entry(
hass: HomeAssistant, entry: ConfigEntry, async_add_entities: AddEntitiesCallback
) -> None:
"""Set up the Discovergy sensors."""
data: DiscovergyData = hass.data[DOMAIN][entry.entry_id]
discovergy_instance: Discovergy = data.api_client
meters: list[Meter] = data.meters # always returns a list
min_time_between_updates = timedelta(
seconds=entry.options.get(CONF_TIME_BETWEEN_UPDATE, DEFAULT_TIME_BETWEEN_UPDATE)
)
entities: list[DiscovergySensor] = []
for meter in meters:
# Get coordinator for meter, set config entry and fetch initial data
# so we have data when entities are added
coordinator = get_coordinator_for_meter(
hass, meter, discovergy_instance, min_time_between_updates
)
coordinator.config_entry = entry
await coordinator.async_config_entry_first_refresh()
# add coordinator to data for diagnostics
data.coordinators[meter.get_meter_id()] = coordinator
meter_id = meter.get_meter_id()
sensors = None
if meter.measurement_type == "ELECTRICITY":
@ -226,10 +164,11 @@ async def async_setup_entry(
if sensors is not None:
for description in sensors:
keys = [description.key] + description.alternative_keys
# check if this meter has this data, then add this sensor
for key in keys:
for key in {description.key, *description.alternative_keys}:
coordinator: DiscovergyUpdateCoordinator = data.coordinators[
meter_id
]
if key in coordinator.data.values:
entities.append(
DiscovergySensor(key, description, meter, coordinator)
@ -238,7 +177,7 @@ async def async_setup_entry(
async_add_entities(entities, False)
class DiscovergySensor(CoordinatorEntity[DataUpdateCoordinator[Reading]], SensorEntity):
class DiscovergySensor(CoordinatorEntity[DiscovergyUpdateCoordinator], SensorEntity):
"""Represents a discovergy smart meter sensor."""
entity_description: DiscovergySensorEntityDescription
@ -250,7 +189,7 @@ class DiscovergySensor(CoordinatorEntity[DataUpdateCoordinator[Reading]], Sensor
data_key: str,
description: DiscovergySensorEntityDescription,
meter: Meter,
coordinator: DataUpdateCoordinator[Reading],
coordinator: DiscovergyUpdateCoordinator,
) -> None:
"""Initialize the sensor."""
super().__init__(coordinator)
@ -258,7 +197,7 @@ class DiscovergySensor(CoordinatorEntity[DataUpdateCoordinator[Reading]], Sensor
self.data_key = data_key
self.entity_description = description
self._attr_unique_id = f"{meter.full_serial_number}-{description.key}"
self._attr_unique_id = f"{meter.full_serial_number}-{data_key}"
self._attr_device_info = {
ATTR_IDENTIFIERS: {(DOMAIN, meter.get_meter_id())},
ATTR_NAME: f"{meter.measurement_type.capitalize()} {meter.location.street} {meter.location.street_number}",

View File

@ -29,15 +29,6 @@
"api_endpoint_reachable": "Discovergy API endpoint reachable"
}
},
"options": {
"step": {
"init": {
"data": {
"time_between_update": "Minimum time between entity updates [s]"
}
}
}
},
"entity": {
"sensor": {
"total_gas_consumption": {

View File

@ -1,75 +1 @@
"""Tests for the Discovergy integration."""
import datetime
from unittest.mock import patch
from pydiscovergy.models import Meter, Reading
from homeassistant.components.discovergy import DOMAIN
from homeassistant.const import CONF_EMAIL, CONF_PASSWORD
from homeassistant.core import HomeAssistant
from tests.common import MockConfigEntry
GET_METERS = [
Meter(
meterId="f8d610b7a8cc4e73939fa33b990ded54",
serialNumber="abc123",
fullSerialNumber="abc123",
type="TST",
measurementType="ELECTRICITY",
loadProfileType="SLP",
location={
"city": "Testhause",
"street": "Teststraße",
"streetNumber": "1",
"country": "Germany",
},
manufacturerId="TST",
printedFullSerialNumber="abc123",
administrationNumber="12345",
scalingFactor=1,
currentScalingFactor=1,
voltageScalingFactor=1,
internalMeters=1,
firstMeasurementTime=1517569090926,
lastMeasurementTime=1678430543742,
),
]
LAST_READING = Reading(
time=datetime.datetime(2023, 3, 10, 7, 32, 6, 702000),
values={
"energy": 119348699715000.0,
"energy1": 2254180000.0,
"energy2": 119346445534000.0,
"energyOut": 55048723044000.0,
"energyOut1": 0.0,
"energyOut2": 0.0,
"power": 531750.0,
"power1": 142680.0,
"power2": 138010.0,
"power3": 251060.0,
"voltage1": 239800.0,
"voltage2": 239700.0,
"voltage3": 239000.0,
},
)
async def init_integration(hass: HomeAssistant) -> MockConfigEntry:
"""Set up the Discovergy integration in Home Assistant."""
entry = MockConfigEntry(
domain=DOMAIN,
title="user@example.org",
unique_id="user@example.org",
data={CONF_EMAIL: "user@example.org", CONF_PASSWORD: "supersecretpassword"},
)
with patch("pydiscovergy.Discovergy.get_meters", return_value=GET_METERS), patch(
"pydiscovergy.Discovergy.get_last_reading", return_value=LAST_READING
):
entry.add_to_hass(hass)
await hass.config_entries.async_setup(entry.entry_id)
await hass.async_block_till_done()
return entry
"""Test the Discovergy integration."""

View File

@ -3,7 +3,12 @@ from unittest.mock import AsyncMock, Mock, patch
import pytest
from tests.components.discovergy import GET_METERS
from homeassistant.components.discovergy import DOMAIN
from homeassistant.const import CONF_EMAIL, CONF_PASSWORD
from homeassistant.core import HomeAssistant
from tests.common import MockConfigEntry
from tests.components.discovergy.const import GET_METERS
@pytest.fixture
@ -12,3 +17,17 @@ def mock_meters() -> Mock:
with patch("pydiscovergy.Discovergy.get_meters") as discovergy:
discovergy.side_effect = AsyncMock(return_value=GET_METERS)
yield discovergy
@pytest.fixture
async def mock_config_entry(hass: HomeAssistant) -> MockConfigEntry:
"""Return a MockConfigEntry for testing."""
entry = MockConfigEntry(
domain=DOMAIN,
title="user@example.org",
unique_id="user@example.org",
data={CONF_EMAIL: "user@example.org", CONF_PASSWORD: "supersecretpassword"},
)
entry.add_to_hass(hass)
return entry

View File

@ -0,0 +1,49 @@
"""Constants for Discovergy integration tests."""
import datetime
from pydiscovergy.models import Meter, Reading
GET_METERS = [
Meter(
meterId="f8d610b7a8cc4e73939fa33b990ded54",
serialNumber="abc123",
fullSerialNumber="abc123",
type="TST",
measurementType="ELECTRICITY",
loadProfileType="SLP",
location={
"city": "Testhause",
"street": "Teststraße",
"streetNumber": "1",
"country": "Germany",
},
manufacturerId="TST",
printedFullSerialNumber="abc123",
administrationNumber="12345",
scalingFactor=1,
currentScalingFactor=1,
voltageScalingFactor=1,
internalMeters=1,
firstMeasurementTime=1517569090926,
lastMeasurementTime=1678430543742,
),
]
LAST_READING = Reading(
time=datetime.datetime(2023, 3, 10, 7, 32, 6, 702000),
values={
"energy": 119348699715000.0,
"energy1": 2254180000.0,
"energy2": 119346445534000.0,
"energyOut": 55048723044000.0,
"energyOut1": 0.0,
"energyOut2": 0.0,
"power": 531750.0,
"power1": 142680.0,
"power2": 138010.0,
"power3": 251060.0,
"voltage1": 239800.0,
"voltage2": 239700.0,
"voltage3": 239000.0,
},
)

View File

@ -1,20 +1,19 @@
"""Test the Discovergy config flow."""
from unittest.mock import patch
from unittest.mock import Mock, patch
from pydiscovergy.error import HTTPError, InvalidLogin
from homeassistant import data_entry_flow, setup
from homeassistant import data_entry_flow
from homeassistant.components.discovergy.const import DOMAIN
from homeassistant.config_entries import SOURCE_REAUTH, SOURCE_USER
from homeassistant.const import CONF_EMAIL, CONF_PASSWORD
from homeassistant.core import HomeAssistant
from tests.components.discovergy import init_integration
from tests.common import MockConfigEntry
async def test_form(hass: HomeAssistant, mock_meters) -> None:
async def test_form(hass: HomeAssistant, mock_meters: Mock) -> None:
"""Test we get the form."""
await setup.async_setup_component(hass, "persistent_notification", {})
result = await hass.config_entries.flow.async_init(
DOMAIN, context={"source": SOURCE_USER}
)
@ -43,29 +42,35 @@ async def test_form(hass: HomeAssistant, mock_meters) -> None:
assert len(mock_setup_entry.mock_calls) == 1
async def test_reauth(hass: HomeAssistant, mock_meters) -> None:
async def test_reauth(
hass: HomeAssistant, mock_meters: Mock, mock_config_entry: MockConfigEntry
) -> None:
"""Test reauth flow."""
entry = await init_integration(hass)
init_result = await hass.config_entries.flow.async_init(
DOMAIN,
context={"source": SOURCE_REAUTH, "unique_id": entry.unique_id},
context={"source": SOURCE_REAUTH, "unique_id": mock_config_entry.unique_id},
data=None,
)
assert init_result["type"] == data_entry_flow.FlowResultType.FORM
assert init_result["step_id"] == "reauth"
configure_result = await hass.config_entries.flow.async_configure(
init_result["flow_id"],
{
CONF_EMAIL: "test@example.com",
CONF_PASSWORD: "test-password",
},
)
with patch(
"homeassistant.components.discovergy.async_setup_entry",
return_value=True,
) as mock_setup_entry:
configure_result = await hass.config_entries.flow.async_configure(
init_result["flow_id"],
{
CONF_EMAIL: "test@example.com",
CONF_PASSWORD: "test-password",
},
)
await hass.async_block_till_done()
assert configure_result["type"] == data_entry_flow.FlowResultType.ABORT
assert configure_result["reason"] == "reauth_successful"
assert configure_result["type"] == data_entry_flow.FlowResultType.ABORT
assert configure_result["reason"] == "reauth_successful"
assert len(mock_setup_entry.mock_calls) == 1
async def test_form_invalid_auth(hass: HomeAssistant) -> None:
@ -126,20 +131,3 @@ async def test_form_unknown_exception(hass: HomeAssistant) -> None:
assert result2["type"] == data_entry_flow.FlowResultType.FORM
assert result2["errors"] == {"base": "unknown"}
async def test_options_flow_init(hass: HomeAssistant) -> None:
"""Test the options flow."""
entry = await init_integration(hass)
result = await hass.config_entries.options.async_init(entry.entry_id)
assert result["type"] == data_entry_flow.FlowResultType.FORM
assert result["step_id"] == "init"
create_result = await hass.config_entries.options.async_configure(
result["flow_id"], {"time_between_update": 2}
)
assert create_result["type"] == data_entry_flow.FlowResultType.CREATE_ENTRY
assert create_result["data"] == {"time_between_update": 2}

View File

@ -1,23 +1,33 @@
"""Test Discovergy diagnostics."""
from unittest.mock import patch
from homeassistant.components.diagnostics import REDACTED
from homeassistant.core import HomeAssistant
from . import init_integration
from tests.common import MockConfigEntry
from tests.components.diagnostics import get_diagnostics_for_config_entry
from tests.components.discovergy.const import GET_METERS, LAST_READING
from tests.typing import ClientSessionGenerator
async def test_entry_diagnostics(
hass: HomeAssistant, hass_client: ClientSessionGenerator
hass: HomeAssistant,
hass_client: ClientSessionGenerator,
mock_config_entry: MockConfigEntry,
) -> None:
"""Test config entry diagnostics."""
entry = await init_integration(hass)
with patch("pydiscovergy.Discovergy.get_meters", return_value=GET_METERS), patch(
"pydiscovergy.Discovergy.get_last_reading", return_value=LAST_READING
):
await hass.config_entries.async_setup(mock_config_entry.entry_id)
await hass.async_block_till_done()
result = await get_diagnostics_for_config_entry(hass, hass_client, entry)
result = await get_diagnostics_for_config_entry(
hass, hass_client, mock_config_entry
)
assert result["entry"] == {
"entry_id": entry.entry_id,
"entry_id": mock_config_entry.entry_id,
"version": 1,
"domain": "discovergy",
"title": REDACTED,