From f37c9ec4ef015c582acde74aa205b5d6c5067cb8 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Mon, 27 Jul 2026 14:42:34 +0000 Subject: [PATCH] Fix air-purifier fan power write and silent preset-mode rejection (review follow-up, issue #130) - The fan's power write hardcoded /power/vs/0 while is_on already read /power/0 as a fallback -- a board reporting only the OCF resource would show correct state but silently no-op on every turn-on/off. Mirrors LocalThingsRangeHoodFan's existing _power_payload pattern: target whichever power href the board actually reports. - async_set_preset_mode fell off the loop silently on an unmatched mode with no log and no error, unlike the rest of this codebase's write-rejection handling. Logs a warning now. - Deduplicated HEPA_FILTER's usage-percent calculation, which was an inline reimplementation of airconditioner._filter_usage_percent; promoted the shared logic to common.filter_usage_percent and pointed both families at it. - Gave air_purifier.SOUND_MODE its own translation_key instead of defaulting to the same catalog entry laundry.SOUND_MODE uses. That entry's state table is {voice, tone, mute}; this board's is {mute, buzzer} -- sharing it left 'buzzer' with no label. --- custom_components/localthings/fan.py | 21 ++++++++++++-- .../registry/capabilities/air_purifier.py | 28 ++++++++++++------- .../registry/capabilities/airconditioner.py | 16 ++--------- .../registry/capabilities/common.py | 13 +++++++++ .../localthings/translations/en.json | 7 +++++ .../localthings/translations/nl.json | 7 +++++ 6 files changed, 66 insertions(+), 26 deletions(-) diff --git a/custom_components/localthings/fan.py b/custom_components/localthings/fan.py index ff789e9..15ac111 100644 --- a/custom_components/localthings/fan.py +++ b/custom_components/localthings/fan.py @@ -8,6 +8,8 @@ issue #130) are named behaviors with no linear order (PRESET_MODE).""" from __future__ import annotations +import logging + from homeassistant.components.fan import FanEntity, FanEntityFeature from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant @@ -23,6 +25,7 @@ from .entity import LocalThingsEntity, _is_included from .registry.capabilities.air_purifier import HREF_MODE as AIR_PURIFIER_FAN_HREF from .registry.entities import FanDesc +_LOGGER = logging.getLogger(__name__) POWER_HREF = '/power/0' POWER_VS_HREF = '/power/vs/0' @@ -158,6 +161,16 @@ class LocalThingsAirPurifierFan(LocalThingsEntity, FanEntity): def _mode_rep(self) -> dict: return self._rep(self._bound.href) + def _power_payload(self, enabled: bool) -> tuple[str, bool, str]: + """Target whichever power resource this unit actually exposes -- + same pattern as LocalThingsRangeHoodFan._power_payload above. + Writing a hardcoded href here would silently no-op on a board that + only reports the other one, even though is_on already falls back + correctly.""" + resources = self.coordinator.last_resources + target = POWER_VS_HREF if POWER_VS_HREF in resources else POWER_HREF + return 'power', enabled, target + @property def is_on(self) -> bool: power = self._rep(POWER_VS_HREF).get('x.com.samsung.da.power') @@ -182,12 +195,12 @@ class LocalThingsAirPurifierFan(LocalThingsEntity, FanEntity): self, percentage: int | None = None, preset_mode: str | None = None, **kwargs, ) -> None: - await self.coordinator.async_send_command(self._bound, ('power', True)) + await self.coordinator.async_send_command(self._bound, self._power_payload(True)) if preset_mode is not None: await self.async_set_preset_mode(preset_mode) async def async_turn_off(self, **kwargs) -> None: - await self.coordinator.async_send_command(self._bound, ('power', False)) + await self.coordinator.async_send_command(self._bound, self._power_payload(False)) async def async_set_preset_mode(self, preset_mode: str) -> None: # Reverse-resolve against the unit's own supportedModes -- the @@ -197,3 +210,7 @@ class LocalThingsAirPurifierFan(LocalThingsEntity, FanEntity): if str(code).lower() == preset_mode: await self.coordinator.async_send_command(self._bound, ('mode', code)) return + _LOGGER.warning( + "%s: %r is not a valid preset mode (supported: %s)", + self.entity_id, preset_mode, self.preset_modes, + ) diff --git a/custom_components/localthings/registry/capabilities/air_purifier.py b/custom_components/localthings/registry/capabilities/air_purifier.py index 6050157..5e867d6 100644 --- a/custom_components/localthings/registry/capabilities/air_purifier.py +++ b/custom_components/localthings/registry/capabilities/air_purifier.py @@ -52,7 +52,7 @@ from ..capability import Capability from ..entities import ( BinarySensorDesc, FanDesc, NumberDesc, SelectDesc, SensorDesc, SwitchDesc, ) -from .common import int_or_none, sensor_item_value +from .common import filter_usage_percent, int_or_none, sensor_item_value from .laundry import bool_option_exists, bool_option_value, option_value, option_write # Newer TP1X_DA-AC-AIR-class boards (e.g. TP1X_DA-AC-AIR-01031_0000, issue @@ -190,8 +190,15 @@ MODE = Capability( def _fan_write(payload, rep, href=None): - kind, value = payload + kind, value, *args = payload if kind == 'power': + # Targets whichever power href fan.py's _power_payload picked (the + # board may only report /power/0) -- a hardcoded vendor href here + # would silently no-op on such a board even though the entity's + # own is_on already falls back to reading it correctly. + power_href = args[0] if args else '/power/vs/0' + if power_href == '/power/0': + return ['power', '0'], {'value': bool(value)} return (['power', 'vs', '0'], {'x.com.samsung.da.power': 'On' if value else 'Off'}) if kind == 'mode': @@ -255,13 +262,8 @@ HEPA_FILTER = Capability( href='/filter/hepafilter/vs/0', poll_tier='cold', entities=( - SensorDesc(key='hepa_filter_usage', rep_fn=lambda rep: ( - round(int_or_none(rep.get('x.com.samsung.da.filterUsage')) / - int_or_none(rep.get('x.com.samsung.da.filterCapacity')) * 100) - if int_or_none(rep.get('x.com.samsung.da.filterUsage')) is not None - and int_or_none(rep.get('x.com.samsung.da.filterCapacity')) - else None - ), unit='%', state_class='measurement', + SensorDesc(key='hepa_filter_usage', rep_fn=filter_usage_percent, + unit='%', state_class='measurement', icon='mdi:air-filter', entity_category='diagnostic'), SensorDesc(key='hepa_filter_status', field='x.com.samsung.da.filterStatus', device_class='enum', @@ -312,7 +314,13 @@ SOUND_MODE = Capability( href='/settings/sound/mode/vs/0', poll_tier='cold', entities=( - SelectDesc(key='sound_mode', field='mode', + # Distinct translation_key from laundry.SOUND_MODE's shared + # 'sound_mode' catalog entry -- that one's state table is + # {voice, tone, mute}, but this board's supportedModes is + # {mute, buzzer}. Sharing the key would leave 'buzzer' unlabelled + # (falls through to the raw code) since the catalogs don't overlap. + SelectDesc(key='sound_mode', translation_key='air_purifier_sound_mode', + field='mode', icon='mdi:volume-high', entity_category='config', options_field='supportedModes', diff --git a/custom_components/localthings/registry/capabilities/airconditioner.py b/custom_components/localthings/registry/capabilities/airconditioner.py index c8e9576..91f0466 100644 --- a/custom_components/localthings/registry/capabilities/airconditioner.py +++ b/custom_components/localthings/registry/capabilities/airconditioner.py @@ -16,7 +16,7 @@ by_type registry. """ from ..capability import Capability from ..entities import BinarySensorDesc, ClimateDesc, SensorDesc, SwitchDesc -from .common import normalize_temp_unit +from .common import filter_usage_percent, normalize_temp_unit from .laundry import option_write # --------------------------------------------------------------------------- @@ -77,18 +77,6 @@ def _temps_vs_unit(rep): return normalize_temp_unit(_temps_vs_item(rep).get('x.com.samsung.da.unit'), '°C') -def _filter_usage_percent(rep): - """Filter usage as a percentage of rated capacity. The device reports - `filterUsage` as a raw count in `filterCapacityUnit` (Hours here, e.g. - 100 of a 500 capacity), so a plain value with a '%' unit would be wrong -- - normalize to used/capacity. Returns None when capacity is missing/zero.""" - used = _num(rep.get('x.com.samsung.da.filterUsage')) - cap = _num(rep.get('x.com.samsung.da.filterCapacity')) - if used is None or not cap: - return None - return round(used / cap * 100) - - def _first_mode(rep): """Representative scalar for the climate entity in the flattened state (golden/regression). The real entity computes hvac_mode from power + mode.""" @@ -251,7 +239,7 @@ AIR_FILTER = Capability( href='/filter/airdustfilter/vs/0', poll_tier='cold', entities=( - SensorDesc(key='air_filter_usage', rep_fn=_filter_usage_percent, + SensorDesc(key='air_filter_usage', rep_fn=filter_usage_percent, unit='%', state_class='measurement', icon='mdi:air-filter', entity_category='diagnostic'), SensorDesc(key='air_filter_status', field='x.com.samsung.da.filterStatus', diff --git a/custom_components/localthings/registry/capabilities/common.py b/custom_components/localthings/registry/capabilities/common.py index 2b6dc59..9d519ff 100644 --- a/custom_components/localthings/registry/capabilities/common.py +++ b/custom_components/localthings/registry/capabilities/common.py @@ -40,6 +40,19 @@ def wh_to_kwh(v): return round(n / 1000.0, 2) if n is not None else None +def filter_usage_percent(rep): + """Filter usage as a percentage of rated capacity. Several families + (AC, air purifier) report `filterUsage` as a raw count in + `filterCapacityUnit` (Hours, e.g. 100 of a 500 capacity), so a plain + value with a '%' unit would be wrong -- normalize to used/capacity. + Returns None when capacity is missing/zero.""" + used = _num(rep.get('x.com.samsung.da.filterUsage')) + cap = _num(rep.get('x.com.samsung.da.filterCapacity')) + if used is None or not cap: + return None + return round(used / cap * 100) + + def normalize_temp_unit(raw, default='°F'): """'C'/'Celsius' -> '°C', 'F'/'Fahrenheit' -> '°F'. Falls back to `default` for any other/missing value. Shared by fridge.py and oven.py, diff --git a/custom_components/localthings/translations/en.json b/custom_components/localthings/translations/en.json index 8d3b5d5..2b5c321 100644 --- a/custom_components/localthings/translations/en.json +++ b/custom_components/localthings/translations/en.json @@ -413,6 +413,13 @@ "mute": "Mute" } }, + "air_purifier_sound_mode": { + "name": "Sound mode", + "state": { + "mute": "Mute", + "buzzer": "Buzzer" + } + }, "spin_speed": { "name": "Spin speed", "state": { diff --git a/custom_components/localthings/translations/nl.json b/custom_components/localthings/translations/nl.json index 7ead2ea..9699b1f 100644 --- a/custom_components/localthings/translations/nl.json +++ b/custom_components/localthings/translations/nl.json @@ -413,6 +413,13 @@ "mute": "Dempen" } }, + "air_purifier_sound_mode": { + "name": "Geluidsmodus", + "state": { + "mute": "Dempen", + "buzzer": "Zoemer" + } + }, "spin_speed": { "name": "Centrifugesnelheid", "state": {