diff --git a/custom_components/localthings/catalog.py b/custom_components/localthings/catalog.py index 9020e57..68b6208 100644 --- a/custom_components/localthings/catalog.py +++ b/custom_components/localthings/catalog.py @@ -48,21 +48,3 @@ def translated_states(platform: str, translation_key: str) -> frozenset[str]: """ entry = _ENTITY_CATALOG.get(platform, {}).get(translation_key) return frozenset(entry.get("state", ())) if entry else frozenset() - - -def translated_state_labels(platform: str, translation_key: str) -> dict[str, str]: - """`state key -> English label` for `platform`.`translation_key`. - - The labels behind translated_states, for the one caller that has to - compare against what a user actually reads rather than which codes are - translated: the download-cycle naming step rejects a name that would be - indistinguishable from a local course in the same dropdown. English only, - matching this catalog -- a name unique here can still collide in another - locale, which the select's local-courses-first ordering resolves toward - the local course. - """ - entry = _ENTITY_CATALOG.get(platform, {}).get(translation_key) - states = entry.get("state") if entry else None - if not isinstance(states, dict): - return {} - return {code: label for code, label in states.items() if isinstance(label, str)} diff --git a/custom_components/localthings/cloudcourse.py b/custom_components/localthings/cloudcourse.py index 3d4af4a..01afffa 100644 --- a/custom_components/localthings/cloudcourse.py +++ b/custom_components/localthings/cloudcourse.py @@ -52,6 +52,7 @@ from __future__ import annotations import threading from .const import CONF_CLOUD_COURSES +from .registry.capabilities.common import hex_pairs, option_value COURSE_HREF = "/course/vs/0" @@ -94,33 +95,29 @@ def _hex_bytes(blob): int(blob, 16) except ValueError: return [] - return [blob[i : i + 2].upper() for i in range(0, len(blob), 2)] + return hex_pairs(blob.upper()) def is_loaded(blob) -> bool: """True when `blob` names an actual program rather than 'none'.""" - parts = _hex_bytes(blob) - return bool(parts) and not blob.upper().startswith(_SENTINEL_PREFIX) + return _slot_and_loaded(blob)[1] def slot_of(blob) -> str | None: """The slot id `blob` belongs to, or None if it names no program.""" + slot, loaded = _slot_and_loaded(blob) + return slot if loaded else None + + +def _slot_and_loaded(blob) -> tuple[str | None, bool]: + """Both answers off one parse -- the public pair above needs the same + byte split, and observe() asks for both about the same payload.""" parts = _hex_bytes(blob) - if not parts or not is_loaded(blob): - return None - return parts[_SLOT_BYTE] - - -def option_value(options, prefix): - """`_` from an options[] array. Duplicated from - laundry.option_value rather than imported: this module is imported by - the coordinator, and reaching into registry.capabilities from there - would invert the dependency direction the rest of the integration - keeps.""" - for o in options or []: - if isinstance(o, str) and o.startswith(prefix + "_"): - return o.split("_", 1)[1] - return None + if not parts: + return None, False + if "".join(parts[:2]) == _SENTINEL_PREFIX: + return None, False + return parts[_SLOT_BYTE], True def advertised_slots(rep) -> list[str]: @@ -131,10 +128,9 @@ def advertised_slots(rep) -> list[str]: raw = option_value(rep.get("x.com.samsung.da.options"), EXTRA_PREFIX) if not isinstance(raw, str) or len(raw) % 2: return [] - slots = [raw[i : i + 2].upper() for i in range(0, len(raw), 2)] # Preserve the device's own order (first-seen wins) while dropping any # repeat, so the flow lists slots the way the appliance does. - return list(dict.fromkeys(slots)) + return list(dict.fromkeys(hex_pairs(raw.upper()))) def supports_cloud_courses(rep) -> bool: @@ -169,13 +165,6 @@ def _coerce(stored) -> tuple[str | None, dict[str, dict[str, str]]]: return download, slots -def stored(entry) -> dict: - """What `entry` has persisted, coerced -- for a reader with no - coordinator to go through (the options flow, on an unloaded entry).""" - download, slots = _coerce(entry.data.get(CONF_CLOUD_COURSES)) - return {"download_course": download, "slots": slots} - - def persist(hass, entry, record: dict) -> None: """Write `record` onto the entry. Runs on the event loop, which async_update_entry requires.""" @@ -230,8 +219,20 @@ class CloudCourses: if not options: return False known_slots = advertised_slots(rep) - changed = False + oneshot = option_value(options, ONESHOT_PREFIX) with self._lock: + # Compared once, at the end, against where this pass started -- + # not set per assignment. The two tokens can name the same slot + # with different payloads (a downloaded program with its settings + # tweaked for one run is exactly that shape), and a per-assignment + # flag would then report a change on every single poll forever: + # each pass writes the default's payload and then the one-shot's + # over it, so neither is ever "already stored". Every one of those + # reports rewrites the config entry, which on the SD-card installs + # this integration runs on is the one cost here that really bites. + # The end state is stable (the one-shot is written last and wins), + # so comparing start to end settles after the first pass. + before = {slot: record["blob"] for slot, record in self._slots.items()} for prefix in (DEFAULT_PREFIX, ONESHOT_PREFIX): blob = option_value(options, prefix) slot = slot_of(blob) @@ -242,11 +243,10 @@ class CloudCourses: record = self._slots.get(slot) if record is None: self._slots[slot] = {"blob": blob.upper(), "name": ""} - changed = True - elif record["blob"] != blob.upper(): + else: record["blob"] = blob.upper() - changed = True - oneshot = option_value(options, ONESHOT_PREFIX) + changed = before != {slot: rec["blob"] for slot, rec in self._slots.items()} + course = option_value(options, COURSE_PREFIX) if course and is_loaded(oneshot) and oneshot != self._last_oneshot: self._candidates[course] = self._candidates.get(course, 0) + 1 @@ -255,10 +255,6 @@ class CloudCourses: # -- reads ------------------------------------------------------------ - def download_course(self) -> str | None: - with self._lock: - return self._download_course - def download_candidates(self) -> list[str]: """Course codes seen at the moment a one-time program was loaded, most-observed first -- what the options flow offers as the likely @@ -267,11 +263,6 @@ class CloudCourses: ranked = sorted(self._candidates.items(), key=lambda kv: (-kv[1], kv[0])) return [code for code, _ in ranked] - def blob(self, slot: str) -> str | None: - with self._lock: - record = self._slots.get(slot.upper()) - return record["blob"] if record else None - def named(self) -> dict[str, str]: """Slots that are both learned and named -- the only ones offerable as a cycle option. An unnamed slot has no label that isn't either @@ -299,7 +290,7 @@ class CloudCourses: "programs": { slot: {"blob": record["blob"], "name": record["name"]} for slot, record in self._slots.items() - if record["name"] + if self._is_usable(record) }, } @@ -315,11 +306,12 @@ class CloudCourses: if record is not None: record["name"] = name.strip() - def clear(self) -> None: - with self._lock: - self._download_course = None - self._slots = {} - self._candidates = {} + @staticmethod + def _is_usable(record) -> bool: + """A slot is offerable once it has a name. The device supplies the + payload; only the user can supply the label, so this is the whole + rule and it is stated once.""" + return bool(record["name"]) def undiscovered(rep: dict, record: dict) -> list[str]: diff --git a/custom_components/localthings/config_flow.py b/custom_components/localthings/config_flow.py index b52f93f..1f196a5 100644 --- a/custom_components/localthings/config_flow.py +++ b/custom_components/localthings/config_flow.py @@ -35,7 +35,6 @@ from homeassistant.helpers.selector import ( ) from . import cloudcourse -from .catalog import translated_state_labels from .const import ( CLIENTHELLO_PROBE_RETRIES, CLIENTHELLO_PROBE_TIMEOUT_S, @@ -945,21 +944,31 @@ class LocalThingsOptionsFlow(config_entries.OptionsFlow): errors = self._apply_cloud_course_names(coord, known, user_input) if not errors: return self.async_create_entry(data=dict(self.config_entry.options)) - return self._cloud_courses_form(coord, rep, known, advertised, errors=errors) + return self._cloud_courses_form(coord, known, advertised, errors=errors) - return self._cloud_courses_form(coord, rep, known, advertised) + return self._cloud_courses_form(coord, known, advertised) def _apply_cloud_course_names(self, coord, known, user_input) -> dict[str, str]: """Validate and store the submitted names + Download course code. - A name that collides with any other entry in the cycle select -- - another download cycle, or one of the appliance's own local course - names -- is rejected rather than silently accepted. The select maps a - chosen label back to a raw value by matching display text, so two - options sharing a label would resolve to whichever comes first. + The select maps a chosen label back to a raw value by matching display + text, so two options sharing a label resolve to whichever comes first. + Two sources of collision are checkable here and both are rejected: + the user's own names against each other, and against the appliance's + personal-course labels, which the device reports verbatim and the + select renders as-is. + + A collision with a *translated* local course name is deliberately not + checked. The catalog this process can read is English (catalog.py), + while what the user actually sees is localized in the frontend -- so + checking it would reject "Cotton" for a German user whose dropdown + says "Baumwolle", and still miss the real collision when they type + "Baumwolle". Wrong in both directions outside one locale, against an + outcome the option ordering already makes deterministic (local + courses come first, so a shared label resolves to the real cycle). """ names = {slot: str(user_input.get(f"name_{slot}", "")).strip() for slot in known} - taken = {name.casefold() for name in self._local_course_names(coord)} + taken = {name.casefold() for name in self._device_course_names(coord)} for name in names.values(): if not name: continue @@ -975,42 +984,23 @@ class LocalThingsOptionsFlow(config_entries.OptionsFlow): if course is not None and course not in cycle_options(coord.canonical_resources(MAIN)): return {"base": "cloud_course_unknown_course"} - for slot, name in names.items(): - coord.cloud_courses.set_name(slot, name) - coord.set_cloud_download_course(course) + coord.apply_cloud_courses(names, course) return {} - def _local_course_names(self, coord) -> set[str]: - """Display names of this appliance's own local courses. + def _device_course_names(self, coord) -> set[str]: + """Course names this appliance reports itself. - Read through the same two sources the select renders from -- the - translation catalog, and the device's own personal-course labels - (laundry.washer_cycle_fallback) -- so the check matches what the user - will actually see side by side in the dropdown. A code neither source - names has no display name to collide with. + Only the personal-course labels: the device sends these as text and + the select renders them unchanged, so they are the same string in + every locale and can be compared against safely. See the caller for + why translated course names are not included. """ - bound = next( - (b for b in coord.bound if b.desc.key == "cycle" and b.href == cloudcourse.COURSE_HREF), - None, - ) - if bound is None: - return set() - resources = coord.canonical_resources(bound.subdevice) - key = bound.desc.translation_key - if callable(key): - key = key(resources) - labels = translated_state_labels("select", key) if key else {} + resources = coord.canonical_resources(MAIN) personal = personal_course_labels(resources) - names = set() - for code in cycle_options(resources): - if (catalogued := labels.get(code.lower())) is not None: - names.add(catalogued) - if (own := personal.get(code.upper())) is not None: - names.add(own) - return names + return {name for code in cycle_options(resources) if (name := personal.get(code.upper()))} def _cloud_courses_form( - self, coord, rep, known, advertised, errors: dict[str, str] | None = None + self, coord, known, advertised, errors: dict[str, str] | None = None ) -> ConfigFlowResult: store = coord.cloud_courses record = store.snapshot() diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index 3121431..c2de6da 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -325,14 +325,22 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): onto /course/vs/0 under cloudcourse.FIELD (issue #342). Merged at read time rather than applied to the state cache, so the - synthetic field can never be polled over, written to the device, or - reach a diagnostics dump -- `last_resources` stays exactly what the - appliance reported. It rides on the rep instead of a resource of its - own because rep_fn receives only its own href's rep: a sibling href - would be invisible to it, and /course/vs/0 is the one resource every - consumer of this data is already bound to. + synthetic field can never be polled over or written to the device -- + `last_resources` stays exactly what the appliance reported. It rides + on the rep instead of a resource of its own because rep_fn receives + only its own href's rep: a sibling href would be invisible to it, and + /course/vs/0 is the one resource every consumer of this data is + already bound to. + + MAIN only, by construction: cloudcourse.COURSE_HREF is a canonical + href and this snapshot is keyed by actual ones, so a composite + appliance's second course resource (//course/vs/0 on the + one-body washer-dryer) is not merged and not learned from. No device + seen so far advertises cloud programs on anything but MAIN; making + this per-subdevice means keying the store by actual href the way + LearnedModes does, and migrating the persisted shape. """ - snapshot = self._cache.snapshot() + snapshot = self.last_resources rep = snapshot.get(cloudcourse.COURSE_HREF) if rep is None: return snapshot @@ -342,6 +350,30 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): snapshot[cloudcourse.COURSE_HREF] = {**rep, cloudcourse.FIELD: view} return snapshot + def entity_rep(self, href: str) -> dict: + """One href's rep as descriptors see it -- `resource()` plus the + merge `entity_resources` would have applied. Exists so the write path + doesn't copy every tracked href to read one rep, which is the very + thing `resource()` was added to avoid.""" + rep = self.resource(href) + if href != cloudcourse.COURSE_HREF or not rep: + return rep + view = self._cloud.view() + return {**rep, cloudcourse.FIELD: view} if view else rep + + def device_resources(self, subdevice: Subdevice) -> dict[str, dict]: + """`subdevice`'s canonical view of exactly what the appliance + reported -- no integration state merged in. + + The counterpart to canonical_resources for everything that *exports* + resources rather than rendering entities from them: diagnostics and + the debug read service. Keeping this a separate call rather than + filtering the merged view downstream is what makes "a dump is what the + device said" a property of which method you call, instead of a + convention every future exporter has to remember. + """ + return canonical_view(subdevice, self.last_resources, self.subdevices) + def canonical_resources(self, subdevice: Subdevice) -> dict[str, dict]: """`subdevice`'s view of the live snapshot, rewritten to canonical hrefs (issue #177, see subdevices.canonical_view). Any platform @@ -466,23 +498,30 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): @property def cloud_courses(self) -> CloudCourses: - """The discovered-program store, for the options flow.""" + """The discovered-program store, for reads (snapshot/view/candidates). + + Mutations go through apply_cloud_courses below, not through this -- + the store itself doesn't persist or invalidate, and `_canonical_cache` + now depends on its contents, so a caller that mutates it directly + leaves entity options stale with no error to say so. + """ return self._cloud def cloud_course_rep(self) -> dict: """/course/vs/0's live rep -- what advertises the slot list.""" return self.resource(cloudcourse.COURSE_HREF) - def set_cloud_course_name(self, slot: str, name: str) -> None: - self._cloud.set_name(slot, name) - self._persist_cloud_courses() + def apply_cloud_courses(self, names: dict[str, str], download_course: str | None) -> None: + """The one mutation path for the cloud-program store (issue #342). - def set_cloud_download_course(self, code: str | None) -> None: - self._cloud.set_download_course(code) - self._persist_cloud_courses() - - def forget_cloud_courses(self) -> None: - self._cloud.clear() + Takes the whole submission at once so a nine-program naming pass is + one config-entry write rather than nine, and so persistence, the + canonical-view invalidation and the Repairs refresh can't be done for + one half of a change and skipped for the other. + """ + for slot, name in names.items(): + self._cloud.set_name(slot, name) + self._cloud.set_download_course(download_course) self._persist_cloud_courses() @callback @@ -1316,11 +1355,12 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): if write_fn is None: return href = bound_entity.href - # Through entity_resources(), not the bare cache: write_fn must see - # the same rep exists_fn/rep_fn were handed, including the merged + # Through entity_rep(), not the bare cache: write_fn must see the + # same rep exists_fn/rep_fn were handed, including the merged # cloud-program field (issue #342). Identical to the cache entry for - # every href that field doesn't touch. - rep = self.entity_resources().get(href or "") or {} + # every href that field doesn't touch, and without copying the whole + # tree to read one rep. + rep = self.entity_rep(href or "") # The remote-control gate below keys off the raw on-the-wire href # and a raw snapshot -- /remotectrl/* is a shared, MAIN-only # resource that a subdevice's canonical_resources() view (owned diff --git a/custom_components/localthings/diagnostics.py b/custom_components/localthings/diagnostics.py index bce3278..15fdd8a 100644 --- a/custom_components/localthings/diagnostics.py +++ b/custom_components/localthings/diagnostics.py @@ -16,6 +16,7 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant from homeassistant.loader import async_get_integration +from . import cloudcourse from .const import DOMAIN from .coordinator import LocalThingsCoordinator from .registry.redact import redact_resources @@ -32,6 +33,7 @@ async def async_get_config_entry_diagnostics( # disk (listdir + open + read_text), which trips HA's event-loop blocking # detector when called inline here. Offload it to the executor. stl_version = await hass.async_add_executor_job(pkg_version, "smartthings-local") + cloud_courses = coordinator.cloud_courses.snapshot() # /oic/p, /oic/d, and /oic/res sit outside the /device/0 batch captured # below, so they'd otherwise never reach an issue report. /oic/d's `rt` @@ -54,7 +56,7 @@ async def async_get_config_entry_diagnostics( # than redacting /information/vs/0 again -- modelNum never matches # redact.py's substring rules, so the value is the same either way. matching = [b for b in coordinator.bound if b.subdevice == su] - res = redact_resources(coordinator.canonical_resources(su)) + res = redact_resources(coordinator.device_resources(su)) return { "kind": su.kind, "key": su.key, @@ -88,7 +90,7 @@ async def async_get_config_entry_diagnostics( # /mode/vs/0 under no attribution. Each sibling reports its own # resources in `subdevices` below instead. For a device with no # subdevices, this is byte-identical to `last_resources`. - "resources": redact_resources(coordinator.canonical_resources(MAIN)), + "resources": redact_resources(coordinator.device_resources(MAIN)), # Sibling indoor subdevices discovered on this connection (issue # #177). subdeviceIdList (the UUID a prefixed subdevice's key comes # from) is deliberately NOT redacted here, unlike elsewhere in @@ -137,6 +139,19 @@ async def async_get_config_entry_diagnostics( "enabled": coordinator.learning_enabled, "codes": coordinator.learned_snapshot(), }, + # Cloud "Download" programs discovered on this device (issue #342), + # reported separately for the same reason as learned_modes above. + # Payloads are the useful part for triage -- they are the only record + # of what a downloaded program contains. The names are not included: + # they are the user's own words, and a dump gets pasted into public + # issues. Which slots are named is still visible, which is all the + # triage question ("is this set up?") actually needs. + "cloud_courses": { + "advertised_slots": cloudcourse.advertised_slots(coordinator.cloud_course_rep()), + "download_course": cloud_courses["download_course"], + "payloads": {slot: rec["blob"] for slot, rec in cloud_courses["slots"].items()}, + "named_slots": sorted(s for s, rec in cloud_courses["slots"].items() if rec["name"]), + }, "integration_version": integration.version, "smartthings_local_version": stl_version, "observe_mode": coordinator.observe_mode, diff --git a/custom_components/localthings/registry/capabilities/common.py b/custom_components/localthings/registry/capabilities/common.py index 17a3057..dfab158 100644 --- a/custom_components/localthings/registry/capabilities/common.py +++ b/custom_components/localthings/registry/capabilities/common.py @@ -136,6 +136,28 @@ def _active_alarm_codes(items): return ", ".join(codes) if codes else "none" +def hex_pairs(codes): + """'1C1D21...' -> ['1C', '1D', '21', ...].""" + return [codes[i : i + 2] for i in range(0, len(codes) - 1, 2)] + + +def option_value(options, prefix): + """Find `_` in an options[] array and return . + + Lives here rather than in laundry.py, which is where it grew, because + cloudcourse.py needs it too and laundry.py imports *that* -- so the + reverse import would be a module cycle. The coordinator already imports + from this module, so nothing about the dependency direction is unusual; + it is specifically the laundry/cloudcourse pair that can't reach each + other. Anchored at position 0 so 'Course_' never matches + 'CloudCourse_'/'OneTimeCloudCourse_'. + """ + for o in options or []: + if isinstance(o, str) and o.startswith(prefix + "_"): + return o.split("_", 1)[1] + return None + + def merge_options_field(cached, new_tokens): """Merge freshly-written `_` tokens into a cached x.com.samsung.da.options[]-style array the same way the device itself diff --git a/custom_components/localthings/registry/capabilities/laundry.py b/custom_components/localthings/registry/capabilities/laundry.py index 19d69eb..db959e8 100644 --- a/custom_components/localthings/registry/capabilities/laundry.py +++ b/custom_components/localthings/registry/capabilities/laundry.py @@ -27,6 +27,7 @@ from ... import cloudcourse from ...catalog import has_entity_translation from ..capability import Capability from ..entities import NumberDesc, SelectDesc, SensorDesc, SwitchDesc, TimeDesc +from .common import hex_pairs, option_value _LED_LEVELS = ("Low", "High") _SOUND_MODES = ("voice", "tone", "mute") @@ -199,11 +200,6 @@ BUZZER_SOUND = Capability( # boards expose the same /course/vs/0 options contract. -def hex_pairs(codes): - """'1C1D21...' -> ['1C', '1D', '21', ...].""" - return [codes[i : i + 2] for i in range(0, len(codes) - 1, 2)] - - def parse_edit_course_list(raw): """'EditCourseList_1C1D21...' -> ['1C', '1D', '21', ...].""" if not isinstance(raw, str) or "_" not in raw: @@ -219,14 +215,6 @@ def cycle_options(resources): return _course_codes_from_supported_options(resources.get("/course/vs/0") or {}) -def option_value(options, prefix): - """Find `_` in the options array and return .""" - for o in options or []: - if isinstance(o, str) and o.startswith(prefix + "_"): - return o.split("_", 1)[1] - return None - - # Drum Clean+ maintenance tracking, from the same options[] array as the # selected course -- shared by washer.py (issue #9) and dryer.py (issue # #258), identical DrumCleanProposal_/WashingTimes_/DrumCleanLog_ tokens. @@ -404,7 +392,7 @@ def cloud_current(rep): return f"{cloudcourse.RAW_PREFIX}{slot}" -def cycle_write(p, rep, href=None, resources=None): +def cycle_write(p, rep, href=None): if not rep.get("x.com.samsung.da.options"): return None if isinstance(p, str) and p.startswith(cloudcourse.RAW_PREFIX): @@ -521,7 +509,7 @@ def cycle_select(*, translation_key, icon, table_href=None, display_fn=None): return candidate if has_entity_translation("select", candidate) else "cycle" def options(resources): - rep = resources.get("/course/vs/0") or {} + rep = resources.get(cloudcourse.COURSE_HREF) or {} # Local courses first: a user-supplied cloud name that happens to # match a translated course name resolves back to the real local # course on write, which is the safer of the two. The options flow diff --git a/custom_components/localthings/registry/redact.py b/custom_components/localthings/registry/redact.py index e9fd5ca..1ed8e47 100644 --- a/custom_components/localthings/registry/redact.py +++ b/custom_components/localthings/registry/redact.py @@ -39,14 +39,6 @@ _SENSITIVE_SUBSTRINGS = ( _SENSITIVE_EXACT = frozenset({"di", "pi", "n"}) -# Fields this integration merges onto a rep for its own use, which the -# device never reported (see coordinator.entity_resources). A diagnostics -# dump is meant to be exactly what the appliance said, so these are dropped -# rather than redacted -- keeping them would both misrepresent the device and -# publish data the user typed (cloud program names are user-supplied). -_SYNTHETIC_KEY_PREFIX = "x.localthings." - - def _is_sensitive_key(key: str) -> bool: lowered = key.lower() if lowered in _SENSITIVE_EXACT: @@ -54,29 +46,8 @@ def _is_sensitive_key(key: str) -> bool: return any(s in lowered for s in _SENSITIVE_SUBSTRINGS) -def strip_synthetic(resources): - """Drop this integration's own merged-in fields, leaving only what the - appliance actually reported. - - Separate from redact_resources because the two answer different - questions: the debug read service wants the device's unredacted state - (serial and all -- that is the point of it) but still shouldn't present - our own bookkeeping as something the device said. - """ - if isinstance(resources, dict): - return { - key: strip_synthetic(value) - for key, value in resources.items() - if not key.startswith(_SYNTHETIC_KEY_PREFIX) - } - if isinstance(resources, list): - return [strip_synthetic(item) for item in resources] - return resources - - def redact_resources(resources): - """Recursively redact dict values whose key matches a sensitive substring, - and drop this integration's own synthetic fields entirely. + """Recursively redact dict values whose key matches a sensitive substring. Works on the shape produced by parse_device0_batch (dict[href, rep]) or any nested dict/list structure within a rep. @@ -85,7 +56,6 @@ def redact_resources(resources): return { key: (REDACTED if _is_sensitive_key(key) else redact_resources(value)) for key, value in resources.items() - if not key.startswith(_SYNTHETIC_KEY_PREFIX) } if isinstance(resources, list): return [redact_resources(item) for item in resources] diff --git a/custom_components/localthings/select.py b/custom_components/localthings/select.py index d405cd5..1038f00 100644 --- a/custom_components/localthings/select.py +++ b/custom_components/localthings/select.py @@ -67,28 +67,20 @@ def _display(value, translation_key: str | None, fallback_fn=None): """ if not isinstance(value, str): return value - # No state table for this key: either the entity isn't translated at all, - # or its name is translated but its options deliberately aren't (an - # unrecognized course table, say). - uncatalogued = False - if translation_key: - known = translated_states("select", translation_key) - if not known: - uncatalogued = True - elif translated := _translation_state(value, known): - return translated - if fallback_fn is not None: - fallback = fallback_fn(value) - if fallback is not None: - return fallback - if uncatalogued: - # Nothing could name this value: not the catalog, and not a - # device-specific fallback (either absent, or present and declining - # to label this one). The raw device value is the best choice -- - # cosmetic reshaping below would only mangle an opaque code, turning - # a course '0E' into '0 E'. Keyed on the fallback's *result*, not on - # whether one was supplied: cycle_select always supplies one now, to - # label cloud programs, and it returns None for everything else. + known = translated_states("select", translation_key) if translation_key else frozenset() + if translated := _translation_state(value, known): + return translated + if fallback_fn is not None and (fallback := fallback_fn(value)) is not None: + return fallback + if translation_key and not known: + # No state table for this key: either the entity isn't translated at + # all, or its name is translated but its options deliberately aren't + # (an unrecognized course table, say). Nothing named this value, so + # the raw device value is the best choice -- the cosmetic reshaping + # below would only mangle an opaque code, turning a course '0E' into + # '0 E'. Reached only when the fallback *declined* the value, not + # merely when none was supplied: cycle_select always supplies one now + # (it labels cloud programs) and returns None for everything else. return value if value.islower(): return value.replace("_", " ").title() diff --git a/custom_components/localthings/services.py b/custom_components/localthings/services.py index fa2c024..8220c6e 100644 --- a/custom_components/localthings/services.py +++ b/custom_components/localthings/services.py @@ -24,7 +24,6 @@ from homeassistant.helpers import device_registry as dr from .const import DOMAIN, SERVICE_READ_RESOURCE, SERVICE_WRITE_RESOURCE from .coordinator import LocalThingsCoordinator, normalize_href -from .registry.redact import strip_synthetic from .registry.subdevices import MAIN, Subdevice ATTR_HREF = "href" @@ -171,13 +170,10 @@ async def _async_read_resource(hass: HomeAssistant, call: ServiceCall) -> Servic # href: lets a user enumerate what exists without hammering the # device (see this module's docstring and the coordinator's # canonical_resources). - # Stripped, not redacted: this response is meant to be what the - # appliance reported (unredacted -- that is the point of a debug - # read), but canonical_resources also carries fields this integration - # merged on for its own use (see entity_resources). - snapshot: dict[str, Any] = { - "resources": strip_synthetic(coordinator.canonical_resources(subdevice)) - } + # device_resources, not canonical_resources: this response is what + # the appliance reported, without the fields this integration merges + # on for its own use (see coordinator.entity_resources). + snapshot: dict[str, Any] = {"resources": coordinator.device_resources(subdevice)} return cast(ServiceResponse, snapshot) # Same normalize-before-translate order as the write path above. diff --git a/tests/test_cloud_courses.py b/tests/test_cloud_courses.py index 599827d..a4cab65 100644 --- a/tests/test_cloud_courses.py +++ b/tests/test_cloud_courses.py @@ -90,8 +90,8 @@ class TestRealDumps: store = cloudcourse.CloudCourses() assert store.observe(rep) is True - assert store.blob("55") == SPORTS - assert store.blob("6B") == JEANS + assert store.snapshot()["slots"]["55"]["blob"] == SPORTS + assert store.snapshot()["slots"]["6B"]["blob"] == JEANS # Learned but unnamed -- nothing is offerable yet. assert store.named() == {} assert store.view() == {} @@ -102,7 +102,7 @@ class TestRealDumps: store.observe(rep) assert store.download_candidates() == ["87"] # A candidate is never used until confirmed. - assert store.download_course() is None + assert store.snapshot()["download_course"] is None def test_wa55_learns_its_saved_program_but_not_the_sentinel(self): rep = _load_device("washer_wa55a7700av")["/course/vs/0"] @@ -110,8 +110,8 @@ class TestRealDumps: store = cloudcourse.CloudCourses() store.observe(rep) - assert store.blob("59") == "001C590549164D114A224C2037F0AC22" - assert store.blob("01") is None # the FFFF sentinel's byte 2 + assert store.snapshot()["slots"]["59"]["blob"] == "001C590549164D114A224C2037F0AC22" + assert "01" not in store.snapshot()["slots"] # the FFFF sentinel's byte 2 def test_wa55_proposes_no_download_course(self): """It is sitting on an ordinary local course with no override @@ -160,7 +160,7 @@ class TestRealDumps: for blob in (b06c, b048): store = cloudcourse.CloudCourses() store.observe(_rep(["CloudExtraCourse_0A", f"CloudCourse_{blob}"])) - assert store.blob("0A") == blob + assert store.snapshot()["slots"]["0A"]["blob"] == blob def test_a_sentinel_slot_byte_is_not_the_current_course(self): """The two sentinels in the corpus disagree about this -- WA55's byte @@ -192,14 +192,14 @@ class TestStoreRules: offers.""" store = cloudcourse.CloudCourses() store.observe(_rep(["CloudExtraCourse_55", f"OneTimeCloudCourse_{JEANS}"])) - assert store.blob("6B") is None + assert "6B" not in store.snapshot()["slots"] def test_a_relearned_blob_replaces_the_old_payload(self): store = cloudcourse.CloudCourses() store.observe(_rep(["CloudExtraCourse_55", f"CloudCourse_{SPORTS}"])) rewritten = SPORTS.replace("F005F0AC00", "F005F0AC11") assert store.observe(_rep(["CloudExtraCourse_55", f"CloudCourse_{rewritten}"])) is True - assert store.blob("55") == rewritten + assert store.snapshot()["slots"]["55"]["blob"] == rewritten def test_observing_the_same_rep_twice_changes_nothing(self): store = cloudcourse.CloudCourses() diff --git a/tests/test_cloud_courses_flow.py b/tests/test_cloud_courses_flow.py index 272f7c2..3914d20 100644 --- a/tests/test_cloud_courses_flow.py +++ b/tests/test_cloud_courses_flow.py @@ -80,8 +80,8 @@ async def test_a_poll_learns_and_persists_the_loaded_programs(hass: HomeAssistan entry = _entry(hass) coordinator = await _coordinator(hass, entry) - assert coordinator.cloud_courses.blob("55") == SPORTS - assert coordinator.cloud_courses.blob("6B") == JEANS + assert coordinator.cloud_courses.snapshot()["slots"]["55"]["blob"] == SPORTS + assert coordinator.cloud_courses.snapshot()["slots"]["6B"]["blob"] == JEANS # Survives a restart -- the payload is only visible while loaded. assert entry.data[CONF_CLOUD_COURSES]["slots"]["55"]["blob"] == SPORTS @@ -96,7 +96,7 @@ async def test_an_optimistic_write_teaches_nothing(hass: HomeAssistant): source="optimistic", ) await _flush(hass) - assert coordinator.cloud_courses.blob("55") is None + assert "55" not in coordinator.cloud_courses.snapshot()["slots"] async def test_learned_but_unnamed_programs_stay_out_of_the_cycle_select(hass: HomeAssistant): @@ -109,8 +109,7 @@ async def test_learned_but_unnamed_programs_stay_out_of_the_cycle_select(hass: H async def test_naming_a_program_puts_it_in_the_cycle_select(hass: HomeAssistant): coordinator = await _coordinator(hass) - coordinator.set_cloud_download_course("87") - coordinator.set_cloud_course_name("55", "Sports") + coordinator.apply_cloud_courses({"55": "Sports"}, "87") await _flush(hass) assert "cloud:55" in _cycle_options(coordinator) @@ -118,7 +117,7 @@ async def test_naming_a_program_puts_it_in_the_cycle_select(hass: HomeAssistant) # Jeans is still unnamed -- so the state falls through to the raw course. assert _cycle_state(coordinator) == "87" - coordinator.set_cloud_course_name("6B", "Jeans") + coordinator.apply_cloud_courses({"6B": "Jeans"}, "87") await _flush(hass) assert _cycle_state(coordinator) == "cloud:6B" @@ -128,8 +127,7 @@ async def test_the_synthetic_field_never_reaches_the_device_snapshot(hass: HomeA appliance reported, so it can't be polled over, written back, or land in a diagnostics dump.""" coordinator = await _coordinator(hass) - coordinator.set_cloud_download_course("87") - coordinator.set_cloud_course_name("55", "Sports") + coordinator.apply_cloud_courses({"55": "Sports"}, "87") await _flush(hass) assert cloudcourse.FIELD in coordinator.entity_resources()[COURSE] @@ -142,8 +140,7 @@ async def test_selecting_a_named_program_writes_both_tokens(hass: HomeAssistant) command path builds its own rep, and a rep taken straight off the state cache carries no cloud programs, so the write would silently no-op.""" coordinator = await _coordinator(hass) - coordinator.set_cloud_download_course("87") - coordinator.set_cloud_course_name("55", "Sports") + coordinator.apply_cloud_courses({"55": "Sports"}, "87") await _flush(hass) sent: list[tuple[list[str], bytes]] = [] @@ -184,7 +181,6 @@ async def test_a_repair_is_raised_until_every_program_is_named(hass: HomeAssista assert issue is not None assert (issue.translation_placeholders or {})["total"] == "9" - coordinator.set_cloud_download_course("87") for slot in cloudcourse.advertised_slots(coordinator.cloud_course_rep()): # Only two are learned; name every advertised slot to close the gap. coordinator.cloud_courses.observe( @@ -195,7 +191,7 @@ async def test_a_repair_is_raised_until_every_program_is_named(hass: HomeAssista ] } ) - coordinator.set_cloud_course_name(slot, f"Program {slot}") + coordinator.apply_cloud_courses({slot: f"Program {slot}"}, "87") await _flush(hass) assert registry.async_get_issue(DOMAIN, issue_id) is None @@ -219,7 +215,7 @@ async def test_a_malformed_entry_record_does_not_block_setup(hass: HomeAssistant entry = _entry(hass, data={CONF_CLOUD_COURSES: {"slots": {"55": {"blob": "junk"}}}}) coordinator = await _coordinator(hass, entry) # Dropped on restore, then relearned from the live poll. - assert coordinator.cloud_courses.blob("55") == SPORTS + assert coordinator.cloud_courses.snapshot()["slots"]["55"]["blob"] == SPORTS # --------------------------------------------------------------------------- @@ -276,19 +272,6 @@ async def test_the_flow_rejects_two_programs_sharing_a_name(hass: HomeAssistant) assert coordinator.cloud_courses.named() == {} -async def test_the_flow_rejects_a_name_that_shadows_a_local_course(hass: HomeAssistant): - """'Drum Clean' is course 74 on this appliance's own list. Two options - rendering the same label would resolve to whichever comes first.""" - coordinator = await _coordinator(hass) - handler = await _options_handler(hass, coordinator) - - result = await handler.async_step_cloud_courses( - {"name_55": "Drum Clean", "download_course": "87"} - ) - assert result["errors"] == {"base": "cloud_course_name_duplicate"} - assert coordinator.cloud_courses.named() == {} - - async def test_clearing_a_name_removes_the_program_from_the_select(hass: HomeAssistant): coordinator = await _coordinator(hass) handler = await _options_handler(hass, coordinator) @@ -358,27 +341,30 @@ async def test_the_synthetic_field_never_reaches_diagnostics( entry = _entry(hass) coordinator = await _coordinator(hass, entry) - coordinator.set_cloud_download_course("87") - coordinator.set_cloud_course_name("55", "Marc's weekend towels") + coordinator.apply_cloud_courses({"55": "Marc's weekend towels"}, "87") await _flush(hass) hass.data.setdefault(DOMAIN, {})[entry.entry_id] = coordinator - dump = json.dumps(await async_get_config_entry_diagnostics(hass, entry)) + diag = await async_get_config_entry_diagnostics(hass, entry) + dump = json.dumps(diag) assert cloudcourse.FIELD not in dump - assert "Marc's weekend towels" not in dump - # The device's own tokens are still there -- only our field is dropped. + # The device's own tokens are still there -- resources stays what it said. assert "CloudExtraCourse_0A5C286B2D0C55301A" in dump + # The store is reported in its own block, so a triager can see the + # payloads -- but not the user's chosen names. + assert "Marc's weekend towels" not in dump + assert diag["cloud_courses"]["payloads"]["55"] == SPORTS + assert diag["cloud_courses"]["named_slots"] == ["55"] + assert diag["cloud_courses"]["download_course"] == "87" + async def test_the_debug_read_service_reports_only_device_state(hass: HomeAssistant): - from custom_components.localthings.registry.redact import strip_synthetic - coordinator = await _coordinator(hass) - coordinator.set_cloud_download_course("87") - coordinator.set_cloud_course_name("55", "Sports") + coordinator.apply_cloud_courses({"55": "Sports"}, "87") await _flush(hass) - stripped = strip_synthetic(coordinator.canonical_resources(MAIN)) + stripped = coordinator.device_resources(MAIN) assert cloudcourse.FIELD not in stripped[COURSE] assert "x.com.samsung.da.options" in stripped[COURSE] @@ -392,7 +378,7 @@ async def test_the_flow_rejects_a_download_course_the_device_does_not_offer( result = await handler.async_step_cloud_courses({"name_55": "Sports", "download_course": "FF"}) assert result["errors"] == {"base": "cloud_course_unknown_course"} - assert coordinator.cloud_courses.download_course() is None + assert coordinator.cloud_courses.snapshot()["download_course"] is None assert coordinator.cloud_courses.named() == {}