laundry: apply cleanup review to the cloud-cycle branch

Four parallel reviews (reuse, simplification, efficiency, altitude). The two
that change behavior:

- observe() could report "changed" on every poll forever, rewriting the
  config entry each time. If both tokens name the same slot with different
  payloads -- a downloaded program with its settings tweaked for one run is
  exactly that shape -- each pass wrote the default's blob then the one-shot's
  over it, so neither was ever already stored. On the SD-card installs this
  integration runs on, sustained entry rewrites are the one cost here that
  bites. The end state is stable, so "changed" is now start-vs-end, not
  per-assignment.
- The write path copied every tracked href to read one rep, walking past the
  accessor added to avoid exactly that. New entity_rep() does the merge for a
  single href; cycle_write drops the resources parameter it never used.

Structure:

- device_resources() is a second accessor giving the pure device view, used
  by diagnostics and the debug read service. That deletes strip_synthetic,
  the _SYNTHETIC_KEY_PREFIX convention and the redact filter added last
  commit: "a dump is what the device said" is now which method you call
  rather than something every future exporter has to remember.
- apply_cloud_courses() is the single mutation path. The flow was reaching
  past the coordinator into the store and relying on a later call to persist
  and invalidate for it; nine names are also now one entry write, not nine.
- option_value/hex_pairs move to capabilities/common.py. The duplicate's
  stated reason -- that the coordinator shouldn't import from
  registry.capabilities -- was simply false; it already does, and so does
  learned.py. The real constraint is narrower: laundry.py imports
  cloudcourse, so the reverse would be a cycle.

Dropped rather than kept:

- The cloud-vs-translated-local-course name check, and catalog.
  translated_state_labels with it. The catalog this process can read is
  English while the dropdown is localized in the frontend, so it rejected
  "Cotton" for a German user seeing "Baumwolle" and missed the real collision
  when they typed "Baumwolle" -- wrong in both directions outside one locale,
  against an outcome option ordering already makes deterministic. The checks
  that survive compare strings that are the same in every locale: the user's
  own names, and the device's personal-course labels.
- stored(), clear()/forget_cloud_courses(), blob(), download_course() -- no
  production callers. stored() was a template artifact whose docstring
  described a caller that cannot exist here.

Diagnostics gains a cloud_courses block, which the store was missing next to
learned_modes -- payloads and which slots are named, but not the names
themselves, since those are the user's words and dumps get pasted publicly.

Kept against one reviewer's advice: option_tokens (two others called
generalizing option_write the right direction) and select._display's
uncatalogued branch, which names a condition the old fallback-is-None proxy
only got right by accident. Deferred: making the store per-subdevice. It is
MAIN-only today and no device seen advertises cloud programs elsewhere; the
limitation is now documented where it is made.
This commit is contained in:
Marc Billow
2026-08-10 03:30:23 +00:00
parent b921bdbb28
commit 2265c52c77
12 changed files with 220 additions and 247 deletions
-18
View File
@@ -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)}
+39 -47
View File
@@ -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):
"""`<prefix>_<value>` 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]:
+28 -38
View File
@@ -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()
+61 -21
View File
@@ -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 (/<uuid>/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
+17 -2
View File
@@ -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,
@@ -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 `<prefix>_<value>` in an options[] array and return <value>.
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 `<Prefix>_<Value>` tokens into a cached
x.com.samsung.da.options[]-style array the same way the device itself
@@ -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 `<prefix>_<value>` in the options array and return <value>."""
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
@@ -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]
+14 -22
View File
@@ -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()
+4 -8
View File
@@ -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.
+8 -8
View File
@@ -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()
+23 -37
View File
@@ -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() == {}