laundry: one hold per cycle, so the sticky bound is actually a bound
Review of the previous commit caught that its docstring promised more than the code did. Not restarting an *open* window still let a progress that flapped out of and back into Finish re-arm a full fresh window once the first had expired, so the value could be held well past sticky_seconds from the first Finish. The new test passed only because its final read left the sticky condition matching; ending the flap on a non-matching read re-armed and would have failed it. Make the guarantee real instead of weakening the claim: arming marks the hold spent, and only sticky_bypass_fn -- a cycle actually running -- clears it. Expiry on its own no longer re-opens the door, because with no cycle in between a second Finish is the same Finish, and re-arming on it strobes the entity Finish -> Idle -> Finish once per window, re-firing the announcements #345 and #358 are both about. That subsumes the old _sticky_armed edge-trigger flag, which existed to stop a stuck field extending the window; "spent until a new cycle" covers that case and the flap case together, before or after expiry. Also give the flap test real headroom -- it fitted 0.09s of sleeps into a 0.1s window and would have failed spuriously on a loaded runner.
This commit is contained in:
@@ -65,8 +65,8 @@ class SensorDesc(SamsungEntityDescription):
|
|||||||
# device-side revisions -- not a general-purpose flag.
|
# device-side revisions -- not a general-purpose flag.
|
||||||
hysteresis: bool = False
|
hysteresis: bool = False
|
||||||
# Opt-in, entity-instance-only hold -- see sensor.py's _apply_sticky
|
# Opt-in, entity-instance-only hold -- see sensor.py's _apply_sticky
|
||||||
# for the full contract (arm/value/bypass semantics, edge-triggering,
|
# for the full contract (arm/value/bypass semantics, one window per
|
||||||
# why this never touches the coordinator cache).
|
# bypass, why this never touches the coordinator cache).
|
||||||
# sticky_fn arms it; sticky_value_fn picks what to freeze at that
|
# 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
|
# 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
|
# hold and lets rep_fn's own live result through; sticky_seconds
|
||||||
|
|||||||
@@ -54,7 +54,7 @@ class LocalThingsSensor(LocalThingsEntity, SensorEntity):
|
|||||||
self._hysteresis_value = None
|
self._hysteresis_value = None
|
||||||
self._sticky_value = None
|
self._sticky_value = None
|
||||||
self._sticky_until: float | None = None
|
self._sticky_until: float | None = None
|
||||||
self._sticky_armed = False
|
self._sticky_spent = False
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def native_unit_of_measurement(self):
|
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
|
held entity and a free-running one agree on what "live" means; a
|
||||||
hook that broke that rule caused issue #358.
|
hook that broke that rule caused issue #358.
|
||||||
|
|
||||||
Edge-triggered, not level-triggered: the window (re)starts only on
|
At most one window per `sticky_bypass_fn` cycle: arming marks the
|
||||||
a fresh False->True transition of `sticky_fn`, is never restarted
|
hold spent, and only the bypass clears that. So a `sticky_fn` that
|
||||||
while already open, and is checked for expiry on every call even
|
keeps matching (firmware leaving the field stuck -- the quirk
|
||||||
while `sticky_fn` keeps matching -- so neither a field stuck
|
`_completion_minutes` works around) can't extend the window, and
|
||||||
matching forever (the quirk `_completion_minutes` works around)
|
one flapping in and out can't restart it either, before or after
|
||||||
nor one flapping in and out can hold the value past
|
expiry. Expiry alone doesn't re-open the door: without something
|
||||||
`sticky_seconds` from when it first armed.
|
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_bypass_fn` drops the hold and returns `raw`, for when "not
|
||||||
sticky right now" is ambiguous between "went idle, honor the hold"
|
sticky right now" is ambiguous between "went idle, honor the hold"
|
||||||
and "genuinely moved on to new data". Being the early-release
|
and "genuinely moved on to new data". It is both the early release
|
||||||
path it should demand positive evidence of the latter; when
|
and the only re-arm, so it should demand positive evidence of that
|
||||||
unsure, letting the window run out is the cheaper mistake.
|
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
|
assert desc.sticky_fn is not None # native_value only calls this when set
|
||||||
rep = self.coordinator.resource(self._bound.href)
|
rep = self.coordinator.resource(self._bound.href)
|
||||||
now = time.monotonic()
|
now = time.monotonic()
|
||||||
holding = self._sticky_until is not None and now < self._sticky_until
|
|
||||||
|
|
||||||
if desc.sticky_fn(rep):
|
if desc.sticky_fn(rep):
|
||||||
if not self._sticky_armed and not holding:
|
if not self._sticky_spent:
|
||||||
self._sticky_value = (
|
self._sticky_value = (
|
||||||
desc.sticky_value_fn(rep) if desc.sticky_value_fn is not None else raw
|
desc.sticky_value_fn(rep) if desc.sticky_value_fn is not None else raw
|
||||||
)
|
)
|
||||||
self._sticky_until = now + desc.sticky_seconds
|
self._sticky_until = now + desc.sticky_seconds
|
||||||
holding = True
|
self._sticky_spent = True
|
||||||
self._sticky_armed = True
|
elif desc.sticky_bypass_fn is not None and desc.sticky_bypass_fn(rep):
|
||||||
else:
|
self._sticky_until = None
|
||||||
self._sticky_armed = False
|
self._sticky_spent = False
|
||||||
if desc.sticky_bypass_fn is not None and desc.sticky_bypass_fn(rep):
|
return raw
|
||||||
self._sticky_until = None
|
|
||||||
return raw
|
|
||||||
|
|
||||||
|
holding = self._sticky_until is not None and now < self._sticky_until
|
||||||
return self._sticky_value if holding else raw
|
return self._sticky_value if holding else raw
|
||||||
|
|
||||||
def _apply_hysteresis(self, raw):
|
def _apply_hysteresis(self, raw):
|
||||||
|
|||||||
+43
-10
@@ -234,31 +234,64 @@ def test_a_paused_new_cycle_is_left_to_the_window_rather_than_released():
|
|||||||
assert sensor.native_value == "Wash"
|
assert sensor.native_value == "Wash"
|
||||||
|
|
||||||
|
|
||||||
def test_a_flapping_finish_cannot_ratchet_the_window_forward():
|
def test_a_flapping_finish_cannot_ratchet_an_open_window_forward():
|
||||||
"""Edge-triggering stops a *stuck* Finish from extending the hold, but
|
"""Edge-triggering stops a *stuck* Finish from extending the hold; a
|
||||||
a device whose progress flaps out of and back into Finish re-arms --
|
progress that flaps out of and back into Finish must not restart it
|
||||||
an already-open window must not restart on that, or the bound stops
|
either, or the bound stops being a bound."""
|
||||||
being a bound."""
|
desc = replace(_PROGRESS_DESC, sticky_seconds=0.3)
|
||||||
desc = replace(_PROGRESS_DESC, sticky_seconds=0.1)
|
|
||||||
sensor, coordinator = _sensor(desc)
|
sensor, coordinator = _sensor(desc)
|
||||||
|
|
||||||
_replace(coordinator, state="Ready", progress="Finish")
|
_replace(coordinator, state="Ready", progress="Finish")
|
||||||
assert sensor.native_value == "Finish"
|
assert sensor.native_value == "Finish"
|
||||||
|
|
||||||
for _ in range(3):
|
for _ in range(3):
|
||||||
time.sleep(0.03)
|
time.sleep(0.05)
|
||||||
_replace(coordinator, state="Ready", progress="None")
|
_replace(coordinator, state="Ready", progress="None")
|
||||||
assert sensor.native_value == "Finish"
|
assert sensor.native_value == "Finish"
|
||||||
_replace(coordinator, state="Ready", progress="Finish")
|
_replace(coordinator, state="Ready", progress="Finish")
|
||||||
assert sensor.native_value == "Finish"
|
assert sensor.native_value == "Finish"
|
||||||
|
|
||||||
# 0.09s of flapping so far; past 0.1s from the *first* Finish it ends,
|
# 0.15s of flapping so far -- the window still ends 0.3s after the
|
||||||
# rather than 0.1s from the most recent re-arm.
|
# first Finish, not 0.3s after the most recent re-entry.
|
||||||
time.sleep(0.03)
|
time.sleep(0.2)
|
||||||
_replace(coordinator, state="Ready", progress="Finish")
|
_replace(coordinator, state="Ready", progress="Finish")
|
||||||
assert sensor.native_value == "Idle"
|
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():
|
def test_hold_expires_after_sticky_seconds():
|
||||||
# A fresh desc (frozen dataclass -- replace(), not mutation, so the
|
# A fresh desc (frozen dataclass -- replace(), not mutation, so the
|
||||||
# module-level _PROGRESS_DESC other tests share stays untouched) with
|
# module-level _PROGRESS_DESC other tests share stays untouched) with
|
||||||
|
|||||||
Reference in New Issue
Block a user