diff --git a/custom_components/localthings/registry/entities.py b/custom_components/localthings/registry/entities.py index 6be0266..57e6149 100644 --- a/custom_components/localthings/registry/entities.py +++ b/custom_components/localthings/registry/entities.py @@ -65,8 +65,8 @@ class SensorDesc(SamsungEntityDescription): # 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/bypass semantics, edge-triggering, - # why this never touches the coordinator cache). + # for the full contract (arm/value/bypass semantics, one window per + # bypass, 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 drops the # hold and lets rep_fn's own live result through; sticky_seconds diff --git a/custom_components/localthings/sensor.py b/custom_components/localthings/sensor.py index f928e21..7f6e58e 100644 --- a/custom_components/localthings/sensor.py +++ b/custom_components/localthings/sensor.py @@ -54,7 +54,7 @@ class LocalThingsSensor(LocalThingsEntity, SensorEntity): self._hysteresis_value = None self._sticky_value = None self._sticky_until: float | None = None - self._sticky_armed = False + self._sticky_spent = False @property def native_unit_of_measurement(self): @@ -89,39 +89,41 @@ class LocalThingsSensor(LocalThingsEntity, SensorEntity): held entity and a free-running one agree on what "live" means; a hook that broke that rule caused issue #358. - Edge-triggered, not level-triggered: the window (re)starts only on - a fresh False->True transition of `sticky_fn`, is never restarted - while already open, and is checked for expiry on every call even - while `sticky_fn` keeps matching -- so neither a field stuck - matching forever (the quirk `_completion_minutes` works around) - nor one flapping in and out can hold the value past - `sticky_seconds` from when it first armed. + At most one window per `sticky_bypass_fn` cycle: arming marks the + hold spent, and only the bypass clears that. So a `sticky_fn` that + keeps matching (firmware leaving the field stuck -- the quirk + `_completion_minutes` works around) can't extend the window, and + one flapping in and out can't restart it either, before or after + expiry. Expiry alone doesn't re-open the door: without something + the calibre of "a new cycle is actually running" in between, a + second Finish is the same Finish, and re-arming on it would strobe + the entity between held and live once per window -- exactly the + repeated announcements #345 and #358 are about. `sticky_bypass_fn` drops the hold and returns `raw`, for when "not sticky right now" is ambiguous between "went idle, honor the hold" - and "genuinely moved on to new data". Being the early-release - path it should demand positive evidence of the latter; when - unsure, letting the window run out is the cheaper mistake. + and "genuinely moved on to new data". It is both the early release + and the only re-arm, so it should demand positive evidence of that + move; when unsure, letting the window run out is the cheaper + mistake. """ assert desc.sticky_fn is not None # native_value only calls this when set rep = self.coordinator.resource(self._bound.href) now = time.monotonic() - holding = self._sticky_until is not None and now < self._sticky_until if desc.sticky_fn(rep): - if not self._sticky_armed and not holding: + if not self._sticky_spent: 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 - holding = True - 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 raw + self._sticky_spent = True + elif desc.sticky_bypass_fn is not None and desc.sticky_bypass_fn(rep): + self._sticky_until = None + self._sticky_spent = False + return raw + holding = self._sticky_until is not None and now < self._sticky_until return self._sticky_value if holding else raw def _apply_hysteresis(self, raw): diff --git a/tests/test_sensor_sticky.py b/tests/test_sensor_sticky.py index a68b536..962f0eb 100644 --- a/tests/test_sensor_sticky.py +++ b/tests/test_sensor_sticky.py @@ -234,31 +234,64 @@ def test_a_paused_new_cycle_is_left_to_the_window_rather_than_released(): assert sensor.native_value == "Wash" -def test_a_flapping_finish_cannot_ratchet_the_window_forward(): - """Edge-triggering stops a *stuck* Finish from extending the hold, but - a device whose progress flaps out of and back into Finish re-arms -- - an already-open window must not restart on that, or the bound stops - being a bound.""" - desc = replace(_PROGRESS_DESC, sticky_seconds=0.1) +def test_a_flapping_finish_cannot_ratchet_an_open_window_forward(): + """Edge-triggering stops a *stuck* Finish from extending the hold; a + progress that flaps out of and back into Finish must not restart it + either, or the bound stops being a bound.""" + desc = replace(_PROGRESS_DESC, sticky_seconds=0.3) sensor, coordinator = _sensor(desc) _replace(coordinator, state="Ready", progress="Finish") assert sensor.native_value == "Finish" for _ in range(3): - time.sleep(0.03) + time.sleep(0.05) _replace(coordinator, state="Ready", progress="None") assert sensor.native_value == "Finish" _replace(coordinator, state="Ready", progress="Finish") assert sensor.native_value == "Finish" - # 0.09s of flapping so far; past 0.1s from the *first* Finish it ends, - # rather than 0.1s from the most recent re-arm. - time.sleep(0.03) + # 0.15s of flapping so far -- the window still ends 0.3s after the + # first Finish, not 0.3s after the most recent re-entry. + time.sleep(0.2) _replace(coordinator, state="Ready", progress="Finish") assert sensor.native_value == "Idle" +def test_a_finish_after_the_window_closes_does_not_re_arm_it(): + """Expiry doesn't re-open the door: with no new cycle in between, a + second Finish is the same Finish. Re-arming on it would strobe the + entity Finish -> Idle -> Finish once per window, re-firing exactly the + announcements #345 is about -- so it takes a bypass (a cycle actually + running) to make the hold available again. + + Distinct from the flap test above: there the window is still open, and + the last read before expiry leaves the sticky condition *matching*. + Here it has already closed, and the flap ends on a non-matching read, + which is the state a spent-on-arm-only guard would let re-arm.""" + 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.1) + assert sensor.native_value == "Idle" + + _replace(coordinator, state="Ready", progress="None") + assert sensor.native_value == "Idle" + _replace(coordinator, state="Ready", progress="Finish") + assert sensor.native_value == "Idle" + + # A real cycle in between is what makes it available again. + _replace(coordinator, state="Run", progress="Drying") + assert sensor.native_value == "Drying" + _replace(coordinator, state="Run", progress="Finish") + assert sensor.native_value == "Finish" + _replace(coordinator, state="Ready", progress="None") + assert sensor.native_value == "Finish" + + 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