From 40104de0bf4b2bf6adfdcc159ec4b55124b35ca2 Mon Sep 17 00:00:00 2001 From: Ullrich Neiss Date: Sat, 20 Nov 2021 11:16:53 +0100 Subject: [PATCH] Address late review of kostal plenticore (#59998) --- .coveragerc | 2 +- .../components/kostal_plenticore/__init__.py | 2 +- .../components/kostal_plenticore/const.py | 20 ++++++-- .../components/kostal_plenticore/helper.py | 4 +- .../components/kostal_plenticore/select.py | 48 +++++++++---------- .../components/kostal_plenticore/switch.py | 18 ++----- 6 files changed, 45 insertions(+), 49 deletions(-) diff --git a/.coveragerc b/.coveragerc index e1ef57b9817..25ef70250cc 100644 --- a/.coveragerc +++ b/.coveragerc @@ -550,9 +550,9 @@ omit = homeassistant/components/kostal_plenticore/__init__.py homeassistant/components/kostal_plenticore/const.py homeassistant/components/kostal_plenticore/helper.py + homeassistant/components/kostal_plenticore/select.py homeassistant/components/kostal_plenticore/sensor.py homeassistant/components/kostal_plenticore/switch.py - homeassistant/components/kostal_plenticore/select.py homeassistant/components/kwb/sensor.py homeassistant/components/lacrosse/sensor.py homeassistant/components/lametric/* diff --git a/homeassistant/components/kostal_plenticore/__init__.py b/homeassistant/components/kostal_plenticore/__init__.py index 8e2beb73cc2..f5c973cc499 100644 --- a/homeassistant/components/kostal_plenticore/__init__.py +++ b/homeassistant/components/kostal_plenticore/__init__.py @@ -11,7 +11,7 @@ from .helper import Plenticore _LOGGER = logging.getLogger(__name__) -PLATFORMS = ["sensor", "switch", "select"] +PLATFORMS = ["select", "sensor", "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 b025738a7b8..ebed1ddcb74 100644 --- a/homeassistant/components/kostal_plenticore/const.py +++ b/homeassistant/components/kostal_plenticore/const.py @@ -1,5 +1,4 @@ """Constants for the Kostal Plenticore Solar Inverter integration.""" -from collections import namedtuple from typing import NamedTuple from homeassistant.components.sensor import ( @@ -692,6 +691,20 @@ SENSOR_SETTINGS_DATA = [ ), ] + +class SwitchData(NamedTuple): + """Representation of a SelectData tuple.""" + + module_id: str + data_id: str + name: str + is_on: str + on_value: str + on_label: str + off_value: str + off_label: str + + # Defines all entities for switches. # # Each entry is defined with a tuple of these values: @@ -702,11 +715,8 @@ SENSOR_SETTINGS_DATA = [ # - on Label (str) # - off Value (str) # - off Label (str) -SWITCH = namedtuple( - "SWITCH", "module_id data_id name is_on on_value on_label off_value off_label" -) SWITCH_SETTINGS_DATA = [ - SWITCH( + SwitchData( "devices:local", "Battery:Strategy", "Battery Strategy:", diff --git a/homeassistant/components/kostal_plenticore/helper.py b/homeassistant/components/kostal_plenticore/helper.py index 264d7e90efb..fd367230c6c 100644 --- a/homeassistant/components/kostal_plenticore/helper.py +++ b/homeassistant/components/kostal_plenticore/helper.py @@ -278,11 +278,11 @@ class SelectDataUpdateCoordinator( _LOGGER.debug("Fetching select %s for %s", self.name, self._fetch) - fetched_data = await self.async_get_currentoption(self._fetch) + fetched_data = await self._async_get_current_option(self._fetch) return fetched_data - async def async_get_currentoption( + async def _async_get_current_option( self, module_id: str | dict[str, Iterable[str]], ) -> dict[str, dict[str, str]]: diff --git a/homeassistant/components/kostal_plenticore/select.py b/homeassistant/components/kostal_plenticore/select.py index 1f9d11dc334..4e594095f7e 100644 --- a/homeassistant/components/kostal_plenticore/select.py +++ b/homeassistant/components/kostal_plenticore/select.py @@ -10,6 +10,7 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.const import DEVICE_DEFAULT_NAME from homeassistant.core import HomeAssistant from homeassistant.helpers.entity import DeviceInfo +from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.update_coordinator import CoordinatorEntity from .const import DOMAIN, SELECT_SETTINGS_DATA @@ -19,15 +20,21 @@ _LOGGER = logging.getLogger(__name__) async def async_setup_entry( - hass: HomeAssistant, entry: ConfigEntry, async_add_entities + hass: HomeAssistant, entry: ConfigEntry, async_add_entities: AddEntitiesCallback ) -> None: """Add kostal plenticore Select widget.""" plenticore: Plenticore = hass.data[DOMAIN][entry.entry_id] + select_data_update_coordinator = SelectDataUpdateCoordinator( + hass, + _LOGGER, + "Settings Data", + timedelta(seconds=30), + plenticore, + ) async_add_entities( PlenticoreDataSelect( - hass=hass, - plenticore=plenticore, + select_data_update_coordinator, entry_id=entry.entry_id, platform_name=entry.title, device_class="kostal_plenticore__battery", @@ -45,12 +52,11 @@ async def async_setup_entry( class PlenticoreDataSelect(CoordinatorEntity, SelectEntity, ABC): - """Representation of a Plenticore Switch.""" + """Representation of a Plenticore Select.""" def __init__( self, - hass: HomeAssistant, - plenticore: Plenticore, + coordinator, entry_id: str, platform_name: str, device_class: str | None, @@ -63,17 +69,8 @@ class PlenticoreDataSelect(CoordinatorEntity, SelectEntity, ABC): device_info: DeviceInfo, unique_id: str, ) -> None: - """Create a new switch Entity for Plenticore process data.""" - super().__init__( - coordinator=SelectDataUpdateCoordinator( - hass, - _LOGGER, - "Select Data", - timedelta(seconds=30), - plenticore, - ) - ) - self.plenticore = plenticore + """Create a new Select Entity for Plenticore process data.""" + super().__init__(coordinator) self.entry_id = entry_id self.platform_name = platform_name self._attr_device_class = device_class @@ -90,20 +87,13 @@ class PlenticoreDataSelect(CoordinatorEntity, SelectEntity, ABC): @property def available(self) -> bool: """Return if entity is available.""" - is_available = ( + return ( super().available and self.coordinator.data is not None and self.module_id in self.coordinator.data and self.data_id in self.coordinator.data[self.module_id] ) - if is_available: - self._attr_current_option = self.coordinator.data[self.module_id][ - self.data_id - ] - - return is_available - async def async_added_to_hass(self) -> None: """Register this entity on the Update Coordinator.""" await super().async_added_to_hass() @@ -127,3 +117,11 @@ class PlenticoreDataSelect(CoordinatorEntity, SelectEntity, ABC): if option != "None": await self.coordinator.async_write_data(self.module_id, {option: "1"}) self.async_write_ha_state() + + @property + def current_option(self) -> str | None: + """Return the selected entity option to represent the entity state.""" + if self.available: + return self.coordinator.data[self.module_id][self.data_id] + + return None diff --git a/homeassistant/components/kostal_plenticore/switch.py b/homeassistant/components/kostal_plenticore/switch.py index 9598e12aac0..b3b1ba29e84 100644 --- a/homeassistant/components/kostal_plenticore/switch.py +++ b/homeassistant/components/kostal_plenticore/switch.py @@ -4,12 +4,12 @@ from __future__ import annotations from abc import ABC from datetime import timedelta import logging -from typing import Any from homeassistant.components.switch import SwitchEntity from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant from homeassistant.helpers.entity import DeviceInfo +from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.update_coordinator import CoordinatorEntity from .const import DOMAIN, SWITCH_SETTINGS_DATA @@ -19,7 +19,7 @@ _LOGGER = logging.getLogger(__name__) async def async_setup_entry( - hass: HomeAssistant, entry: ConfigEntry, async_add_entities + hass: HomeAssistant, entry: ConfigEntry, async_add_entities: AddEntitiesCallback ): """Add kostal plenticore Switch.""" plenticore = hass.data[DOMAIN][entry.entry_id] @@ -87,13 +87,12 @@ class PlenticoreDataSwitch(CoordinatorEntity, SwitchEntity, ABC): attr_name: str, attr_unique_id: str, ): - """Create a new switch Entity for Plenticore process data.""" + """Create a new Switch Entity for Plenticore process data.""" super().__init__(coordinator) self.entry_id = entry_id self.platform_name = platform_name self.module_id = module_id self.data_id = data_id - self._last_run_success: bool | None = None self._name = name self._is_on = is_on self._attr_name = attr_name @@ -130,24 +129,18 @@ class PlenticoreDataSwitch(CoordinatorEntity, SwitchEntity, ABC): if await self.coordinator.async_write_data( self.module_id, {self.data_id: self.on_value} ): - self._last_run_success = True self.coordinator.name = f"{self.platform_name} {self._name} {self.on_label}" await self.coordinator.async_request_refresh() - else: - self._last_run_success = False async def async_turn_off(self, **kwargs) -> None: """Turn device off.""" if await self.coordinator.async_write_data( self.module_id, {self.data_id: self.off_value} ): - self._last_run_success = True self.coordinator.name = ( f"{self.platform_name} {self._name} {self.off_label}" ) await self.coordinator.async_request_refresh() - else: - self._last_run_success = False @property def device_info(self) -> DeviceInfo: @@ -164,8 +157,3 @@ class PlenticoreDataSwitch(CoordinatorEntity, SwitchEntity, ABC): f"{self.platform_name} {self._name} {self.off_label}" ) return bool(self.coordinator.data[self.module_id][self.data_id] == self._is_on) - - @property - def extra_state_attributes(self) -> dict[str, Any]: - """Return the state attributes.""" - return {"last_run_success": self._last_run_success}