From 45d1f8bc55e72c7bbe5f8e011b05e8a1d13f8718 Mon Sep 17 00:00:00 2001 From: stegm Date: Mon, 18 Jul 2022 23:08:18 +0200 Subject: [PATCH] Address late review of kostal plenticore (#75297) * Changtes from review #64927 * Fix unit tests for number. * Changes from review. --- .../components/kostal_plenticore/__init__.py | 2 +- .../components/kostal_plenticore/const.py | 54 ---- .../components/kostal_plenticore/number.py | 72 ++++- .../kostal_plenticore/test_number.py | 265 +++++++++--------- 4 files changed, 201 insertions(+), 192 deletions(-) diff --git a/homeassistant/components/kostal_plenticore/__init__.py b/homeassistant/components/kostal_plenticore/__init__.py index 6c7a7cde523..24e8ab9f0d3 100644 --- a/homeassistant/components/kostal_plenticore/__init__.py +++ b/homeassistant/components/kostal_plenticore/__init__.py @@ -12,7 +12,7 @@ from .helper import Plenticore _LOGGER = logging.getLogger(__name__) -PLATFORMS = [Platform.SELECT, Platform.SENSOR, Platform.SWITCH, Platform.NUMBER] +PLATFORMS = [Platform.NUMBER, Platform.SELECT, Platform.SENSOR, Platform.SWITCH] async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: diff --git a/homeassistant/components/kostal_plenticore/const.py b/homeassistant/components/kostal_plenticore/const.py index 11bb794f799..7ae0b13f0e8 100644 --- a/homeassistant/components/kostal_plenticore/const.py +++ b/homeassistant/components/kostal_plenticore/const.py @@ -1,8 +1,6 @@ """Constants for the Kostal Plenticore Solar Inverter integration.""" -from dataclasses import dataclass from typing import NamedTuple -from homeassistant.components.number import NumberEntityDescription from homeassistant.components.sensor import ( ATTR_STATE_CLASS, SensorDeviceClass, @@ -18,7 +16,6 @@ from homeassistant.const import ( PERCENTAGE, POWER_WATT, ) -from homeassistant.helpers.entity import EntityCategory DOMAIN = "kostal_plenticore" @@ -794,57 +791,6 @@ SENSOR_PROCESS_DATA = [ ] -@dataclass -class PlenticoreNumberEntityDescriptionMixin: - """Define an entity description mixin for number entities.""" - - module_id: str - data_id: str - fmt_from: str - fmt_to: str - - -@dataclass -class PlenticoreNumberEntityDescription( - NumberEntityDescription, PlenticoreNumberEntityDescriptionMixin -): - """Describes a Plenticore number entity.""" - - -NUMBER_SETTINGS_DATA = [ - PlenticoreNumberEntityDescription( - key="battery_min_soc", - entity_category=EntityCategory.CONFIG, - entity_registry_enabled_default=False, - icon="mdi:battery-negative", - name="Battery min SoC", - native_unit_of_measurement=PERCENTAGE, - native_max_value=100, - native_min_value=5, - native_step=5, - module_id="devices:local", - data_id="Battery:MinSoc", - fmt_from="format_round", - fmt_to="format_round_back", - ), - PlenticoreNumberEntityDescription( - key="battery_min_home_consumption", - device_class=SensorDeviceClass.POWER, - entity_category=EntityCategory.CONFIG, - entity_registry_enabled_default=False, - name="Battery min Home Consumption", - native_unit_of_measurement=POWER_WATT, - native_max_value=38000, - native_min_value=50, - native_step=1, - module_id="devices:local", - data_id="Battery:MinHomeComsumption", - fmt_from="format_round", - fmt_to="format_round_back", - ), -] - - class SwitchData(NamedTuple): """Representation of a SelectData tuple.""" diff --git a/homeassistant/components/kostal_plenticore/number.py b/homeassistant/components/kostal_plenticore/number.py index 1ad911f6d15..69fba631b34 100644 --- a/homeassistant/components/kostal_plenticore/number.py +++ b/homeassistant/components/kostal_plenticore/number.py @@ -2,25 +2,82 @@ from __future__ import annotations from abc import ABC +from dataclasses import dataclass from datetime import timedelta -from functools import partial import logging from kostal.plenticore import SettingsData -from homeassistant.components.number import NumberEntity, NumberMode +from homeassistant.components.number import ( + NumberEntity, + NumberEntityDescription, + NumberMode, +) +from homeassistant.components.sensor import SensorDeviceClass from homeassistant.config_entries import ConfigEntry +from homeassistant.const import PERCENTAGE, POWER_WATT from homeassistant.core import HomeAssistant -from homeassistant.helpers.entity import DeviceInfo +from homeassistant.helpers.entity import DeviceInfo, EntityCategory from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.update_coordinator import CoordinatorEntity -from .const import DOMAIN, NUMBER_SETTINGS_DATA, PlenticoreNumberEntityDescription +from .const import DOMAIN from .helper import PlenticoreDataFormatter, SettingDataUpdateCoordinator _LOGGER = logging.getLogger(__name__) +@dataclass +class PlenticoreNumberEntityDescriptionMixin: + """Define an entity description mixin for number entities.""" + + module_id: str + data_id: str + fmt_from: str + fmt_to: str + + +@dataclass +class PlenticoreNumberEntityDescription( + NumberEntityDescription, PlenticoreNumberEntityDescriptionMixin +): + """Describes a Plenticore number entity.""" + + +NUMBER_SETTINGS_DATA = [ + PlenticoreNumberEntityDescription( + key="battery_min_soc", + entity_category=EntityCategory.CONFIG, + entity_registry_enabled_default=False, + icon="mdi:battery-negative", + name="Battery min SoC", + native_unit_of_measurement=PERCENTAGE, + native_max_value=100, + native_min_value=5, + native_step=5, + module_id="devices:local", + data_id="Battery:MinSoc", + fmt_from="format_round", + fmt_to="format_round_back", + ), + PlenticoreNumberEntityDescription( + key="battery_min_home_consumption", + device_class=SensorDeviceClass.POWER, + entity_category=EntityCategory.CONFIG, + entity_registry_enabled_default=False, + name="Battery min Home Consumption", + native_unit_of_measurement=POWER_WATT, + native_max_value=38000, + native_min_value=50, + native_step=1, + module_id="devices:local", + data_id="Battery:MinHomeComsumption", + fmt_from="format_round", + fmt_to="format_round_back", + ), +] + + async def async_setup_entry( hass: HomeAssistant, entry: ConfigEntry, async_add_entities: AddEntitiesCallback ) -> None: @@ -54,10 +111,9 @@ async def async_setup_entry( continue setting_data = next( - filter( - partial(lambda id, sd: id == sd.id, description.data_id), - available_settings_data[description.module_id], - ) + sd + for sd in available_settings_data[description.module_id] + if description.data_id == sd.id ) entities.append( diff --git a/tests/components/kostal_plenticore/test_number.py b/tests/components/kostal_plenticore/test_number.py index f0fb42f6e78..f6978612cff 100644 --- a/tests/components/kostal_plenticore/test_number.py +++ b/tests/components/kostal_plenticore/test_number.py @@ -1,72 +1,98 @@ """Test Kostal Plenticore number.""" -from unittest.mock import AsyncMock, MagicMock +from collections.abc import Generator +from datetime import timedelta +from unittest.mock import patch -from kostal.plenticore import SettingsData +from kostal.plenticore import PlenticoreApiClient, SettingsData import pytest -from homeassistant.components.kostal_plenticore.const import ( - PlenticoreNumberEntityDescription, +from homeassistant.components.number import ( + ATTR_VALUE, + DOMAIN as NUMBER_DOMAIN, + SERVICE_SET_VALUE, ) -from homeassistant.components.kostal_plenticore.number import PlenticoreDataNumber +from homeassistant.components.number.const import ATTR_MAX, ATTR_MIN +from homeassistant.const import ATTR_ENTITY_ID, STATE_UNAVAILABLE from homeassistant.core import HomeAssistant from homeassistant.helpers.entity_registry import async_get +from homeassistant.util import dt -from tests.common import MockConfigEntry +from tests.common import MockConfigEntry, async_fire_time_changed @pytest.fixture -def mock_coordinator() -> MagicMock: - """Return a mocked coordinator for tests.""" - coordinator = MagicMock() - coordinator.async_write_data = AsyncMock() - coordinator.async_refresh = AsyncMock() - return coordinator +def mock_plenticore_client() -> Generator[PlenticoreApiClient, None, None]: + """Return a patched PlenticoreApiClient.""" + with patch( + "homeassistant.components.kostal_plenticore.helper.PlenticoreApiClient", + autospec=True, + ) as plenticore_client_class: + yield plenticore_client_class.return_value @pytest.fixture -def mock_number_description() -> PlenticoreNumberEntityDescription: - """Return a PlenticoreNumberEntityDescription for tests.""" - return PlenticoreNumberEntityDescription( - key="mock key", - module_id="moduleid", - data_id="dataid", - native_min_value=0, - native_max_value=1000, - fmt_from="format_round", - fmt_to="format_round_back", - ) +def mock_get_setting_values(mock_plenticore_client: PlenticoreApiClient) -> list: + """Add a setting value to the given Plenticore client. + Returns a list with setting values which can be extended by test cases. + """ -@pytest.fixture -def mock_setting_data() -> SettingsData: - """Return a default SettingsData for tests.""" - return SettingsData( - { - "default": None, - "min": None, - "access": None, - "max": None, - "unit": None, - "type": None, - "id": "data_id", - } - ) - - -async def test_setup_all_entries( - hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_plenticore: MagicMock -): - """Test if all available entries are setup up.""" - mock_plenticore.client.get_settings.return_value = { + mock_plenticore_client.get_settings.return_value = { "devices:local": [ - SettingsData({"id": "Battery:MinSoc", "min": None, "max": None}), SettingsData( - {"id": "Battery:MinHomeComsumption", "min": None, "max": None} + { + "default": None, + "min": 5, + "max": 100, + "access": "readwrite", + "unit": "%", + "type": "byte", + "id": "Battery:MinSoc", + } + ), + SettingsData( + { + "default": None, + "min": 50, + "max": 38000, + "access": "readwrite", + "unit": "W", + "type": "byte", + "id": "Battery:MinHomeComsumption", + } ), ] } + # this values are always retrieved by the integration on startup + setting_values = [ + { + "devices:local": { + "Properties:SerialNo": "42", + "Branding:ProductName1": "PLENTICORE", + "Branding:ProductName2": "plus 10", + "Properties:VersionIOC": "01.45", + "Properties:VersionMC": " 01.46", + }, + "scb:network": {"Hostname": "scb"}, + } + ] + + mock_plenticore_client.get_setting_values.side_effect = setting_values + + return setting_values + + +async def test_setup_all_entries( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_plenticore_client: PlenticoreApiClient, + mock_get_setting_values: list, + entity_registry_enabled_by_default, +): + """Test if all available entries are setup.""" + mock_config_entry.add_to_hass(hass) assert await hass.config_entries.async_setup(mock_config_entry.entry_id) @@ -78,10 +104,16 @@ async def test_setup_all_entries( async def test_setup_no_entries( - hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_plenticore: MagicMock + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_plenticore_client: PlenticoreApiClient, + mock_get_setting_values: list, + entity_registry_enabled_by_default, ): - """Test that no entries are setup up.""" - mock_plenticore.client.get_settings.return_value = [] + """Test that no entries are setup if Plenticore does not provide data.""" + + mock_plenticore_client.get_settings.return_value = [] + mock_config_entry.add_to_hass(hass) assert await hass.config_entries.async_setup(mock_config_entry.entry_id) @@ -92,106 +124,81 @@ async def test_setup_no_entries( assert ent_reg.async_get("number.scb_battery_min_home_consumption") is None -def test_number_returns_value_if_available( - mock_coordinator: MagicMock, - mock_number_description: PlenticoreNumberEntityDescription, - mock_setting_data: SettingsData, +async def test_number_has_value( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_plenticore_client: PlenticoreApiClient, + mock_get_setting_values: list, + entity_registry_enabled_by_default, ): - """Test if value property on PlenticoreDataNumber returns an int if available.""" + """Test if number has a value if data is provided on update.""" - mock_coordinator.data = {"moduleid": {"dataid": "42"}} + mock_get_setting_values.append({"devices:local": {"Battery:MinSoc": "42"}}) - entity = PlenticoreDataNumber( - mock_coordinator, "42", "scb", None, mock_number_description, mock_setting_data - ) + mock_config_entry.add_to_hass(hass) - assert entity.value == 42 - assert type(entity.value) == int + await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + async_fire_time_changed(hass, dt.utcnow() + timedelta(seconds=3)) + await hass.async_block_till_done() + + state = hass.states.get("number.scb_battery_min_soc") + assert state.state == "42" + assert state.attributes[ATTR_MIN] == 5 + assert state.attributes[ATTR_MAX] == 100 -def test_number_returns_none_if_unavailable( - mock_coordinator: MagicMock, - mock_number_description: PlenticoreNumberEntityDescription, - mock_setting_data: SettingsData, +async def test_number_is_unavailable( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_plenticore_client: PlenticoreApiClient, + mock_get_setting_values: list, + entity_registry_enabled_by_default, ): - """Test if value property on PlenticoreDataNumber returns none if unavailable.""" + """Test if number is unavailable if no data is provided on update.""" - mock_coordinator.data = {} # makes entity not available + mock_config_entry.add_to_hass(hass) - entity = PlenticoreDataNumber( - mock_coordinator, "42", "scb", None, mock_number_description, mock_setting_data - ) + await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() - assert entity.value is None + async_fire_time_changed(hass, dt.utcnow() + timedelta(seconds=3)) + await hass.async_block_till_done() + + state = hass.states.get("number.scb_battery_min_soc") + assert state.state == STATE_UNAVAILABLE async def test_set_value( - mock_coordinator: MagicMock, - mock_number_description: PlenticoreNumberEntityDescription, - mock_setting_data: SettingsData, + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_plenticore_client: PlenticoreApiClient, + mock_get_setting_values: list, + entity_registry_enabled_by_default, ): - """Test if set value calls coordinator with new value.""" + """Test if a new value could be set.""" - entity = PlenticoreDataNumber( - mock_coordinator, "42", "scb", None, mock_number_description, mock_setting_data - ) + mock_get_setting_values.append({"devices:local": {"Battery:MinSoc": "42"}}) - await entity.async_set_native_value(42) + mock_config_entry.add_to_hass(hass) - mock_coordinator.async_write_data.assert_called_once_with( - "moduleid", {"dataid": "42"} - ) - mock_coordinator.async_refresh.assert_called_once() + await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + async_fire_time_changed(hass, dt.utcnow() + timedelta(seconds=3)) + await hass.async_block_till_done() -async def test_minmax_overwrite( - mock_coordinator: MagicMock, - mock_number_description: PlenticoreNumberEntityDescription, -): - """Test if min/max value is overwritten from retrieved settings data.""" - - setting_data = SettingsData( + await hass.services.async_call( + NUMBER_DOMAIN, + SERVICE_SET_VALUE, { - "min": "5", - "max": "100", - } + ATTR_ENTITY_ID: "number.scb_battery_min_soc", + ATTR_VALUE: 80, + }, + blocking=True, ) - entity = PlenticoreDataNumber( - mock_coordinator, "42", "scb", None, mock_number_description, setting_data + mock_plenticore_client.set_setting_values.assert_called_once_with( + "devices:local", {"Battery:MinSoc": "80"} ) - - assert entity.min_value == 5 - assert entity.max_value == 100 - - -async def test_added_to_hass( - mock_coordinator: MagicMock, - mock_number_description: PlenticoreNumberEntityDescription, - mock_setting_data: SettingsData, -): - """Test if coordinator starts fetching after added to hass.""" - - entity = PlenticoreDataNumber( - mock_coordinator, "42", "scb", None, mock_number_description, mock_setting_data - ) - - await entity.async_added_to_hass() - - mock_coordinator.start_fetch_data.assert_called_once_with("moduleid", "dataid") - - -async def test_remove_from_hass( - mock_coordinator: MagicMock, - mock_number_description: PlenticoreNumberEntityDescription, - mock_setting_data: SettingsData, -): - """Test if coordinator stops fetching after remove from hass.""" - - entity = PlenticoreDataNumber( - mock_coordinator, "42", "scb", None, mock_number_description, mock_setting_data - ) - - await entity.async_will_remove_from_hass() - - mock_coordinator.stop_fetch_data.assert_called_once_with("moduleid", "dataid")