Merge pull request #346 from mbillow/issue-345-progress-finished-hold

washer/dryer: hold progress/progress_percentage at Finish/100 for 5 min

custom_components/localthings/registry/capabilities/operational.py

@@ -35,12 +35,44 @@ def _int(v):
         return None
 
 
+def _state_is_active(rep):
+    return _SAMSUNG_STATE_TO_OCF.get(rep.get("x.com.samsung.da.state")) == "active"
+
+
 def _is_active(rep):
     """Check if appliance is actively running and cycle is not finished."""
-    return (
+    return _state_is_active(rep) and rep.get("x.com.samsung.da.progress") != "Finish"
-        _SAMSUNG_STATE_TO_OCF.get(rep.get("x.com.samsung.da.state")) == "active"
+
-        and rep.get("x.com.samsung.da.progress") != "Finish"
+
-    )
+def _just_finished(rep):
+    """progress/progress_percentage's sticky_fn (issue #345, see sensor.py's
+    _apply_sticky): arms their grace window the moment `progress` reads
+    'Finish'.
+
+    Deliberately not also requiring `state == 'active'` in the same rep,
+    unlike _is_active above: #345 reports a washer whose `state` can
+    already read idle by the time `progress` is observed at 'Finish' --
+    the same-family dryer's firmware apparently keeps `state` at 'active'
+    longer, per the report -- so requiring both together risked the arm
+    condition never actually firing on the one device this fixes.
+    _apply_sticky's edge-triggering (only a fresh False->True transition
+    (re)arms) is what keeps a `progress` stuck at 'Finish' indefinitely
+    (the same class of quirk `_completion_minutes` below already works
+    around) from holding this open forever instead."""
+    return rep.get("x.com.samsung.da.progress") == "Finish"
+
+
+def _live_progress_code(rep):
+    """progress/progress_percentage's sticky_bypass_fn: a concrete,
+    non-Finish progress code being reported right now -- e.g. a new
+    cycle's own real 'Wash'/'Spin' -- must win over a still-open hold from
+    the previous cycle immediately. Not keyed on `state` (unlike
+    _is_active): _just_finished's whole premise is that `state` can't be
+    trusted to still say 'active' while a fresh, real progress value is
+    already there, and the same applies to recognizing when it's moved on
+    to a new one -- including while paused, e.g. adding a sock mid-hold."""
+    v = rep.get("x.com.samsung.da.progress")
+    return v is not None and v not in ("None", "Finish")
 
 
 def _remaining_seconds(raw):
@@ -153,19 +185,31 @@ OPERATIONAL_STATE = Capability(
         BinarySensorDesc(
             key="cycle_active",
             device_class="running",
-            rep_fn=lambda rep: (
+            rep_fn=_is_active,
-                _SAMSUNG_STATE_TO_OCF.get(rep.get("x.com.samsung.da.state")) == "active"
-                and rep.get("x.com.samsung.da.progress") != "Finish"
-            ),
         ),
+        # sticky_* (issue #345): once progress reads 'Finish', keep
+        # showing Finish/100 for a grace window even after machine_state
+        # reverts, rather than falling to Idle/0 the instant it does --
+        # see sensor.py's _apply_sticky. rep_fn below is otherwise
+        # unchanged; the hold is entirely a read-side, per-entity concern,
+        # deliberately not gated on machine_state (_just_finished's
+        # docstring explains why). sticky_live_fn reads the raw field the
+        # same ungated way, for sticky_bypass_fn's benefit: a real
+        # progress value reported while paused (e.g. adding a sock
+        # mid-cycle) must win over a stale hold even though rep_fn itself
+        # would show "Idle"/0 there.
         SensorDesc(
             key="progress",
             icon="mdi:progress-wrench",
             rep_fn=lambda rep: (
                 "Idle"
-                if _SAMSUNG_STATE_TO_OCF.get(rep.get("x.com.samsung.da.state")) != "active"
+                if not _state_is_active(rep)
                 else _progress(rep.get("x.com.samsung.da.progress"))
             ),
+            sticky_fn=_just_finished,
+            sticky_value_fn=lambda rep: "Finish",
+            sticky_live_fn=lambda rep: _progress(rep.get("x.com.samsung.da.progress")),
+            sticky_bypass_fn=_live_progress_code,
         ),
         SensorDesc(
             key="progress_percentage",
@@ -173,9 +217,13 @@ OPERATIONAL_STATE = Capability(
             state_class="measurement",
             rep_fn=lambda rep: (
                 0
-                if _SAMSUNG_STATE_TO_OCF.get(rep.get("x.com.samsung.da.state")) != "active"
+                if not _state_is_active(rep)
                 else _int(rep.get("x.com.samsung.da.progressPercentage"))
             ),
+            sticky_fn=_just_finished,
+            sticky_value_fn=lambda rep: 100,
+            sticky_live_fn=lambda rep: _int(rep.get("x.com.samsung.da.progressPercentage")) or 0,
+            sticky_bypass_fn=_live_progress_code,
         ),
         # Only show finish time while actively running -- firmware leaves a
         # stale remainingTime after a cycle ends, frozen at '00:01:00'.

custom_components/localthings/registry/entities.py

@@ -64,6 +64,18 @@ class SensorDesc(SamsungEntityDescription):
     # (see sensor.py). Only for values expected to jitter between
     # device-side revisions -- not a general-purpose flag.
     hysteresis: bool = False
+    # Opt-in, entity-instance-only hold -- see sensor.py's _apply_sticky
+    # for the full contract (arm/value/live/bypass semantics,
+    # edge-triggering, why this never touches the coordinator cache).
+    # sticky_fn arms it; sticky_value_fn picks what to freeze at that
+    # moment (defaults to rep_fn's own result); sticky_bypass_fn forces
+    # sticky_live_fn's result through and drops the hold; sticky_seconds
+    # bounds how long it can hold.
+    sticky_fn: Callable[[dict], bool] | None = None
+    sticky_value_fn: Callable[[dict], Any] | None = None
+    sticky_live_fn: Callable[[dict], Any] | None = None
+    sticky_bypass_fn: Callable[[dict], bool] | None = None
+    sticky_seconds: float = 300.0
 
 
 @dataclass(frozen=True, kw_only=True)

custom_components/localthings/sensor.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import time
 from datetime import timedelta
 from typing import cast
 
@@ -51,6 +52,9 @@ class LocalThingsSensor(LocalThingsEntity, SensorEntity):
         if desc.options:
             self._attr_options = list(desc.options)
         self._hysteresis_value = None
+        self._sticky_value = None
+        self._sticky_until: float | None = None
+        self._sticky_armed = False
 
     @property
     def native_unit_of_measurement(self):
@@ -63,10 +67,68 @@ class LocalThingsSensor(LocalThingsEntity, SensorEntity):
     def native_value(self):
         raw = (self.coordinator.data or {}).get(self._state_key)
         desc = cast(SensorDesc, self._bound.desc)
+        if desc.sticky_fn is not None:
+            raw = self._apply_sticky(raw, desc)
         if not desc.hysteresis:
             return raw
         return self._apply_hysteresis(raw)
 
+    def _apply_sticky(self, raw, desc: SensorDesc):
+        """Freeze this entity at a value for up to `desc.sticky_seconds`
+        after `desc.sticky_fn` next stops matching this href's live rep
+        (issue #345 -- see operational.py's `_just_finished` for the
+        motivating case). Entity-instance state only, exactly like
+        `_hysteresis_value` above -- never written back to the coordinator
+        cache, so write_fn, diagnostics, and the observe-mode sweep
+        comparison keep seeing real device data throughout.
+
+        Reads `sticky_fn` and friends against this href's own live rep,
+        not the already-computed `raw`, so they can be independent of
+        whatever rep_fn itself gates on -- notably `sticky_live_fn`, which
+        is *not* `raw`: `raw` is rep_fn's own (possibly differently gated)
+        result, e.g. progress's rep_fn shows "Idle" while paused, but a
+        real progress value reported while paused (adding a sock mid-
+        cycle) must still win over a stale hold, which means reading it
+        ungated here rather than through that gate.
+
+        Edge-triggered, not level-triggered: the window only (re)starts on
+        a fresh False->True transition of `sticky_fn`, and -- this is the
+        part level-triggering alone misses -- expiry is still checked on
+        every call even while `sticky_fn` keeps matching. Without the
+        latter, a firmware that leaves the underlying field stuck matching
+        forever (the same class of quirk `_completion_minutes` already
+        works around) would show the frozen value forever too, defeating
+        "bounded, not indefinite".
+
+        `sticky_bypass_fn`, when it matches, always passes
+        `sticky_live_fn`'s result through and drops any hold -- for a
+        condition where "not sticky right now" is ambiguous between "went
+        idle, honor the hold" and "genuinely live, different data" (e.g. a
+        new cycle's own real progress), which "consult the hold whenever
+        sticky_fn is False" alone can't tell apart.
+        """
+        assert desc.sticky_fn is not None  # native_value only calls this when set
+        rep = self.coordinator.resource(self._bound.href)
+        now = time.monotonic()
+        matches = desc.sticky_fn(rep)
+
+        if matches:
+            if not self._sticky_armed:
+                self._sticky_value = (
+                    desc.sticky_value_fn(rep) if desc.sticky_value_fn is not None else raw
+                )
+                self._sticky_until = now + desc.sticky_seconds
+            self._sticky_armed = True
+        else:
+            self._sticky_armed = False
+            if desc.sticky_bypass_fn is not None and desc.sticky_bypass_fn(rep):
+                self._sticky_until = None
+                return desc.sticky_live_fn(rep) if desc.sticky_live_fn is not None else raw
+
+        if self._sticky_until is not None and now < self._sticky_until:
+            return self._sticky_value
+        return raw
+
     def _apply_hysteresis(self, raw):
         """Hold the last value this entity actually reported until a new one
         differs by at least the configured threshold, regardless of how long

tests/test_operational_capability.py

@@ -1,6 +1,10 @@
 """Unit tests for operational state capabilities."""
 
-from custom_components.localthings.registry.capabilities.operational import OPERATIONAL_STATE
+from custom_components.localthings.registry.capabilities.operational import (
+    OPERATIONAL_STATE,
+    _just_finished,
+    _live_progress_code,
+)
 from custom_components.localthings.registry.entities import NumberDesc
 
 
@@ -11,6 +15,55 @@ def test_machine_state_maps_samsung_to_ocf():
     assert ms.value_fn("Ready") == "idle"
 
 
+class TestJustFinished:
+    """`_just_finished` is progress/progress_percentage's sticky_fn arm
+    condition (issue #345) -- see sensor.py's _apply_sticky."""
+
+    def test_true_when_progress_is_finish_while_active(self):
+        assert _just_finished(
+            {"x.com.samsung.da.state": "Run", "x.com.samsung.da.progress": "Finish"}
+        )
+
+    def test_true_even_once_state_has_already_left_active(self):
+        """The exact issue #345 scenario this exists for: a washer's
+        `state` can already read idle by the time `progress` is observed
+        at 'Finish' -- unlike _is_active, this must still arm."""
+        assert _just_finished(
+            {"x.com.samsung.da.state": "Ready", "x.com.samsung.da.progress": "Finish"}
+        )
+
+    def test_false_while_active_but_not_yet_finished(self):
+        assert not _just_finished(
+            {"x.com.samsung.da.state": "Run", "x.com.samsung.da.progress": "Spin"}
+        )
+
+    def test_false_when_progress_absent(self):
+        assert not _just_finished({"x.com.samsung.da.state": "Run"})
+
+
+class TestLiveProgressCode:
+    """`_live_progress_code` is progress/progress_percentage's
+    sticky_bypass_fn (issue #345) -- see sensor.py's _apply_sticky."""
+
+    def test_true_for_a_concrete_non_finish_code(self):
+        assert _live_progress_code({"x.com.samsung.da.progress": "Wash"})
+
+    def test_true_regardless_of_state(self):
+        """Not gated on machine_state -- a new cycle's own real progress
+        must win over a held hold even while paused (e.g. adding a sock
+        mid-hold), not just while actively running."""
+        assert _live_progress_code(
+            {"x.com.samsung.da.state": "Pause", "x.com.samsung.da.progress": "Wash"}
+        )
+
+    def test_false_for_finish(self):
+        assert not _live_progress_code({"x.com.samsung.da.progress": "Finish"})
+
+    def test_false_when_absent_or_none(self):
+        assert not _live_progress_code({})
+        assert not _live_progress_code({"x.com.samsung.da.progress": "None"})
+
+
 class TestProgressPercentage:
     """issue #9: device firmware leaves progressPercentage stale (e.g. '1')
     after a cycle ends instead of resetting it, so it must be gated on

tests/test_sensor_sticky.py

@@ -0,0 +1,272 @@
+"""Unit tests for LocalThingsSensor's sticky gate (issue #345).
+
+Modeled on test_sensor_hysteresis.py: a _FakeCoordinator with just enough
+surface for LocalThingsEntity/LocalThingsSensor, driven directly rather
+than through a full coordinator/HA setup. Unlike that file, `data` here
+is derived from the real `flatten()` over the same `resources` dict
+`resource()` serves -- a single source of truth, so a test can't hide a
+production desync bug by hand-computing the two independently.
+"""
+
+from __future__ import annotations
+
+import time
+from dataclasses import replace
+from typing import cast
+
+from custom_components.localthings.coordinator import LocalThingsCoordinator
+from custom_components.localthings.registry.adapter import flatten
+from custom_components.localthings.registry.capabilities.operational import OPERATIONAL_STATE
+from custom_components.localthings.registry.discovery import BoundEntity
+from custom_components.localthings.sensor import LocalThingsSensor
+
+_PROGRESS_DESC = next(e for e in OPERATIONAL_STATE.entities if e.key == "progress")
+_PROGRESS_PERCENTAGE_DESC = next(
+    e for e in OPERATIONAL_STATE.entities if e.key == "progress_percentage"
+)
+_MACHINE_STATE_DESC = next(e for e in OPERATIONAL_STATE.entities if e.key == "machine_state")
+
+_HREF = "/operational/state/vs/0"
+_ALL_BOUND = [
+    BoundEntity(href=_HREF, capability=OPERATIONAL_STATE, desc=desc)
+    for desc in OPERATIONAL_STATE.entities
+]
+
+
+class _FakeConfigEntry:
+    def __init__(self):
+        self.options: dict = {}
+
+
+class _FakeCoordinator:
+    """Just enough surface for LocalThingsEntity/LocalThingsSensor.
+
+    `resources` is the one source of truth (standing in for the real
+    coordinator's live cache); `data` and `resource()` are both derived
+    from it exactly as the real coordinator derives `.data` from
+    `flatten(self.bound, self._cache.snapshot())` and `.resource()` from
+    the same snapshot -- so a test can't drift them apart in a way
+    production couldn't.
+    """
+
+    def __init__(self):
+        self.device_serial = "TEST-SERIAL"
+        self.config_entry = _FakeConfigEntry()
+        self.resources: dict[str, dict] = {}
+
+    def resource(self, href: str) -> dict:
+        return self.resources.get(href) or {}
+
+    @property
+    def data(self) -> dict:
+        return flatten(_ALL_BOUND, self.resources)
+
+
+def _sensor(desc):
+    coordinator = _FakeCoordinator()
+    bound = BoundEntity(href=_HREF, capability=OPERATIONAL_STATE, desc=desc)
+    sensor = LocalThingsSensor(cast(LocalThingsCoordinator, coordinator), bound)
+    return sensor, coordinator
+
+
+def _apply(coordinator, **fields):
+    """Merge `fields` onto the href's existing rep, the same shallow
+    {**cached, **rep} merge ObserveManager.apply() does in production
+    (issue #27) -- so a field this call doesn't mention (e.g. a stale
+    `progress` the device didn't repeat) stays exactly as a real partial
+    update would leave it, rather than being silently wiped."""
+    href_fields = {f"x.com.samsung.da.{k}": v for k, v in fields.items()}
+    coordinator.resources[_HREF] = {**coordinator.resources.get(_HREF, {}), **href_fields}
+
+
+def _replace(coordinator, **fields):
+    """A full rep replacement -- for a device reporting fresh from
+    scratch (e.g. a full poll GET), unlike _apply's partial-update merge."""
+    coordinator.resources[_HREF] = {f"x.com.samsung.da.{k}": v for k, v in fields.items()}
+
+
+def test_holds_finish_after_state_leaves_active():
+    """The #345 regression this guards: progress used to fall straight to
+    'Idle' the instant machine_state left active, even right after
+    reporting Finish."""
+    sensor, coordinator = _sensor(_PROGRESS_DESC)
+
+    _replace(coordinator, state="Run", progress="Finish", progressPercentage="100")
+    assert sensor.native_value == "Finish"
+
+    _replace(coordinator, state="Ready")  # device has moved on
+    assert sensor.native_value == "Finish"
+
+
+def test_holds_finish_even_when_state_already_idle_at_first_observation():
+    """issue #345's actual reported failure mode: state can already read
+    idle by the time `progress: Finish` is ever observed (the report's
+    washer skips a Run+Finish moment the same-family dryer still shows) --
+    the hold must still arm from that single rep."""
+    sensor, coordinator = _sensor(_PROGRESS_DESC)
+
+    _replace(coordinator, state="Ready", progress="Finish", progressPercentage="100")
+    assert sensor.native_value == "Finish"
+
+    # Still held on a later poll, even once the device stops repeating it.
+    _replace(coordinator, state="Ready")
+    assert sensor.native_value == "Finish"
+
+
+def test_progress_percentage_holds_100_regardless_of_the_raw_field_at_finish():
+    """The hold pins 100 explicitly (sticky_value_fn), not whatever the
+    raw field happened to hold at the finish moment -- a device that
+    never populates progressPercentage at all, or (issue #9's failure
+    mode) leaves a stale non-100 value there, must still show 100 while
+    held, not None/unknown or the stale figure."""
+    sensor, coordinator = _sensor(_PROGRESS_PERCENTAGE_DESC)
+
+    _replace(coordinator, state="Run", progress="Finish")  # no progressPercentage at all
+    assert sensor.native_value == 100
+
+    _replace(coordinator, state="Ready")
+    assert sensor.native_value == 100
+
+
+def test_real_data_flows_through_unheld_while_active():
+    sensor, coordinator = _sensor(_PROGRESS_DESC)
+
+    _replace(coordinator, state="Run", progress="Spin")
+    assert sensor.native_value == "Spin"
+
+    _replace(coordinator, state="Run", progress="Rinse")
+    assert sensor.native_value == "Rinse"
+
+
+def test_never_finished_stays_idle():
+    """A device that never actually reached Finish (e.g. Stop pressed
+    mid-cycle) must not be held at some prior mid-cycle value."""
+    sensor, coordinator = _sensor(_PROGRESS_DESC)
+
+    _replace(coordinator, state="Run", progress="Spin")
+    assert sensor.native_value == "Spin"
+
+    _replace(coordinator, state="Ready")
+    assert sensor.native_value == "Idle"
+
+
+def test_a_new_cycle_starting_overrides_the_hold():
+    """Starting a second cycle must show its own real progress
+    immediately, not the held Finish from the previous one."""
+    sensor, coordinator = _sensor(_PROGRESS_DESC)
+
+    _replace(coordinator, state="Run", progress="Finish", progressPercentage="100")
+    assert sensor.native_value == "Finish"
+
+    _replace(coordinator, state="Ready")
+    assert sensor.native_value == "Finish"  # still held
+
+    _replace(coordinator, state="Run", progress="Wash")
+    assert sensor.native_value == "Wash"
+
+
+def test_a_paused_new_cycle_also_overrides_the_hold():
+    """Not just an actively-running new cycle: adding a sock and pausing
+    mid-cycle must also show the real, current progress rather than a
+    stale hold from the previous cycle -- machine_state isn't 'active'
+    while paused, so a bypass keyed on that alone would miss this."""
+    sensor, coordinator = _sensor(_PROGRESS_DESC)
+
+    _replace(coordinator, state="Run", progress="Finish", progressPercentage="100")
+    assert sensor.native_value == "Finish"
+    _replace(coordinator, state="Ready")
+    assert sensor.native_value == "Finish"  # still held
+
+    _replace(coordinator, state="Pause", progress="Wash")
+    assert sensor.native_value == "Wash"
+
+
+def test_hold_expires_after_sticky_seconds():
+    # A fresh desc (frozen dataclass -- replace(), not mutation, so the
+    # module-level _PROGRESS_DESC other tests share stays untouched) with
+    # a short real window -- consistent with this codebase's own
+    # settle-guard tests (test_observe.py's mark_write_pending(...,
+    # settle_s=0.05)) -- rather than a zero window, which exercises "held
+    # disabled" rather than "an armed hold actually expires".
+    desc = replace(_PROGRESS_DESC, sticky_seconds=0.05)
+    sensor, coordinator = _sensor(desc)
+
+    _replace(coordinator, state="Run", progress="Finish", progressPercentage="100")
+    assert sensor.native_value == "Finish"
+
+    _replace(coordinator, state="Ready")
+    assert sensor.native_value == "Finish"  # still within the window
+
+    time.sleep(0.1)
+    assert sensor.native_value == "Idle"
+
+
+def test_a_progress_stuck_at_finish_does_not_hold_open_the_window_forever():
+    """Edge-triggering: sticky_fn keeps matching on every poll if the
+    device's own `progress` field never resets on its own (the same class
+    of quirk _completion_minutes' remainingTime-freeze workaround already
+    documents) -- the hold must still expire on schedule from its first
+    sighting, not have its deadline pushed out by every subsequent poll
+    that still (correctly, from the device's perspective) reports Finish
+    while machine_state has already gone idle."""
+    desc = replace(_PROGRESS_DESC, sticky_seconds=0.05)
+    sensor, coordinator = _sensor(desc)
+
+    _replace(coordinator, state="Ready", progress="Finish")
+    assert sensor.native_value == "Finish"
+
+    time.sleep(0.03)
+    # Device still (incorrectly) reports Finish on every subsequent poll --
+    # must not restart the window.
+    _replace(coordinator, state="Ready", progress="Finish")
+    assert sensor.native_value == "Finish"
+
+    time.sleep(0.03)  # 0.06s total since the first sighting -- past 0.05s
+    _replace(coordinator, state="Ready", progress="Finish")
+    assert sensor.native_value == "Idle"
+
+
+def test_non_sticky_sensor_is_unaffected():
+    """machine_state has no sticky_fn -- reads straight through, unchanged."""
+    sensor, coordinator = _sensor(_MACHINE_STATE_DESC)
+    _replace(coordinator, state="Run")
+    assert sensor.native_value == "active"
+    _replace(coordinator, state="Ready")
+    assert sensor.native_value == "idle"
+
+
+def test_cycle_active_and_machine_state_are_never_held():
+    """The whole point of scoping the hold to progress/progress_percentage
+    only: cycle_active (Running) and machine_state must keep reflecting
+    the device's real-time state throughout, never claiming the appliance
+    is still running once it isn't."""
+    progress_sensor, coordinator = _sensor(_PROGRESS_DESC)
+    machine_state_bound = BoundEntity(
+        href=_HREF, capability=OPERATIONAL_STATE, desc=_MACHINE_STATE_DESC
+    )
+    machine_state_sensor = LocalThingsSensor(
+        cast(LocalThingsCoordinator, coordinator), machine_state_bound
+    )
+
+    _replace(coordinator, state="Run", progress="Finish", progressPercentage="100")
+    assert progress_sensor.native_value == "Finish"
+    assert machine_state_sensor.native_value == "active"
+
+    _replace(coordinator, state="Ready")
+    assert progress_sensor.native_value == "Finish"  # held
+    assert machine_state_sensor.native_value == "idle"  # real-time, unaffected
+
+
+def test_a_partial_update_that_omits_progress_does_not_erase_the_hold():
+    """A device's partial poll/notify that doesn't repeat `progress` at
+    all (issue #27's documented shape) merges onto the cache rather than
+    replacing it -- confirms the hold reads the merged rep, still showing
+    'Finish' from the earlier full rep, not a wiped/absent field."""
+    sensor, coordinator = _sensor(_PROGRESS_DESC)
+
+    _replace(coordinator, state="Run", progress="Finish", progressPercentage="100")
+    assert sensor.native_value == "Finish"
+
+    _apply(coordinator, state="Ready")  # partial merge, doesn't restate progress
+    assert coordinator.resources[_HREF]["x.com.samsung.da.progress"] == "Finish"
+    assert sensor.native_value == "Finish"