Scope learning to the device, and simplify the store

The href alone was not a sufficient key. /mode/convenient/vs/0 is
declared by three family registries with three meanings: a real preset
resource on the AC, explicitly unmodeled on the dehumidifier (no live
current-value field), empty on the air purifier. Matching on the href
globally meant a dehumidifier reporting a mode there would learn it,
persist it, and show it in diagnostics for a resource nothing offers.
The coordinator now narrows LEARNABLE to the hrefs a climate entity is
actually bound to, at discovery -- which also retires the per-rep
subdevice walk, since those hrefs are already actual.

With that, LEARNABLE is a plain frozenset of hrefs and the per-href
LearnRule goes away: its two fields were the same module constants for
its only entry. observe() now returns the codes it learned rather than a
bool the caller re-reads the store to interpret, so the log names what
was new instead of everything ever learned.

learned.py also takes ownership of the entry key and persisted shape --
the options flow was the second module that knew both, and the shape has
already changed once.

Comment trims throughout, per CONTRIBUTING: the LEARNABLE entry no
longer recounts how many reporters there were, and three copies of the
same test-stub comment are gone.

custom_components/localthings/config_flow.py

@@ -46,7 +46,6 @@ from .const import (
     CONF_LEAF_CERT_PEM,
     CONF_LEAF_KEY_PEM,
     CONF_LEARN_MODES,
-    CONF_LEARNED_MODES,
     CONF_MANUFACTURER,
     CONF_MODEL,
     CONF_PORT,
@@ -61,7 +60,8 @@ from .const import (
     PROBE_PORT_RANGE,
     SERVICE_WRITE_RESOURCE,
 )
-from .learned import LearnedModes
+from .learned import persist as learned_persist
+from .learned import stored as learned_stored
 
 _TEXT = TextSelector(TextSelectorConfig(type=TextSelectorType.TEXT))
 _MULTILINE = TextSelector(TextSelectorConfig(type=TextSelectorType.TEXT, multiline=True))
@@ -879,13 +879,12 @@ class LocalThingsOptionsFlow(config_entries.OptionsFlow):
         form; the description lists what's about to be forgotten.
         """
         coord = self._coordinator()
+        # learned.py owns the entry key and the persisted shape, so this
+        # step never parses or writes it itself -- including on an unloaded
+        # entry, where a malformed record would otherwise abort the one
+        # screen that can clear it.
         learned = (
-            coord.learned_snapshot()
+            coord.learned_snapshot() if coord is not None else learned_stored(self.config_entry)
-            if coord is not None
-            # Through LearnedModes, not the raw entry value: this step is
-            # the one screen that can clear a malformed persisted record,
-            # so it must not be the one screen that trips over it.
-            else LearnedModes(self.config_entry.data.get(CONF_LEARNED_MODES)).snapshot()
         )
         codes = sorted({code for codes in learned.values() for code in codes})
 
@@ -893,13 +892,7 @@ class LocalThingsOptionsFlow(config_entries.OptionsFlow):
             if coord is not None:
                 coord.forget_learned_modes()
             else:
-                # Not loaded, so there's no store to clear -- drop the
+                learned_persist(self.hass, self.config_entry, {})
-                # persisted copy directly, which is all a reload would
-                # restore from anyway.
-                self.hass.config_entries.async_update_entry(
-                    self.config_entry,
-                    data={**self.config_entry.data, CONF_LEARNED_MODES: {}},
-                )
             return self.async_create_entry(data=dict(self.config_entry.options))
 
         return self.async_show_form(

custom_components/localthings/coordinator.py

@@ -40,7 +40,7 @@ from .const import (
     DTLS_LOCAL_PORT_BASE,
     SUMMARY_INTERVAL_S,
 )
-from .learned import LearnedModes
+from .learned import LEARNABLE, LearnedModes, persist
 from .observe import GRACE_PERIOD_S, MODE_OBSERVE, MODE_POLL, ObserveManager
 from .registry import CAPABILITIES
 from .registry.adapter import flatten
@@ -53,6 +53,7 @@ from .registry.capabilities.common import (
     remote_control_required_for_write,
 )
 from .registry.discovery import BoundEntity
+from .registry.entities import ClimateDesc
 from .registry.identity import (
     DeviceIdentity,
     device_display_name,
@@ -247,6 +248,9 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
         # supported (issue #327), restored from the entry so one learned
         # last week is still offered today. See learned.py.
         self._learned = LearnedModes(entry.data.get(CONF_LEARNED_MODES))
+        # Narrowed to this device's own climate hrefs once discovery has
+        # run -- see _refresh_learnable_hrefs.
+        self._learnable_hrefs: set[str] = set()
         self._observe.set_on_applied(self._on_rep_applied)
         self._push_pending = False
         self._push_pending_lock = threading.Lock()
@@ -349,13 +353,23 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
         if self._entry.data.get(CONF_LEARNED_MODES):
             self._persist_learned()
 
-    def _canonical_href(self, actual: str) -> str:
+    def _refresh_learnable_hrefs(self) -> None:
-        """`actual` in the namespace the registry -- and so learned.LEARNABLE
+        """The actual hrefs learning applies to on this device: LEARNABLE's
-        -- is written against; identity for MAIN's own hrefs (issue #177)."""
+        canonical set, narrowed to the ones a climate entity is bound to
-        for subdevice in self.subdevices:
+        read back (climate._supported is the only consumer) and translated
-            if subdevice.owns(actual):
+        through that entity's own subdevice (issue #177).
-                return subdevice.to_canonical(actual) or actual
+
-        return actual
+        The href alone isn't a sufficient key here. `/mode/convenient/vs/0`
+        is also declared by the dehumidifier registry (explicitly
+        unmodeled) and the air purifier's (empty), so a global match would
+        persist a code for a resource that family will never offer.
+        """
+        self._learnable_hrefs = {
+            bound.subdevice.to_actual(href)
+            for bound in self.bound
+            if isinstance(bound.desc, ClimateDesc)
+            for href in LEARNABLE
+        }
 
     def _on_rep_applied(self, href: str, rep: dict, source: str) -> None:
         """ObserveManager.set_on_applied hook. Runs on whichever thread
@@ -364,23 +378,22 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
 
         An 'optimistic' rep is the value this integration just wrote, not
         one the device reported, so there is nothing to learn from it."""
-        if source == "optimistic" or not self.learning_enabled:
+        if source == "optimistic" or href not in self._learnable_hrefs:
             return
-        if self._learned.observe(self._canonical_href(href), href, rep):
+        if not self.learning_enabled:
+            return
+        if new := self._learned.observe(href, rep):
             self._log.info(
                 "%s reported mode(s) it does not advertise as supported; "
                 "remembering %s so they stay selectable (issue #327)",
                 href,
-                self._learned.codes(href),
+                new,
             )
             self.hass.add_job(self._persist_learned)
 
     @callback
     def _persist_learned(self) -> None:
-        self.hass.config_entries.async_update_entry(
+        persist(self.hass, self._entry, self._learned.snapshot())
-            self._entry,
-            data={**self._entry.data, CONF_LEARNED_MODES: self._learned.snapshot()},
-        )
 
     def device_info_for(self, subdevice: Subdevice) -> DeviceInfo:
         """DeviceInfo for one logical subdevice on this connection (issue
@@ -802,6 +815,7 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
         self.device_type_name = device_type_name
         self.bound = bound
         self._unbound_hrefs = unbound
+        self._refresh_learnable_hrefs()
 
         # The entry's stored identity wins; this poll's answer is only
         # adopted when nothing is stored (a legacy migration couldn't

custom_components/localthings/learned.py

@@ -11,47 +11,37 @@ Learning is deliberately not global. A current value that isn't a
 selectable option is common across this corpus -- an oven idling in
 'NoOperation', a fridge's /mode/vs/0 carrying capability tokens like
 'WATERFILTER_DISABLE' -- and remembering one of those permanently would
-put an option in the UI that the device can only reject. LEARNABLE is the
+put an option in the UI that the device can only reject. LEARNABLE names
-allowlist of canonical hrefs where a device-reported current mode is known
+the canonical hrefs where a reported mode is known to be genuinely
-to be a genuine selectable option; every consumer of it (climate's
+selectable, and the coordinator narrows it further to the hrefs this
-_supported today) must read its supported list through the coordinator.
+device actually binds a climate entity to (see _refresh_learnable_hrefs):
+the same href is declared explicitly unmodeled on a dehumidifier and
+empty on an air purifier, and learning for those would persist a code
+nothing ever offers.
+
+This module also owns the entry key the store persists under, so the
+shape lives in exactly one place.
 """
 
 from __future__ import annotations
 
 import threading
-from dataclasses import dataclass
 
+from .const import CONF_LEARNED_MODES
 from .registry.capabilities.airconditioner import HREF_CONVENIENT
 
 MODES_FIELD = "x.com.samsung.da.modes"
 SUPPORTED_FIELD = "x.com.samsung.da.supportedModes"
 
-
+# Convenient (preset) mode: firmware omits an active preset (e.g. Quiet)
-@dataclass(frozen=True)
+# from its own supportedModes -- a reporting gap, not a capability
-class LearnRule:
+# difference (issue #327).
-    """Which field of a rep names the current mode, and which lists the
+LEARNABLE: frozenset[str] = frozenset({HREF_CONVENIENT})
-    supported ones. Both are per-href because Samsung spells them
-    differently across resources (`supportedModes` on the vendor `/vs/`
-    ones, bare `modes`/`supportedModes` on a few OCF-shaped ones)."""
-
-    current_field: str
-    supported_field: str
-
-
-LEARNABLE: dict[str, LearnRule] = {
-    # Convenient (preset) mode. Reported by two independent reporters on
-    # ARTIK051_PRAC_20K, one of whom has three identical units where only
-    # the two sharing an outdoor unit hide Quiet -- so this is a firmware
-    # reporting gap, not a real capability difference.
-    HREF_CONVENIENT: LearnRule(MODES_FIELD, SUPPORTED_FIELD),
-}
 
 
 def _codes(value) -> list[str]:
-    """The mode codes in a `modes`-style field, which is an array on every
+    """Mode codes from a `modes`-style field, which some firmwares send as
-    board seen but a bare string on none -- tolerated anyway, since this
+    a bare string rather than an array."""
-    runs against whatever the device sends."""
     if isinstance(value, str):
         return [value]
     if isinstance(value, (list, tuple)):
@@ -73,11 +63,21 @@ def _coerce(stored) -> dict[str, list[str]]:
     return restored
 
 
+def stored(entry) -> dict[str, list[str]]:
+    """What `entry` has persisted, coerced -- for a reader that can't go
+    through a coordinator (the options flow, on an unloaded entry)."""
+    return _coerce(entry.data.get(CONF_LEARNED_MODES))
+
+
+def persist(hass, entry, codes: dict[str, list[str]]) -> None:
+    """Write `codes` onto the entry. Runs on the event loop, which
+    async_update_entry requires."""
+    hass.config_entries.async_update_entry(entry, data={**entry.data, CONF_LEARNED_MODES: codes})
+
+
 class LearnedModes:
     """Per-device store of learned codes, keyed by actual (on-the-wire)
     href so two subdevices of one composite appliance learn separately.
-    One href carries one LEARNABLE rule, so the rule's fields are how a rep
-    is read, never part of the key.
 
     Mutated from whichever thread applied the update (the DTLS reader for
     an OBSERVE notify, an executor thread for a poll -- see
@@ -89,9 +89,9 @@ class LearnedModes:
         self._lock = threading.Lock()
         self._learned = _coerce(stored)
 
-    def observe(self, canonical_href: str, actual_href: str, rep: dict) -> bool:
+    def observe(self, actual_href: str, rep: dict) -> list[str]:
-        """Learn from one applied rep; True when something new was learned
+        """Learn from one applied rep; returns the codes newly learned, so
-        (i.e. the caller should persist).
+        an empty list means there is nothing to persist.
 
         A rep that carries no supported list teaches nothing: "missing from
         the list" is only meaningful against a list that exists, and
@@ -101,23 +101,19 @@ class LearnedModes:
         alone (issue #27) still sees the supported list from the last full
         poll.
         """
-        rule = LEARNABLE.get(canonical_href)
+        supported = _codes(rep.get(SUPPORTED_FIELD))
-        if rule is None:
-            return False
-        supported = _codes(rep.get(rule.supported_field))
         if not supported:
-            return False
+            return []
         with self._lock:
             known = self._learned.get(actual_href, [])
             new = [
                 code
-                for code in _codes(rep.get(rule.current_field))
+                for code in _codes(rep.get(MODES_FIELD))
                 if code and code not in supported and code not in known
             ]
-            if not new:
+            if new:
-                return False
+                self._learned[actual_href] = [*known, *new]
-            self._learned[actual_href] = [*known, *new]
+        return new
-        return True
 
     def codes(self, actual_href: str) -> list[str]:
         with self._lock:

custom_components/localthings/observe.py

@@ -78,22 +78,18 @@ class ObserveManager:
         # have notified. Guards only `_notified` mutations + the `wait_for`.
         self._notify_cond = threading.Condition()
         self.fallback_hrefs: set[str] = set()
-        # Called with (href, merged_rep, source) after every accepted
-        # device update, on the applying thread -- see set_on_applied.
         self._on_applied: Callable[[str, dict, str], None] | None = None
         self._refresh_task: ObserveRefreshTask | None = None
         self._refresh_stop: threading.Event | None = None
         self._refresh_thread: threading.Thread | None = None
 
     def set_on_applied(self, callback: Callable[[str, dict, str], None]) -> None:
-        """Register a hook run after every rep this manager accepts, with
+        """Hook run after every accepted rep, on the applying thread.
-        the merged rep that reached the cache.
 
-        Unlike StateCache.set_on_change, which reports only that
+        Unlike StateCache.set_on_change it carries the href and rep, and
-        *something* changed, this hands over the href and rep -- and fires
+        fires even when the rep is unchanged -- which learned.py needs, a
-        even when the rep matched what was already cached, which the
+        device sitting in an unadvertised mode re-sending the same rep
-        learned-modes store (learned.py) depends on: a device sitting in
+        every poll.
-        an unadvertised mode sends an unchanged rep every poll.
         """
         self._on_applied = callback
 

tests/test_airconditioner_artik051_krac.py

@@ -65,8 +65,6 @@ class _FakeCoordinator:
         self.commands.append((bound, payload))
 
     def learned_modes(self, href):
-        # Nothing learned in this stub -- issue #327's store lives on the
-        # real coordinator; climate._supported unions it in.
         return []
 
 

tests/test_airconditioner_tp1x_rac_01001_fan.py

@@ -49,8 +49,6 @@ class _FakeCoordinator:
         self.commands.append((bound, payload))
 
     def learned_modes(self, href):
-        # Nothing learned in this stub -- issue #327's store lives on the
-        # real coordinator; climate._supported unions it in.
         return []
 
 

tests/test_climate_ac_modes.py

@@ -120,8 +120,6 @@ def test_fac_bora_wind_strength_codes_fit_the_standard_scale():
             return self.last_resources
 
         def learned_modes(self, href):
-            # Nothing learned in this stub -- issue #327's store lives on
-            # the real coordinator; climate._supported unions it in.
             return []
 
     resources = _load_device("airconditioner_fac_bora")

tests/test_learned_modes.py

@@ -31,7 +31,7 @@ from custom_components.localthings.learned import (
     LearnedModes,
 )
 from custom_components.localthings.registry.entities import ClimateDesc
-from tests.test_subdevice_discovery import ENTRY_DATA, _discover
+from tests.test_subdevice_discovery import ENTRY_DATA, _climate_bound, _discover
 
 FIXTURE = "airconditioner_tp1x_fac_time_23k"
 CONVENIENT = "/mode/convenient/vs/0"
@@ -50,7 +50,7 @@ QUIET_REP = {MODES_FIELD: "Quiet", SUPPORTED_FIELD: ADVERTISED}
 
 def test_learns_a_current_mode_missing_from_the_supported_list():
     learned = LearnedModes()
-    assert learned.observe(CONVENIENT, CONVENIENT, QUIET_REP) is True
+    assert learned.observe(CONVENIENT, QUIET_REP) == ["Quiet"]
     assert learned.codes(CONVENIENT) == ["Quiet"]
 
 
@@ -58,15 +58,15 @@ def test_relearning_the_same_mode_is_not_a_change():
     """The device reports the same rep on every poll while it sits in the
     mode, so only the first one may report back as something to persist."""
     learned = LearnedModes()
-    assert learned.observe(CONVENIENT, CONVENIENT, QUIET_REP) is True
+    assert learned.observe(CONVENIENT, QUIET_REP) == ["Quiet"]
-    assert learned.observe(CONVENIENT, CONVENIENT, QUIET_REP) is False
+    assert learned.observe(CONVENIENT, QUIET_REP) == []
     assert learned.codes(CONVENIENT) == ["Quiet"]
 
 
 def test_an_advertised_mode_is_never_learned():
     learned = LearnedModes()
     rep = {MODES_FIELD: "Sleep", SUPPORTED_FIELD: ADVERTISED}
-    assert learned.observe(CONVENIENT, CONVENIENT, rep) is False
+    assert learned.observe(CONVENIENT, rep) == []
     assert learned.codes(CONVENIENT) == []
 
 
@@ -75,26 +75,23 @@ def test_a_rep_with_no_supported_list_teaches_nothing():
     exists -- a board publishing none would otherwise get an option list
     invented out of whatever it happened to be doing."""
     learned = LearnedModes()
-    assert learned.observe(CONVENIENT, CONVENIENT, {MODES_FIELD: "Quiet"}) is False
+    assert learned.observe(CONVENIENT, {MODES_FIELD: "Quiet"}) == []
     assert learned.codes(CONVENIENT) == []
 
 
-def test_an_href_outside_the_allowlist_learns_nothing():
+def test_the_allowlist_is_only_the_convenient_href():
     """The guard that keeps this feature off resources whose current value
-    isn't a selectable option: an oven idling in 'NoOperation' reports
+    isn't a selectable option -- an oven idling in 'NoOperation' on
-    exactly this shape on /mode/vs/0, and remembering it would put a
+    /mode/vs/0, whose codes would become permanent junk options in that
-    permanent junk option in that unit's cook-mode select."""
+    unit's cook-mode select."""
-    learned = LearnedModes()
+    assert set(LEARNABLE) == {CONVENIENT}
-    rep = {MODES_FIELD: "NoOperation", SUPPORTED_FIELD: ["Bake", "Broil"]}
-    assert learned.observe("/mode/vs/0", "/mode/vs/0", rep) is False
-    assert learned.snapshot() == {}
 
 
 def test_two_subdevices_learn_separately():
     """Keyed by the actual on-the-wire href (issue #177), so a composite
     appliance's second indoor unit doesn't inherit the first's gap."""
     learned = LearnedModes()
-    learned.observe(CONVENIENT, "/mode/convenient/vs/1", QUIET_REP)
+    learned.observe("/mode/convenient/vs/1", QUIET_REP)
     assert learned.codes("/mode/convenient/vs/1") == ["Quiet"]
     assert learned.codes(CONVENIENT) == []
 
@@ -130,7 +127,9 @@ def test_clear_forgets_everything():
 def test_every_learnable_href_is_one_climate_resolves():
     """climate._supported is the only consumer that unions learned codes
     in today, so an href added to LEARNABLE that climate never reads would
-    be learned, persisted, and never offered anywhere."""
+    be learned, persisted, and never offered anywhere. The coordinator
+    enforces the device half of this (only hrefs a climate entity binds
+    are learnable); this is the static half."""
     from custom_components.localthings.climate import (
         CONVENIENT_HREF,
         MODE_HREF,
@@ -171,8 +170,7 @@ async def _flush(hass: HomeAssistant) -> None:
 async def _climate(hass: HomeAssistant, entry) -> tuple[LocalThingsCoordinator, Any]:
     coordinator = LocalThingsCoordinator(hass, entry)
     await _discover(coordinator, FIXTURE)
-    bound = next(b for b in coordinator.bound if isinstance(b.desc, ClimateDesc))
+    return coordinator, LocalThingsClimate(coordinator, _climate_bound(coordinator, None))
-    return coordinator, LocalThingsClimate(coordinator, bound)
 
 
 async def test_the_fixture_really_does_not_advertise_quiet(hass: HomeAssistant):
@@ -231,6 +229,23 @@ async def test_a_learned_preset_survives_a_restart(hass: HomeAssistant):
     assert "quiet" in entity.preset_modes
 
 
+async def test_a_device_with_no_climate_entity_learns_nothing(hass: HomeAssistant):
+    """The href alone isn't a sufficient key: the dehumidifier registry
+    declares this same /mode/convenient/vs/0 explicitly unmodeled (no live
+    current-value field), so a code learned there would be persisted for a
+    resource nothing will ever offer."""
+    entry = _entry(hass)
+    coordinator = LocalThingsCoordinator(hass, entry)
+    await _discover(coordinator, "dehumidifier")
+    assert not any(isinstance(b.desc, ClimateDesc) for b in coordinator.bound)
+
+    coordinator._observe.apply(
+        CONVENIENT, {MODES_FIELD: "Quiet", SUPPORTED_FIELD: ADVERTISED}, source="poll"
+    )
+
+    assert coordinator.learned_snapshot() == {}
+
+
 async def test_an_optimistic_write_teaches_nothing(hass: HomeAssistant):
     """An optimistic cache entry is the value this integration just wrote,
     not something the device reported."""