Merge pull request #139 from mbillow/claude/air-purifier-fan-controls-ananzq

Add real fan-speed control for ARTIK051_TVTL air purifiers (issue #56)

custom_components/localthings/fan.py

@@ -1,10 +1,15 @@
 """Fan platform for Samsung range hoods and air purifiers.
 
-Two FanDesc-bound hrefs exist, one per family, dispatched by href in
-async_setup_entry below since they need different HA fan semantics: the
-range hood's fan speed is an ordered set of numeric levels (SET_SPEED), while
-the newer air-purifier board family's modes (Smart/Max/Mid/WindFree/Sleep,
-issue #130) are named behaviors with no linear order (PRESET_MODE)."""
+Three FanDesc-bound hrefs exist, dispatched by href in async_setup_entry
+below since each needs different HA fan semantics: the range hood's fan
+speed and the older ARTIK051_TVTL air-purifier family's Auto/Sleep/Low/
+Medium/High (issue #56) are both an ordered set of numeric levels
+(SET_SPEED) -- the latter confirmed monotonic in capabilities/
+air_purifier.py's module docstring, with no named-mode list to preserve
+since this board never self-reports one. The newer TP1X air-purifier
+family's modes (Smart/Max/Mid/WindFree/Sleep, issue #130) are named
+behaviors with no linear order (PRESET_MODE), reported directly by that
+board's own supportedModes."""
 
 from __future__ import annotations
 
@@ -22,6 +27,7 @@ from homeassistant.util.percentage import (
 from .const import DOMAIN
 from .coordinator import LocalThingsCoordinator
 from .entity import LocalThingsEntity, _is_included
+from .registry.capabilities.air_purifier import HREF_AIRFLOW
 from .registry.capabilities.air_purifier import HREF_MODE as AIR_PURIFIER_FAN_HREF
 from .registry.entities import FanDesc
 
@@ -48,6 +54,8 @@ async def async_setup_entry(
             continue
         if bound.href == AIR_PURIFIER_FAN_HREF:
             entities.append(LocalThingsAirPurifierFan(coordinator, bound))
+        elif bound.href == HREF_AIRFLOW:
+            entities.append(LocalThingsAirflowFan(coordinator, bound))
         else:
             entities.append(LocalThingsRangeHoodFan(coordinator, bound))
     async_add_entities(entities)
@@ -214,3 +222,87 @@ class LocalThingsAirPurifierFan(LocalThingsEntity, FanEntity):
             "%s: %r is not a valid preset mode (supported: %s)",
             self.entity_id, preset_mode, self.preset_modes,
         )
+
+
+_AIRFLOW_SPEED_FIELD = 'speed'
+# Raw `speed` codes, low-to-high -- confirmed monotonic (Auto=0, Sleep=1,
+# Low=2, Medium=3, High=4) via air_purifier.py's module docstring. Ordered
+# as plain strings, same as _all_speed_codes above, so
+# ordered_list_item_to_percentage/percentage_to_ordered_list_item can treat
+# it exactly like the range hood's numeric levels -- no named-preset table
+# needed since this board never reports mode names to hang one off of.
+_AIRFLOW_SPEED_CODES = ('0', '1', '2', '3', '4')
+
+
+class LocalThingsAirflowFan(LocalThingsEntity, FanEntity):
+    """ARTIK051_TVTL-class air purifier fan (issue #56): an ordered numeric
+    speed range, same SET_SPEED shape as LocalThingsRangeHoodFan above."""
+
+    _enable_turn_on_off_backwards_compatibility = False
+    _attr_supported_features = (
+        FanEntityFeature.SET_SPEED
+        | FanEntityFeature.TURN_ON
+        | FanEntityFeature.TURN_OFF
+    )
+
+    def __init__(self, coordinator: LocalThingsCoordinator, bound) -> None:
+        super().__init__(coordinator, bound)
+        self._attr_name = None
+
+    def _rep(self, href: str) -> dict:
+        return self.coordinator.resource(href) or {}
+
+    def _power_payload(self, enabled: bool) -> tuple[str, bool, str]:
+        """Prefer /power/0 like LocalThingsRangeHoodFan above, NOT
+        LocalThingsAirPurifierFan's vs/0-first order -- that order is only
+        harmless for the TP1X board because it never reports /power/0 at
+        all. This family's dumps carry both hrefs, and common.POWER_GENERIC
+        (the power_switch entity) is unconditionally bound to /power/0
+        whenever it's present, so writing here to /power/vs/0 first would
+        leave power_switch and this fan reading/writing two different
+        resources -- disagreeing until the next poll refreshes the other
+        one (the same optimistic-apply lag coordinator.py's own comments
+        warn about)."""
+        resources = self.coordinator.last_resources
+        target = POWER_HREF if POWER_HREF in resources else POWER_VS_HREF
+        return 'power', enabled, target
+
+    @property
+    def is_on(self) -> bool:
+        power = self._rep(POWER_HREF)
+        if 'value' in power:
+            return bool(power.get('value'))
+        return str(self._rep(POWER_VS_HREF).get('x.com.samsung.da.power', '')).lower() == 'on'
+
+    @property
+    def speed_count(self) -> int:
+        return len(_AIRFLOW_SPEED_CODES)
+
+    @property
+    def percentage(self) -> int | None:
+        if not self.is_on:
+            return 0
+        current = str(self._rep(self._bound.href).get(_AIRFLOW_SPEED_FIELD, ''))
+        if current not in _AIRFLOW_SPEED_CODES:
+            return None
+        return ordered_list_item_to_percentage(_AIRFLOW_SPEED_CODES, current)
+
+    async def async_turn_on(
+        self, percentage: int | None = None, preset_mode: str | None = None,
+        **kwargs,
+    ) -> None:
+        await self.coordinator.async_send_command(self._bound, self._power_payload(True))
+        if percentage is not None:
+            await self.async_set_percentage(percentage)
+
+    async def async_turn_off(self, **kwargs) -> None:
+        await self.coordinator.async_send_command(self._bound, self._power_payload(False))
+
+    async def async_set_percentage(self, percentage: int) -> None:
+        if percentage <= 0:
+            await self.async_turn_off()
+            return
+        if not self.is_on:
+            await self.coordinator.async_send_command(self._bound, self._power_payload(True))
+        code = percentage_to_ordered_list_item(_AIRFLOW_SPEED_CODES, percentage)
+        await self.coordinator.async_send_command(self._bound, ('speed', int(code)))

custom_components/localthings/registry/capabilities/air_purifier.py

@@ -39,14 +39,23 @@ set to Auto/Sleep/Low/Medium/High):
                             automatic side effect of sleep mode, e.g. a
                             display-dimming level, but that's still a guess).
 
-/airflow/0 and /airflow/vs/0's `speed` still isn't modeled as a real
-fan-speed control: across the same five dumps it read 0 for both Auto *and*
-High, and 3 for Low/Medium *and* Sleep -- not a monotonic mapping to any
-selectable level, and the dumps were all captured within about three minutes
-of each other (only one poll cycle apart at this integration's 30s summary
-interval), so the values may not have settled after each change before the
-diagnostics snapshot was taken. Exposed read-only pending a confirmed,
-stable capture -- see the issue #56 discussion for what's needed.
+/airflow/0's `speed` is now a real fan-speed control (issue #56 follow-up).
+The first round of five dumps above wasn't conclusive -- it read 0 for both
+Auto *and* High, and 3 for Low/Medium *and* Sleep, likely because all five
+were captured within about a minute of each other, faster than this
+integration's own ~30s poll cycle could settle each change. A second round,
+captured 60-90s apart per setting on two independent units, confirmed a
+clean monotonic mapping instead: Auto=0, Sleep=1, Low=2, Medium=3, High=4.
+AIRFLOW_GENERIC below builds an ordered-speed fan off that confirmed 0-4
+range -- same SET_SPEED shape as range_hood.py's fan, mapping HA's
+percentage steps straight onto the raw code, no named-preset table needed
+(unlike the TP1X family's FAN, which exposes real named modes because its
+board actually reports a supportedModes list to hang names off of).
+
+/airflow/vs/0's vendor `speedLevel` is NOT used for the same purpose -- it
+was unreliable on both units in that second round (Low/Medium collided on
+one unit, stuck at 0 throughout on the other), so AIRFLOW_VS_FALLBACK below
+stays a plain read-only diagnostic even after this change.
 """
 from ..capability import Capability
 from ..entities import (
@@ -64,6 +73,7 @@ from .laundry import bool_option_exists, bool_option_value, option_value, option
 # Both board generations share the /mode/vs/0 href, so FAN and MODE below
 # are mutually exclusive via this presence check rather than colliding.
 HREF_MODE = '/mode/vs/0'
+HREF_AIRFLOW = '/airflow/0'
 
 
 def _has_top_level_modes(rep, resources):
@@ -126,21 +136,52 @@ DEVICE_ACTIVE = Capability(
     ),
 )
 
-# OCF-native / vendor pair for fan speed+direction -- see module docstring for
-# why these are read-only for now.
+def _airflow_fan_write(payload, rep, href=None):
+    kind, value, *args = payload
+    if kind == 'power':
+        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 == 'speed':
+        return ['airflow', '0'], {'speed': int(value)}
+    return None
+
+
+# Confirmed via issue #56's second, properly-spaced round of diagnostics
+# (two independent units, 60-90s apart per setting): /airflow/0's `speed` is
+# a clean, monotonic 0-4 code across Auto/Sleep/Low/Medium/High, so it now
+# backs a real ordered-speed fan (fan.py's LocalThingsAirflowFan, same
+# SET_SPEED shape as the range hood's) instead of a read-only sensor --
+# no named-preset table needed, since HA's percentage steps map onto the
+# raw 0-4 code directly, the same way the range hood's numeric levels do.
+# `direction` stays a plain diagnostic: every dump seen (both rounds, both
+# units) reads 'Off' for it regardless of fan setting, so there's nothing
+# confirmed to control there yet.
+#
+# Keyed 'airflow_fan', not 'fan' -- FAN below (bound to the shared
+# /mode/vs/0 href) also uses 'fan', and BoundEntity's unique_id is built
+# from key alone (entity.py's _key), not href. FAN and AIRFLOW_GENERIC are
+# only *empirically* mutually exclusive (every dump seen has one board
+# generation's shape or the other, never both), not architecturally
+# enforced the way same-href caps are by _build()'s match_fn check -- a
+# same key would collide if a future board ever reported both.
 AIRFLOW_GENERIC = Capability(
-    href='/airflow/0',
+    href=HREF_AIRFLOW,
     poll_tier='warm',
     entities=(
-        SensorDesc(key='fan_speed_level', field='speed',
-                   icon='mdi:fan',
-                   state_class='measurement', entity_category='diagnostic'),
+        FanDesc(key='airflow_fan', field='speed', write_fn=_airflow_fan_write),
         SensorDesc(key='fan_direction', field='direction',
                    icon='mdi:rotate-3d-variant',
                    entity_category='diagnostic'),
     ),
 )
 
+# Left exactly as a read-only fallback -- speedLevel is NOT the same
+# confirmed-reliable field as /airflow/0's speed above (see module
+# docstring): it collided Low/Medium on one unit and stuck at 0 throughout
+# on the other in the same properly-spaced round.
 AIRFLOW_VS_FALLBACK = Capability(
     href='/airflow/vs/0',
     match_fn=lambda rep, resources: '/airflow/0' not in resources,

tests/fixtures/golden/air_purifier.json

@@ -1,5 +1,6 @@
 {
   "state_keys": [
+    "airflow_fan",
     "alarm_code",
     "clean_level",
     "device_active",
@@ -11,7 +12,6 @@
     "energy_saved_kwh",
     "energy_this_month_kwh",
     "fan_direction",
-    "fan_speed_level",
     "filter_progress",
     "fine_dust",
     "odor",

tests/test_air_purifier_airflow_fan.py

@@ -0,0 +1,105 @@
+"""HA fan-entity mapping tests for the ARTIK051_TVTL air purifier (issue #56)."""
+
+from custom_components.localthings.fan import LocalThingsAirflowFan
+from custom_components.localthings.registry.by_type import air_purifier
+from custom_components.localthings.registry.capabilities.air_purifier import HREF_AIRFLOW
+from custom_components.localthings.registry.discovery import discover
+from custom_components.localthings.registry.entities import FanDesc
+from tests.conftest import _load_device
+
+
+class _FakeCoordinator:
+    device_serial = 'TEST-AIRFLOW-SERIAL'
+    device_info = {}
+    data = {}
+
+    def __init__(self, resources):
+        self.last_resources = resources
+        self.commands = []
+
+    def resource(self, href):
+        return self.last_resources.get(href, {})
+
+    async def async_send_command(self, bound, payload):
+        self.commands.append((bound, payload))
+
+
+def _entity(resources, coordinator=None):
+    bound = discover(
+        resources,
+        air_purifier.REGISTRY.capabilities,
+        air_purifier.REGISTRY.pattern_capabilities,
+    )
+    fan_bound = next(
+        item for item in bound
+        if isinstance(item.desc, FanDesc) and item.href == HREF_AIRFLOW
+    )
+    return LocalThingsAirflowFan(
+        coordinator or _FakeCoordinator(resources), fan_bound,
+    )
+
+
+def test_power_off_maps_to_zero_percent_and_five_retained_speeds():
+    entity = _entity(_load_device('air_purifier'))
+    assert entity.is_on is False
+    assert entity.speed_count == 5
+    assert entity.percentage == 0
+
+
+def test_active_codes_map_to_ordered_percentages():
+    """Confirmed via issue #56's second, properly-spaced diagnostics round:
+    /airflow/0's speed is a clean 0-4 code across Auto/Sleep/Low/Medium/High."""
+    resources = _load_device('air_purifier')
+    resources['/power/0']['value'] = True
+
+    for code, expected_percentage in ((0, 20), (1, 40), (2, 60), (3, 80), (4, 100)):
+        resources['/airflow/0']['speed'] = code
+        assert _entity(resources).percentage == expected_percentage
+
+
+async def test_power_write_prefers_standard_resource_when_both_exist():
+    """Both /power/0 and /power/vs/0 are present on this family's dumps --
+    /power/0 must win, matching common.POWER_GENERIC's own preference (the
+    power_switch entity is unconditionally bound to /power/0 whenever it's
+    present, so writing here to /power/vs/0 first would leave the two
+    entities disagreeing until the next poll)."""
+    resources = _load_device('air_purifier')
+    coordinator = _FakeCoordinator(resources)
+    entity = _entity(resources, coordinator)
+
+    await entity.async_turn_on()
+
+    assert coordinator.commands[-1][1] == ('power', True, '/power/0')
+
+
+async def test_power_write_falls_back_to_vendor_resource():
+    resources = _load_device('air_purifier')
+    resources.pop('/power/0')
+    coordinator = _FakeCoordinator(resources)
+    entity = _entity(resources, coordinator)
+
+    await entity.async_turn_off()
+
+    assert coordinator.commands[-1][1] == ('power', False, '/power/vs/0')
+
+
+async def test_set_percentage_writes_raw_speed_code():
+    resources = _load_device('air_purifier')
+    resources['/power/0']['value'] = True
+    coordinator = _FakeCoordinator(resources)
+    entity = _entity(resources, coordinator)
+
+    await entity.async_set_percentage(60)
+
+    assert coordinator.commands[-1][1] == ('speed', 2)
+
+
+async def test_set_percentage_zero_turns_off():
+    resources = _load_device('air_purifier')
+    resources['/power/0']['value'] = True
+    coordinator = _FakeCoordinator(resources)
+    entity = _entity(resources, coordinator)
+
+    await entity.async_set_percentage(0)
+
+    assert coordinator.commands[-1][1] == ('power', False, '/power/0')

tests/test_air_purifier_capabilities.py

@@ -41,7 +41,7 @@ def test_expected_entities_present():
     for key in (
         'power_switch', 'alarm_code', 'dust', 'fine_dust', 'super_fine_dust',
         'odor', 'clean_level', 'filter_progress', 'device_active',
-        'diagnosis_status', 'fan_speed_level', 'fan_direction',
+        'diagnosis_status', 'airflow_fan', 'fan_direction',
         'display_light', 'operating_mode',
     ):
         assert key in state, key
@@ -114,3 +114,27 @@ def test_airflow_vs_fallback_only_binds_without_generic():
         {}, {'/airflow/0': {'speed': 0, 'direction': 'Off'}},
     ) is False
     assert air_purifier.AIRFLOW_VS_FALLBACK.match_fn({}, {}) is True
+
+
+def test_airflow_fan_write_contract():
+    """Confirmed via issue #56's second, properly-spaced diagnostics round
+    (two independent units, 60-90s apart per setting): /airflow/0's `speed`
+    is a clean, monotonic 0-4 code, so the write is a plain int passthrough
+    -- no named-preset table needed (see fan.py's LocalThingsAirflowFan)."""
+    fan_desc = next(e for e in air_purifier.AIRFLOW_GENERIC.entities if e.key == 'airflow_fan')
+    assert fan_desc.write_fn(('speed', 3), {}) == (['airflow', '0'], {'speed': 3})
+    assert fan_desc.write_fn(('power', True, '/power/vs/0'), {}) == (
+        ['power', 'vs', '0'], {'x.com.samsung.da.power': 'On'},
+    )
+    assert fan_desc.write_fn(('power', False, '/power/0'), {}) == (
+        ['power', '0'], {'value': False},
+    )
+
+
+def test_airflow_speed_reads_from_dump():
+    """The fixture's /airflow/0 (power-off snapshot) reads speed=0, and
+    fan_direction stays a plain diagnostic (every dump seen reads 'Off'
+    regardless of fan setting)."""
+    state = _state()
+    assert state['airflow_fan'] == 0
+    assert state['fan_direction'] == 'Off'

tests/test_translations.py

@@ -126,8 +126,10 @@ def test_no_catalog_carries_unresolved_core_references():
 
 
 # The hood fan is its device's primary feature: fan.py sets _attr_name = None
-# so it presents as the device itself, and never reads a catalog name.
-UNNAMED_DESCRIPTORS = {("fan", "fan")}
+# so it presents as the device itself, and never reads a catalog name. Same
+# for the ARTIK051 air-purifier's airflow_fan (issue #56) -- ordered speed
+# levels, no presets, same _attr_name = None treatment.
+UNNAMED_DESCRIPTORS = {("fan", "fan"), ("fan", "airflow_fan")}
 
 
 def test_every_descriptor_has_an_entity_catalog_entry():