diff --git a/custom_components/localthings/observe.py b/custom_components/localthings/observe.py index 0c96a4b..1215766 100644 --- a/custom_components/localthings/observe.py +++ b/custom_components/localthings/observe.py @@ -56,6 +56,26 @@ SUCCESS_FRACTION = 0.8 PUSH_HEALTH_WINDOW_S = 60.0 +def _is_alarms_href(href: str) -> bool: + """True for /alarms/vs/ in any subdevice-translated shape -- + the canonical MAIN form (/alarms/vs/0), an indexed subdevice's + renumbered instance (/alarms/vs/), or a prefixed subdevice's + UUID-qualified form (//alarms/vs/0). `Subdevice.to_actual` + (registry/subdevices.py) only ever rewrites the trailing index + segment or prepends a prefix -- it never touches the 'alarms/vs' + stem -- so matching that fixed segment plus a wildcard tail catches + every shape without this module needing to be subdevice-aware. + + See `ObserveManager.apply`'s use of this for why the href matters: + unlike most resources, /alarms/vs/0's `x.com.samsung.da.items` array + is a complete snapshot of every currently-active alarm, not a + possibly-partial field update -- so it must never be merged onto a + stale prior rep (issue #348). + """ + head, _, _ = href.rpartition("/") + return head.endswith("/alarms/vs") + + class ObserveManager: """Per-device observe-mode state: mode, write-settle guard, and (later) subscription/staleness tracking. Pure sync logic — safe to call from @@ -122,6 +142,20 @@ class ObserveManager: comes through, even though nothing about the device's actual supported options changed. + `_is_alarms_href` is the one exception to that merge (issue #348): + /alarms/vs/0's `items` array is always sent as a complete + snapshot of every currently-active alarm, never a partial delta + -- confirmed by a live `read_resource` GET returning `{}` (no + `items` key at all) the moment a washer's board actually clears + an alarm, which entity.py already documents as this resource's + normal no-alarm shape. Merging that `{}` onto the prior rep the + same way as everywhere else silently kept the stale `items` + entry forever: an absent key merges as "unchanged" everywhere + else, but on this href absent specifically means "cleared". + Every family that exposes an alarm sensor shares this href + (common.ALARMS, range_hood's own copy), so this is a full + replace for all of them, not a washer-specific carve-out. + `apply()` is the sole path StateCache mutations flow through in this component (poll, sweep, and OBSERVE notify all funnel here), so `_cache_lock` serializes the read-then-write across those @@ -149,7 +183,7 @@ class ObserveManager: self.log.debug("dropping %s update for %s (settling)", source, href) return False with self._cache_lock: - merged = {**(self.cache.get(href) or {}), **rep} + merged = dict(rep) if _is_alarms_href(href) else {**(self.cache.get(href) or {}), **rep} changed = self.cache.apply_rep(href, merged, source=source) # Outside the cache lock -- the hook takes locks of its own and # never reads the cache back. `source` is passed along rather than diff --git a/tests/localthings/test_observe.py b/tests/localthings/test_observe.py index 5a24766..87af3e4 100644 --- a/tests/localthings/test_observe.py +++ b/tests/localthings/test_observe.py @@ -58,6 +58,49 @@ def test_apply_merges_partial_update_onto_prior_rep(): assert cached["x.com.samsung.da.supportedOptions"] == ["CV_FDR_WINE", "CV_FDR_MEAT"] +def test_apply_fully_replaces_alarms_href_instead_of_merging(): + """Regression test for issue #348: /alarms/vs/0's `items` array is a + complete snapshot of every currently-active alarm, not a partial field + update like /mode/vs/0 (issue #27). A washer's board reports a cleared + alarm by omitting `items` entirely -- a live read_resource GET showed + `{}` -- so merging that onto the prior rep (as every other href does) + left the stale ErrorCode_DC entry in the cache forever. This must + instead behave like a full replace: the empty rep wins outright.""" + mgr = _manager() + active = { + "x.com.samsung.da.items": [ + {"x.com.samsung.da.code": "ErrorCode_DC", "x.com.samsung.da.state": "Created"} + ] + } + mgr.apply("/alarms/vs/0", active, source="poll") + assert mgr.cache.get("/alarms/vs/0") == active + + cleared = mgr.apply("/alarms/vs/0", {}, source="poll") + + assert cleared is True + assert mgr.cache.get("/alarms/vs/0") == {} + + +def test_apply_fully_replaces_alarms_href_for_subdevice_shapes(): + """The same full-replace behavior must hold for both hrefs + `Subdevice.to_actual` can produce: an indexed subdevice renumbers only + the trailing '0' (/alarms/vs/1), and a prefixed one prepends a UUID + (//alarms/vs/0) -- neither ever touches the 'alarms/vs' stem + itself (registry/subdevices.py).""" + for href in ("/alarms/vs/1", "/6c2dff6d-ee5c-dad1-6a5e-000000000001/alarms/vs/0"): + mgr = _manager() + mgr.apply( + href, + {"x.com.samsung.da.items": [{"x.com.samsung.da.code": "ErrorCode_UB"}]}, + source="poll", + ) + + cleared = mgr.apply(href, {}, source="poll") + + assert cleared is True + assert mgr.cache.get(href) == {} + + def test_apply_drops_update_during_settle_window(): mgr = _manager() mgr.cache.apply_rep("/oven/vs/0", {"a": 1}, source="seed")