From b272d9cf9aa29aa7ba1ebc61ae21c4d01026834d Mon Sep 17 00:00:00 2001 From: Andre Richter Date: Sat, 2 Oct 2021 21:24:55 +0000 Subject: [PATCH 1/6] Use DataUpdateCoordinator in Vallox --- homeassistant/components/vallox/__init__.py | 143 ++++++++++---------- homeassistant/components/vallox/const.py | 3 +- homeassistant/components/vallox/fan.py | 112 ++++++--------- homeassistant/components/vallox/sensor.py | 132 ++++++------------ 4 files changed, 151 insertions(+), 239 deletions(-) diff --git a/homeassistant/components/vallox/__init__.py b/homeassistant/components/vallox/__init__.py index 620114863a8d7f..a2c811a3cd123a 100644 --- a/homeassistant/components/vallox/__init__.py +++ b/homeassistant/components/vallox/__init__.py @@ -1,7 +1,7 @@ """Support for Vallox ventilation units.""" from __future__ import annotations -from datetime import datetime +from dataclasses import dataclass, field import ipaddress import logging from typing import Any @@ -15,9 +15,8 @@ from homeassistant.core import HomeAssistant, ServiceCall from homeassistant.helpers import config_validation as cv from homeassistant.helpers.discovery import async_load_platform -from homeassistant.helpers.dispatcher import async_dispatcher_send -from homeassistant.helpers.event import async_track_time_interval from homeassistant.helpers.typing import ConfigType, StateType +from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed from .const import ( DEFAULT_FAN_SPEED_AWAY, @@ -28,8 +27,7 @@ METRIC_KEY_PROFILE_FAN_SPEED_AWAY, METRIC_KEY_PROFILE_FAN_SPEED_BOOST, METRIC_KEY_PROFILE_FAN_SPEED_HOME, - SIGNAL_VALLOX_STATE_UPDATE, - STATE_PROXY_SCAN_INTERVAL, + STATE_SCAN_INTERVAL, STR_TO_VALLOX_PROFILE_SETTABLE, ) @@ -91,109 +89,110 @@ } -async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: - """Set up the client and boot the platforms.""" - conf = config[DOMAIN] - host = conf.get(CONF_HOST) - name = conf.get(CONF_NAME) - - client = Vallox(host) - state_proxy = ValloxStateProxy(hass, client) - service_handler = ValloxServiceHandler(client, state_proxy) - - hass.data[DOMAIN] = {"client": client, "state_proxy": state_proxy, "name": name} - - for vallox_service, method in SERVICE_TO_METHOD.items(): - schema = method["schema"] - hass.services.async_register( - DOMAIN, vallox_service, service_handler.async_handle, schema=schema - ) - - # The vallox hardware expects quite strict timings for websocket requests. Timings that machines - # with less processing power, like Raspberries, cannot live up to during the busy start phase of - # Home Asssistant. Hence, async_add_entities() for fan and sensor in respective code will be - # called with update_before_add=False to intentionally delay the first request, increasing - # chance that it is issued only when the machine is less busy again. - hass.async_create_task(async_load_platform(hass, "sensor", DOMAIN, {}, config)) - hass.async_create_task(async_load_platform(hass, "fan", DOMAIN, {}, config)) +@dataclass +class ValloxState: + """Describes the current state of the unit.""" - async_track_time_interval(hass, state_proxy.async_update, STATE_PROXY_SCAN_INTERVAL) - - return True - - -class ValloxStateProxy: - """Helper class to reduce websocket API calls.""" - - def __init__(self, hass: HomeAssistant, client: Vallox) -> None: - """Initialize the proxy.""" - self._hass = hass - self._client = client - self._metric_cache: dict[str, Any] = {} - self._profile = VALLOX_PROFILE.NONE - self._valid = False + metric_cache: dict[str, Any] = field(default_factory=dict) + profile: VALLOX_PROFILE = VALLOX_PROFILE.NONE - def fetch_metric(self, metric_key: str) -> StateType: + def get_metric(self, metric_key: str) -> StateType: """Return cached state value.""" _LOGGER.debug("Fetching metric key: %s", metric_key) - if not self._valid: - raise OSError("Device state out of sync.") - if metric_key not in vlxDevConstants.__dict__: - raise KeyError(f"Unknown metric key: {metric_key}") + _LOGGER.error("Metric key invalid: %s", metric_key) - if (value := self._metric_cache[metric_key]) is None: + if (value := self.metric_cache.get(metric_key)) is None: return None if not isinstance(value, (str, int, float)): - raise TypeError( - f"Return value of metric {metric_key} has unexpected type {type(value)}" + _LOGGER.error( + "Return value of metric %s has unexpected type %s", + metric_key, + type(value), ) + return None return value - def get_profile(self) -> VALLOX_PROFILE: - """Return cached profile value.""" - _LOGGER.debug("Returning profile") - if not self._valid: - raise OSError("Device state out of sync.") +async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: + """Set up the client and boot the platforms.""" + conf = config[DOMAIN] + host = conf.get(CONF_HOST) + name = conf.get(CONF_NAME) - return self._profile + client = Vallox(host) - async def async_update(self, time: datetime | None = None) -> None: + async def async_update_data() -> ValloxState: """Fetch state update.""" _LOGGER.debug("Updating Vallox state cache") try: - self._metric_cache = await self._client.fetch_metrics() - self._profile = await self._client.get_profile() + metric_cache = await client.fetch_metrics() + profile = await client.get_profile() except (OSError, ValloxApiException) as err: - self._valid = False - _LOGGER.error("Error during state cache update: %s", err) - return + raise UpdateFailed("Error during state cache update") from err + + return ValloxState(metric_cache, profile) + + coordinator = DataUpdateCoordinator[ValloxState]( + hass, + _LOGGER, + name=f"{name} DataUpdateCoordinator", + update_interval=STATE_SCAN_INTERVAL, + update_method=async_update_data, + ) - self._valid = True - async_dispatcher_send(self._hass, SIGNAL_VALLOX_STATE_UPDATE) + # The vallox hardware expects quite strict timings for websocket requests. Timings that machines + # with less processing power, like a Raspberry Pi, cannot live up to during the busy start phase + # of Home Asssistant. Hence, "async_add_entities()" for any platform code will be called with + # "update_before_add=False" to intentionally delay the first request, increasing chance that it + # is issued only when the machine is less busy again. + # + # For the same reason, don't call "coordinator.async_refresh()" here. Instead, set + # "coordinator.last_update_success" to "False", so that Home Assistant does not try to update + # the state of entities whose code assumes that "coordinator.data" is never "None". + # + # In summary, this will cause the first websocket requests to the Vallox client to go out after + # one "coordinator.update_interval" has passed. + coordinator.last_update_success = False + + service_handler = ValloxServiceHandler(client, coordinator) + for vallox_service, method in SERVICE_TO_METHOD.items(): + schema = method["schema"] + hass.services.async_register( + DOMAIN, vallox_service, service_handler.async_handle, schema=schema + ) + + hass.data[DOMAIN] = {"client": client, "coordinator": coordinator, "name": name} + + hass.async_create_task(async_load_platform(hass, "sensor", DOMAIN, {}, config)) + hass.async_create_task(async_load_platform(hass, "fan", DOMAIN, {}, config)) + + return True class ValloxServiceHandler: """Services implementation.""" - def __init__(self, client: Vallox, state_proxy: ValloxStateProxy) -> None: + def __init__( + self, client: Vallox, coordinator: DataUpdateCoordinator[ValloxState] + ) -> None: """Initialize the proxy.""" self._client = client - self._state_proxy = state_proxy + self._coordinator = coordinator async def async_set_profile(self, profile: str = "Home") -> bool: """Set the ventilation profile.""" _LOGGER.debug("Setting ventilation profile to: %s", profile) _LOGGER.warning( - "Attention: The service 'vallox.set_profile' is superseded by the 'fan.set_preset_mode' service." - "It will be removed in the future, please migrate to 'fan.set_preset_mode' to prevent breakage" + "Attention: The service 'vallox.set_profile' is superseded by the " + "'fan.set_preset_mode' service. It will be removed in the future, please migrate to " + "'fan.set_preset_mode' to prevent breakage" ) try: @@ -269,4 +268,4 @@ async def async_handle(self, call: ServiceCall) -> None: # This state change affects other entities like sensors. Force an immediate update that can # be observed by all parties involved. if result: - await self._state_proxy.async_update() + await self._coordinator.async_request_refresh() diff --git a/homeassistant/components/vallox/const.py b/homeassistant/components/vallox/const.py index 6a9c4ddc5f495f..6d12d0bab8ed26 100644 --- a/homeassistant/components/vallox/const.py +++ b/homeassistant/components/vallox/const.py @@ -7,8 +7,7 @@ DOMAIN = "vallox" DEFAULT_NAME = "Vallox" -SIGNAL_VALLOX_STATE_UPDATE = "vallox_state_update" -STATE_PROXY_SCAN_INTERVAL = timedelta(seconds=60) +STATE_SCAN_INTERVAL = timedelta(seconds=60) # Common metric keys and (default) values. METRIC_KEY_MODE = "A_CYC_MODE" diff --git a/homeassistant/components/vallox/fan.py b/homeassistant/components/vallox/fan.py index 8ee1b8b471fc31..ac10d1ffb72222 100644 --- a/homeassistant/components/vallox/fan.py +++ b/homeassistant/components/vallox/fan.py @@ -13,12 +13,15 @@ FanEntity, NotValidPresetModeError, ) -from homeassistant.core import HomeAssistant, callback -from homeassistant.helpers.dispatcher import async_dispatcher_connect +from homeassistant.core import HomeAssistant from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType +from homeassistant.helpers.update_coordinator import ( + CoordinatorEntity, + DataUpdateCoordinator, +) -from . import ValloxStateProxy +from . import ValloxState from .const import ( DOMAIN, METRIC_KEY_MODE, @@ -27,7 +30,6 @@ METRIC_KEY_PROFILE_FAN_SPEED_HOME, MODE_OFF, MODE_ON, - SIGNAL_VALLOX_STATE_UPDATE, STR_TO_VALLOX_PROFILE_SETTABLE, VALLOX_PROFILE_TO_STR_SETTABLE, ) @@ -62,31 +64,27 @@ async def async_setup_platform( client.set_settable_address(METRIC_KEY_MODE, int) device = ValloxFan( - hass.data[DOMAIN]["name"], client, hass.data[DOMAIN]["state_proxy"] + hass.data[DOMAIN]["name"], client, hass.data[DOMAIN]["coordinator"] ) async_add_entities([device], update_before_add=False) -class ValloxFan(FanEntity): +class ValloxFan(CoordinatorEntity[ValloxState], FanEntity): """Representation of the fan.""" - _attr_should_poll = False - def __init__( - self, name: str, client: Vallox, state_proxy: ValloxStateProxy + self, + name: str, + client: Vallox, + coordinator: DataUpdateCoordinator[ValloxState], ) -> None: """Initialize the fan.""" + super().__init__(coordinator) + self._client = client - self._state_proxy = state_proxy - self._is_on = False - self._preset_mode: str | None = None - self._fan_speed_home: int | None = None - self._fan_speed_away: int | None = None - self._fan_speed_boost: int | None = None self._attr_name = name - self._attr_available = False @property def supported_features(self) -> int: @@ -102,72 +100,38 @@ def preset_modes(self) -> list[str]: @property def is_on(self) -> bool: """Return if device is on.""" - return self._is_on + return self.coordinator.data.get_metric(METRIC_KEY_MODE) == MODE_ON @property def preset_mode(self) -> str | None: """Return the current preset mode.""" - return self._preset_mode + vallox_profile = self.coordinator.data.profile + return VALLOX_PROFILE_TO_STR_SETTABLE.get(vallox_profile) @property def extra_state_attributes(self) -> Mapping[str, int | None]: """Return device specific state attributes.""" - return { - ATTR_PROFILE_FAN_SPEED_HOME["description"]: self._fan_speed_home, - ATTR_PROFILE_FAN_SPEED_AWAY["description"]: self._fan_speed_away, - ATTR_PROFILE_FAN_SPEED_BOOST["description"]: self._fan_speed_boost, - } - - async def async_added_to_hass(self) -> None: - """Call to update.""" - self.async_on_remove( - async_dispatcher_connect( - self.hass, SIGNAL_VALLOX_STATE_UPDATE, self._update_callback - ) - ) - - @callback - def _update_callback(self) -> None: - """Call update method.""" - self.async_schedule_update_ha_state(True) - - async def async_update(self) -> None: - """Fetch state from the device.""" - try: - # Fetch if the whole device is in regular operation state. - self._is_on = self._state_proxy.fetch_metric(METRIC_KEY_MODE) == MODE_ON - - vallox_profile = self._state_proxy.get_profile() - - # Fetch the profile fan speeds. - fan_speed_home = self._state_proxy.fetch_metric( - ATTR_PROFILE_FAN_SPEED_HOME["metric_key"] - ) - fan_speed_away = self._state_proxy.fetch_metric( - ATTR_PROFILE_FAN_SPEED_AWAY["metric_key"] - ) - fan_speed_boost = self._state_proxy.fetch_metric( - ATTR_PROFILE_FAN_SPEED_BOOST["metric_key"] - ) - - except (OSError, KeyError, TypeError) as err: - self._attr_available = False - _LOGGER.error("Error updating fan: %s", err) - return - - self._preset_mode = VALLOX_PROFILE_TO_STR_SETTABLE.get(vallox_profile) - - self._fan_speed_home = ( - int(fan_speed_home) if isinstance(fan_speed_home, (int, float)) else None + fan_speed_home = self.coordinator.data.get_metric( + ATTR_PROFILE_FAN_SPEED_HOME["metric_key"] ) - self._fan_speed_away = ( - int(fan_speed_away) if isinstance(fan_speed_away, (int, float)) else None + fan_speed_away = self.coordinator.data.get_metric( + ATTR_PROFILE_FAN_SPEED_AWAY["metric_key"] ) - self._fan_speed_boost = ( - int(fan_speed_boost) if isinstance(fan_speed_boost, (int, float)) else None + fan_speed_boost = self.coordinator.data.get_metric( + ATTR_PROFILE_FAN_SPEED_BOOST["metric_key"] ) - self._attr_available = True + return { + ATTR_PROFILE_FAN_SPEED_HOME["description"]: int(fan_speed_home) + if isinstance(fan_speed_home, (int, float)) + else None, + ATTR_PROFILE_FAN_SPEED_AWAY["description"]: int(fan_speed_away) + if isinstance(fan_speed_away, (int, float)) + else None, + ATTR_PROFILE_FAN_SPEED_BOOST["description"]: int(fan_speed_boost) + if isinstance(fan_speed_boost, (int, float)) + else None, + } async def _async_set_preset_mode_internal(self, preset_mode: str) -> bool: """ @@ -201,7 +165,7 @@ async def async_set_preset_mode(self, preset_mode: str) -> None: if update_needed: # This state change affects other entities like sensors. Force an immediate update that # can be observed by all parties involved. - await self._state_proxy.async_update() + await self.coordinator.async_request_refresh() async def async_turn_on( self, @@ -211,7 +175,7 @@ async def async_turn_on( **kwargs: Any, ) -> None: """Turn the device on.""" - _LOGGER.debug("Turn on: %s", speed) + _LOGGER.debug("Turn on") update_needed = False @@ -231,7 +195,7 @@ async def async_turn_on( if update_needed: # This state change affects other entities like sensors. Force an immediate update that # can be observed by all parties involved. - await self._state_proxy.async_update() + await self.coordinator.async_request_refresh() async def async_turn_off(self, **kwargs: Any) -> None: """Turn the device off.""" @@ -246,4 +210,4 @@ async def async_turn_off(self, **kwargs: Any) -> None: return # Same as for turn_on method. - await self._state_proxy.async_update() + await self.coordinator.async_request_refresh() diff --git a/homeassistant/components/vallox/sensor.py b/homeassistant/components/vallox/sensor.py index 7bf9dca700f34f..f3bea8cbe7fd8b 100644 --- a/homeassistant/components/vallox/sensor.py +++ b/homeassistant/components/vallox/sensor.py @@ -19,143 +19,93 @@ PERCENTAGE, TEMP_CELSIUS, ) -from homeassistant.core import HomeAssistant, callback -from homeassistant.helpers.dispatcher import async_dispatcher_connect +from homeassistant.core import HomeAssistant from homeassistant.helpers.entity_platform import AddEntitiesCallback -from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType - -from . import ValloxStateProxy -from .const import ( - DOMAIN, - METRIC_KEY_MODE, - MODE_ON, - SIGNAL_VALLOX_STATE_UPDATE, - VALLOX_PROFILE_TO_STR_REPORTABLE, +from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType, StateType +from homeassistant.helpers.update_coordinator import ( + CoordinatorEntity, + DataUpdateCoordinator, ) +from . import ValloxState +from .const import DOMAIN, METRIC_KEY_MODE, MODE_ON, VALLOX_PROFILE_TO_STR_REPORTABLE + _LOGGER = logging.getLogger(__name__) -class ValloxSensor(SensorEntity): +class ValloxSensor(CoordinatorEntity[ValloxState], SensorEntity): """Representation of a Vallox sensor.""" - _attr_should_poll = False entity_description: ValloxSensorEntityDescription def __init__( self, name: str, - state_proxy: ValloxStateProxy, + coordinator: DataUpdateCoordinator[ValloxState], description: ValloxSensorEntityDescription, ) -> None: """Initialize the Vallox sensor.""" - self._state_proxy = state_proxy + super().__init__(coordinator) self.entity_description = description self._attr_name = f"{name} {description.name}" - self._attr_available = False - - async def async_added_to_hass(self) -> None: - """Call to update.""" - self.async_on_remove( - async_dispatcher_connect( - self.hass, SIGNAL_VALLOX_STATE_UPDATE, self._update_callback - ) - ) - - @callback - def _update_callback(self) -> None: - """Call update method.""" - self.async_schedule_update_ha_state(True) - - async def async_update(self) -> None: - """Fetch state from the ventilation unit.""" + + @property + def native_value(self) -> StateType: + """Return the value reported by the sensor.""" if (metric_key := self.entity_description.metric_key) is None: - self._attr_available = False _LOGGER.error("Error updating sensor. Empty metric key") - return - - try: - self._attr_native_value = self._state_proxy.fetch_metric(metric_key) + return None - except (OSError, KeyError, TypeError) as err: - self._attr_available = False - _LOGGER.error("Error updating sensor: %s", err) - return - - self._attr_available = True + return self.coordinator.data.get_metric(metric_key) class ValloxProfileSensor(ValloxSensor): """Child class for profile reporting.""" - async def async_update(self) -> None: - """Fetch state from the ventilation unit.""" - try: - vallox_profile = self._state_proxy.get_profile() - - except OSError as err: - self._attr_available = False - _LOGGER.error("Error updating sensor: %s", err) - return + @property + def native_value(self) -> StateType: + """Return the value reported by the sensor.""" + vallox_profile = self.coordinator.data.profile + return VALLOX_PROFILE_TO_STR_REPORTABLE.get(vallox_profile) - self._attr_native_value = VALLOX_PROFILE_TO_STR_REPORTABLE.get(vallox_profile) - self._attr_available = True - -# There seems to be a quirk with respect to the fan speed reporting. The device keeps on reporting -# the last valid fan speed from when the device was in regular operation mode, even if it left that -# state and has been shut off in the meantime. +# There is a quirk with respect to the fan speed reporting. The device keeps on reporting the last +# valid fan speed from when the device was in regular operation mode, even if it left that state and +# has been shut off in the meantime. # # Therefore, first query the overall state of the device, and report zero percent fan speed in case # it is not in regular operation mode. class ValloxFanSpeedSensor(ValloxSensor): """Child class for fan speed reporting.""" - async def async_update(self) -> None: - """Fetch state from the ventilation unit.""" - try: - fan_on = self._state_proxy.fetch_metric(METRIC_KEY_MODE) == MODE_ON - - except (OSError, KeyError, TypeError) as err: - self._attr_available = False - _LOGGER.error("Error updating sensor: %s", err) - return - - if fan_on: - await super().async_update() - else: - # Report zero percent otherwise. - self._attr_native_value = 0 - self._attr_available = True + @property + def native_value(self) -> StateType: + """Return the value reported by the sensor.""" + fan_is_on = self.coordinator.data.get_metric(METRIC_KEY_MODE) == MODE_ON + return super().native_value if fan_is_on else 0 class ValloxFilterRemainingSensor(ValloxSensor): """Child class for filter remaining time reporting.""" - async def async_update(self) -> None: - """Fetch state from the ventilation unit.""" - await super().async_update() - - # Check if the update in the super call was a success. - if not self._attr_available: - return + @property + def native_value(self) -> StateType: + """Return the value reported by the sensor.""" + super_native_value = super().native_value - if not isinstance(self._attr_native_value, (int, float)): - self._attr_available = False - _LOGGER.error( - "Value has unexpected type: %s", type(self._attr_native_value) - ) - return + if not isinstance(super_native_value, (int, float)): + _LOGGER.error("Value has unexpected type: %s", type(super_native_value)) + return None # Since only a delta of days is received from the device, fix the time so the timestamp does # not change with every update. - days_remaining = float(self._attr_native_value) + days_remaining = float(super_native_value) days_remaining_delta = timedelta(days=days_remaining) now = datetime.utcnow().replace(hour=13, minute=0, second=0, microsecond=0) - self._attr_native_value = (now + days_remaining_delta).isoformat() + return (now + days_remaining_delta).isoformat() @dataclass @@ -259,11 +209,11 @@ async def async_setup_platform( return name = hass.data[DOMAIN]["name"] - state_proxy = hass.data[DOMAIN]["state_proxy"] + coordinator = hass.data[DOMAIN]["coordinator"] async_add_entities( [ - description.sensor_type(name, state_proxy, description) + description.sensor_type(name, coordinator, description) for description in SENSORS ], update_before_add=False, From 6a3575eb4e7433c796fab3648f5995181ff83768 Mon Sep 17 00:00:00 2001 From: Andre Richter Date: Tue, 12 Oct 2021 07:31:08 +0000 Subject: [PATCH 2/6] Address reviews 1 --- homeassistant/components/vallox/__init__.py | 33 ++++++++++----------- homeassistant/components/vallox/fan.py | 2 +- homeassistant/components/vallox/sensor.py | 3 +- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/homeassistant/components/vallox/__init__.py b/homeassistant/components/vallox/__init__.py index a2c811a3cd123a..0a729ee930881e 100644 --- a/homeassistant/components/vallox/__init__.py +++ b/homeassistant/components/vallox/__init__.py @@ -11,7 +11,7 @@ from vallox_websocket_api.exceptions import ValloxApiException import voluptuous as vol -from homeassistant.const import CONF_HOST, CONF_NAME +from homeassistant.const import CONF_HOST, CONF_NAME, EVENT_HOMEASSISTANT_STARTED from homeassistant.core import HomeAssistant, ServiceCall from homeassistant.helpers import config_validation as cv from homeassistant.helpers.discovery import async_load_platform @@ -146,20 +146,6 @@ async def async_update_data() -> ValloxState: update_method=async_update_data, ) - # The vallox hardware expects quite strict timings for websocket requests. Timings that machines - # with less processing power, like a Raspberry Pi, cannot live up to during the busy start phase - # of Home Asssistant. Hence, "async_add_entities()" for any platform code will be called with - # "update_before_add=False" to intentionally delay the first request, increasing chance that it - # is issued only when the machine is less busy again. - # - # For the same reason, don't call "coordinator.async_refresh()" here. Instead, set - # "coordinator.last_update_success" to "False", so that Home Assistant does not try to update - # the state of entities whose code assumes that "coordinator.data" is never "None". - # - # In summary, this will cause the first websocket requests to the Vallox client to go out after - # one "coordinator.update_interval" has passed. - coordinator.last_update_success = False - service_handler = ValloxServiceHandler(client, coordinator) for vallox_service, method in SERVICE_TO_METHOD.items(): schema = method["schema"] @@ -169,8 +155,21 @@ async def async_update_data() -> ValloxState: hass.data[DOMAIN] = {"client": client, "coordinator": coordinator, "name": name} - hass.async_create_task(async_load_platform(hass, "sensor", DOMAIN, {}, config)) - hass.async_create_task(async_load_platform(hass, "fan", DOMAIN, {}, config)) + async def _async_load_platform_delayed(*_: Any) -> None: + await coordinator.async_refresh() + hass.async_create_task(async_load_platform(hass, "sensor", DOMAIN, {}, config)) + hass.async_create_task(async_load_platform(hass, "fan", DOMAIN, {}, config)) + + # The Vallox hardware expects quite strict timings for websocket requests. Timings that machines + # with less processing power, like a Raspberry Pi, cannot live up to during the busy start phase + # of Home Asssistant. + # + # Hence, wait for the started event before doing a first data refresh and loading the platforms, + # because it usually means the system is less busy after the event and can now meet the + # websocket timing requirements. + hass.bus.async_listen_once( + EVENT_HOMEASSISTANT_STARTED, _async_load_platform_delayed + ) return True diff --git a/homeassistant/components/vallox/fan.py b/homeassistant/components/vallox/fan.py index ac10d1ffb72222..ea169a5813f107 100644 --- a/homeassistant/components/vallox/fan.py +++ b/homeassistant/components/vallox/fan.py @@ -67,7 +67,7 @@ async def async_setup_platform( hass.data[DOMAIN]["name"], client, hass.data[DOMAIN]["coordinator"] ) - async_add_entities([device], update_before_add=False) + async_add_entities([device]) class ValloxFan(CoordinatorEntity[ValloxState], FanEntity): diff --git a/homeassistant/components/vallox/sensor.py b/homeassistant/components/vallox/sensor.py index f3bea8cbe7fd8b..8282a54a2e364e 100644 --- a/homeassistant/components/vallox/sensor.py +++ b/homeassistant/components/vallox/sensor.py @@ -215,6 +215,5 @@ async def async_setup_platform( [ description.sensor_type(name, coordinator, description) for description in SENSORS - ], - update_before_add=False, + ] ) From 562fc99174ad4c52a5303aba4b4f5f684371dcb1 Mon Sep 17 00:00:00 2001 From: Andre Richter Date: Sun, 24 Oct 2021 12:54:30 +0000 Subject: [PATCH 3/6] Address reviews 2: Fan extra state attrs --- homeassistant/components/vallox/fan.py | 62 +++++++++++++------------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/homeassistant/components/vallox/fan.py b/homeassistant/components/vallox/fan.py index ea169a5813f107..3ff66870c7c5f3 100644 --- a/homeassistant/components/vallox/fan.py +++ b/homeassistant/components/vallox/fan.py @@ -3,7 +3,7 @@ from collections.abc import Mapping import logging -from typing import Any +from typing import Any, NamedTuple from vallox_websocket_api import Vallox from vallox_websocket_api.exceptions import ValloxApiException @@ -15,7 +15,7 @@ ) from homeassistant.core import HomeAssistant from homeassistant.helpers.entity_platform import AddEntitiesCallback -from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType +from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType, StateType from homeassistant.helpers.update_coordinator import ( CoordinatorEntity, DataUpdateCoordinator, @@ -36,18 +36,25 @@ _LOGGER = logging.getLogger(__name__) -ATTR_PROFILE_FAN_SPEED_HOME = { - "description": "fan_speed_home", - "metric_key": METRIC_KEY_PROFILE_FAN_SPEED_HOME, -} -ATTR_PROFILE_FAN_SPEED_AWAY = { - "description": "fan_speed_away", - "metric_key": METRIC_KEY_PROFILE_FAN_SPEED_AWAY, -} -ATTR_PROFILE_FAN_SPEED_BOOST = { - "description": "fan_speed_boost", - "metric_key": METRIC_KEY_PROFILE_FAN_SPEED_BOOST, -} + +class ExtraStateAttributeDetails(NamedTuple): + """Extra state attribute properties.""" + + description: str + metric_key: str + + +EXTRA_STATE_ATTRIBUTES = ( + ExtraStateAttributeDetails( + description="fan_speed_home", metric_key=METRIC_KEY_PROFILE_FAN_SPEED_HOME + ), + ExtraStateAttributeDetails( + description="fan_speed_away", metric_key=METRIC_KEY_PROFILE_FAN_SPEED_AWAY + ), + ExtraStateAttributeDetails( + description="fan_speed_boost", metric_key=METRIC_KEY_PROFILE_FAN_SPEED_BOOST + ), +) async def async_setup_platform( @@ -111,26 +118,17 @@ def preset_mode(self) -> str | None: @property def extra_state_attributes(self) -> Mapping[str, int | None]: """Return device specific state attributes.""" - fan_speed_home = self.coordinator.data.get_metric( - ATTR_PROFILE_FAN_SPEED_HOME["metric_key"] - ) - fan_speed_away = self.coordinator.data.get_metric( - ATTR_PROFILE_FAN_SPEED_AWAY["metric_key"] - ) - fan_speed_boost = self.coordinator.data.get_metric( - ATTR_PROFILE_FAN_SPEED_BOOST["metric_key"] - ) + data = self.coordinator.data + + def check_and_convert(value: StateType) -> int | None: + if isinstance(value, (int, float)): + return int(value) + + return None return { - ATTR_PROFILE_FAN_SPEED_HOME["description"]: int(fan_speed_home) - if isinstance(fan_speed_home, (int, float)) - else None, - ATTR_PROFILE_FAN_SPEED_AWAY["description"]: int(fan_speed_away) - if isinstance(fan_speed_away, (int, float)) - else None, - ATTR_PROFILE_FAN_SPEED_BOOST["description"]: int(fan_speed_boost) - if isinstance(fan_speed_boost, (int, float)) - else None, + attr.description: check_and_convert(data.get_metric(attr.metric_key)) + for attr in EXTRA_STATE_ATTRIBUTES } async def _async_set_preset_mode_internal(self, preset_mode: str) -> bool: From 49848f8221993488e22afb236d770db7660f358f Mon Sep 17 00:00:00 2001 From: Andre Richter Date: Sun, 24 Oct 2021 13:02:25 +0000 Subject: [PATCH 4/6] Address Reviews 3: Downgrade spammy logs --- homeassistant/components/vallox/__init__.py | 4 ++-- homeassistant/components/vallox/sensor.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/vallox/__init__.py b/homeassistant/components/vallox/__init__.py index 0a729ee930881e..394bbcb2b2514e 100644 --- a/homeassistant/components/vallox/__init__.py +++ b/homeassistant/components/vallox/__init__.py @@ -101,13 +101,13 @@ def get_metric(self, metric_key: str) -> StateType: _LOGGER.debug("Fetching metric key: %s", metric_key) if metric_key not in vlxDevConstants.__dict__: - _LOGGER.error("Metric key invalid: %s", metric_key) + _LOGGER.debug("Metric key invalid: %s", metric_key) if (value := self.metric_cache.get(metric_key)) is None: return None if not isinstance(value, (str, int, float)): - _LOGGER.error( + _LOGGER.debug( "Return value of metric %s has unexpected type %s", metric_key, type(value), diff --git a/homeassistant/components/vallox/sensor.py b/homeassistant/components/vallox/sensor.py index 8282a54a2e364e..ba8055a307e708 100644 --- a/homeassistant/components/vallox/sensor.py +++ b/homeassistant/components/vallox/sensor.py @@ -55,7 +55,7 @@ def __init__( def native_value(self) -> StateType: """Return the value reported by the sensor.""" if (metric_key := self.entity_description.metric_key) is None: - _LOGGER.error("Error updating sensor. Empty metric key") + _LOGGER.debug("Error updating sensor. Empty metric key") return None return self.coordinator.data.get_metric(metric_key) @@ -96,7 +96,7 @@ def native_value(self) -> StateType: super_native_value = super().native_value if not isinstance(super_native_value, (int, float)): - _LOGGER.error("Value has unexpected type: %s", type(super_native_value)) + _LOGGER.debug("Value has unexpected type: %s", type(super_native_value)) return None # Since only a delta of days is received from the device, fix the time so the timestamp does From a7e0473ee31f3969321d38ee491c07968dda45de Mon Sep 17 00:00:00 2001 From: Andre Richter Date: Sun, 24 Oct 2021 20:13:17 +0000 Subject: [PATCH 5/6] Address reviews 4: Type annotations --- homeassistant/components/vallox/__init__.py | 8 +++++++- homeassistant/components/vallox/fan.py | 13 ++++++------- homeassistant/components/vallox/sensor.py | 12 +++++------- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/homeassistant/components/vallox/__init__.py b/homeassistant/components/vallox/__init__.py index 394bbcb2b2514e..3f441054fb1b95 100644 --- a/homeassistant/components/vallox/__init__.py +++ b/homeassistant/components/vallox/__init__.py @@ -117,6 +117,12 @@ def get_metric(self, metric_key: str) -> StateType: return value +class ValloxDataUpdateCoordinator(DataUpdateCoordinator): + """The DataUpdateCoordinator for Vallox.""" + + data: ValloxState + + async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: """Set up the client and boot the platforms.""" conf = config[DOMAIN] @@ -138,7 +144,7 @@ async def async_update_data() -> ValloxState: return ValloxState(metric_cache, profile) - coordinator = DataUpdateCoordinator[ValloxState]( + coordinator = ValloxDataUpdateCoordinator( hass, _LOGGER, name=f"{name} DataUpdateCoordinator", diff --git a/homeassistant/components/vallox/fan.py b/homeassistant/components/vallox/fan.py index 3ff66870c7c5f3..660386ed04141e 100644 --- a/homeassistant/components/vallox/fan.py +++ b/homeassistant/components/vallox/fan.py @@ -16,12 +16,9 @@ from homeassistant.core import HomeAssistant from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType, StateType -from homeassistant.helpers.update_coordinator import ( - CoordinatorEntity, - DataUpdateCoordinator, -) +from homeassistant.helpers.update_coordinator import CoordinatorEntity -from . import ValloxState +from . import ValloxDataUpdateCoordinator from .const import ( DOMAIN, METRIC_KEY_MODE, @@ -77,14 +74,16 @@ async def async_setup_platform( async_add_entities([device]) -class ValloxFan(CoordinatorEntity[ValloxState], FanEntity): +class ValloxFan(CoordinatorEntity, FanEntity): """Representation of the fan.""" + coordinator: ValloxDataUpdateCoordinator + def __init__( self, name: str, client: Vallox, - coordinator: DataUpdateCoordinator[ValloxState], + coordinator: ValloxDataUpdateCoordinator, ) -> None: """Initialize the fan.""" super().__init__(coordinator) diff --git a/homeassistant/components/vallox/sensor.py b/homeassistant/components/vallox/sensor.py index ba8055a307e708..2fdfd2cd472f68 100644 --- a/homeassistant/components/vallox/sensor.py +++ b/homeassistant/components/vallox/sensor.py @@ -22,26 +22,24 @@ from homeassistant.core import HomeAssistant from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType, StateType -from homeassistant.helpers.update_coordinator import ( - CoordinatorEntity, - DataUpdateCoordinator, -) +from homeassistant.helpers.update_coordinator import CoordinatorEntity -from . import ValloxState +from . import ValloxDataUpdateCoordinator from .const import DOMAIN, METRIC_KEY_MODE, MODE_ON, VALLOX_PROFILE_TO_STR_REPORTABLE _LOGGER = logging.getLogger(__name__) -class ValloxSensor(CoordinatorEntity[ValloxState], SensorEntity): +class ValloxSensor(CoordinatorEntity, SensorEntity): """Representation of a Vallox sensor.""" entity_description: ValloxSensorEntityDescription + coordinator: ValloxDataUpdateCoordinator def __init__( self, name: str, - coordinator: DataUpdateCoordinator[ValloxState], + coordinator: ValloxDataUpdateCoordinator, description: ValloxSensorEntityDescription, ) -> None: """Initialize the Vallox sensor.""" From 4defc8a3e37528db78e16cb23040e569505189be Mon Sep 17 00:00:00 2001 From: Andre Richter Date: Sun, 24 Oct 2021 20:17:26 +0000 Subject: [PATCH 6/6] Address reviews 5: Utility function --- homeassistant/components/vallox/fan.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/vallox/fan.py b/homeassistant/components/vallox/fan.py index 660386ed04141e..39242b01b4ec83 100644 --- a/homeassistant/components/vallox/fan.py +++ b/homeassistant/components/vallox/fan.py @@ -54,6 +54,13 @@ class ExtraStateAttributeDetails(NamedTuple): ) +def _convert_fan_speed_value(value: StateType) -> int | None: + if isinstance(value, (int, float)): + return int(value) + + return None + + async def async_setup_platform( hass: HomeAssistant, config: ConfigType, @@ -119,14 +126,8 @@ def extra_state_attributes(self) -> Mapping[str, int | None]: """Return device specific state attributes.""" data = self.coordinator.data - def check_and_convert(value: StateType) -> int | None: - if isinstance(value, (int, float)): - return int(value) - - return None - return { - attr.description: check_and_convert(data.get_metric(attr.metric_key)) + attr.description: _convert_fan_speed_value(data.get_metric(attr.metric_key)) for attr in EXTRA_STATE_ATTRIBUTES }