An independent review of the last two commits' diff turned up five real
problems and one CI-breaking one. Fixed all of them:
- tests/test_coordinator_error_handling.py assigned directly onto
coordinator instance attributes (coordinator._poll_once = dict), which
`ty check custom_components tests` -- what CI actually runs, not the
narrower `ty check custom_components` this branch had only been
spot-checked against -- flags as invalid-assignment. Switched to
monkeypatch.setattr, matching every other new test on this branch.
- coordinator.py's subdevice-enumeration failure comment claimed the
probe "retries naturally next cycle." It doesn't: _run_discovery sets
self._discovered = True unconditionally later in the same cycle, which
is what gates the whole block, so a failure here is a first-and-only
attempt, not a retried one -- a composite appliance's sibling
subdevices are missing for the config entry's lifetime until reload.
(A separate flag to retry wouldn't actually fix that either: every
platform's async_setup_entry enumerates coordinator.bound exactly
once, so a later-successful enumeration still couldn't add entities
without a reload.) Corrected the comment and raised debug to warning,
since the effect is silent and permanent otherwise.
- config_flow.py's new diagnostic-handshake fallback (_resolve_alert)
was being called once per failing candidate, inside _handshake_and_read's
scan loop -- contradicting _diagnostic_alert's own docstring ("only
runs once every real candidate has already failed"). Two real costs:
up to CLIENTHELLO_PROBE_TIMEOUT_S extra latency per failing port on
the sweep-fallback path (several candidates), and -- more seriously --
the diagnostic commits DTLS association state on each port it touches,
which can make _probe_and_validate's own CertRejected re-mint retry
(a fresh _handshake_and_read call against that same scan) time out
against the very port it just polluted, per the RFC 6347 §4.2.8
concern already documented elsewhere in this file. Moved to a new
_diagnose_failures helper called once, after the loop, against the
single best (confirmed-live) candidate.
- _resolve_alert also ignored the alert's level: ProbeResult.alert is
set for a received alert of either severity, but only a fatal one (2)
means the appliance broke off the handshake over it -- a warning
(e.g. close_notify) was being read as a rejection reason. Older
library exception text never had this ambiguity (OpenSSL only renders
an exception for a fatal alert), so this was a bug the redaction
fallback introduced. Now filters to level == 2.
- async_raw_read's new HomeAssistantError wrapper reported a translated
error but left a confirmed-dead session installed, so every
subsequent read/write would keep failing identically for up to the
next full poll interval. Now closes the session on any non-TimeoutError
failure, matching _poll_once's own posture.
- async_raw_write_sequence's verify_after fallback (vcode, vrep = 0, {})
is indistinguishable from a real 4.04 by raw_code alone. Added a
read_error field so a caller can tell "couldn't verify" from "the
device said no."
Tests: 8 new/extended (once-not-per-port diagnostic count, alert-level
filtering, the warning log + permanent-loss framing, session-closing on
both async_raw_read and the verify_after path, read_error surfacing).
Full suite (1527 tests), ruff, ruff format, and -- critically --
`ty check custom_components tests` (the CI-matching invocation) all pass.
Excessive for user-facing docs -- this is implementation detail
(_defer_reconnect_for's tolerance logic) that belongs in the
coordinator's own comments, where it still lives, not in "Known
device behavior". No functional change.
Answers a review callout on the 0.1.6 upgrade that wasn't actually
addressed, only mentioned in a commit message: a dead reader now raises
SessionClosedError (a ConnectionError) instead of hanging a request out
to its timeout and surfacing as an ambiguous TimeoutError. Mechanically
this was already routed correctly -- SessionClosedError isn't a
TimeoutError, so _defer_reconnect_for's isinstance check already skips
its multi-cycle tolerance for it -- but nothing recorded *why*, and the
callout's whole point was that downstream (i.e. this repo) should hear
about the resulting timing change explicitly, not infer it from the
dependency bump.
Before 0.1.6, a truly dead reader was indistinguishable here from a
slow blockwise transfer: both could only ever surface as a TimeoutError,
so _POLL_TIMEOUT_LIMIT's multi-cycle tolerance (~2 minutes at the
default 30s interval) was the only thing standing between a genuinely
dead session and a reconnect. Now that the library confirms reader
death directly, that failure mode skips the tolerance and reconnects on
the very first occurrence -- intended, and strictly faster recovery,
but a real change in observed timing worth calling out for anyone
correlating reconnect-log cadence with device behavior.
- _poll_once, _defer_reconnect_for, and the _POLL_TIMEOUT_LIMIT comment
now say so directly, cross-referencing each other.
- README's "Known device behavior" section gets a paragraph so this
isn't only visible to someone reading the coordinator's source.
- Two new unit tests pin the distinction directly:
_defer_reconnect_for(SessionClosedError()) is False (no tolerance),
while SessionTimeoutError keeps the existing _POLL_TIMEOUT_LIMIT
tolerance -- so a future change can't quietly merge the two paths
back together.
Full suite (1523 tests), ruff, and ty pass.
A follow-up review of the 0.1.6 upgrade found four call sites where a
library exception (new typed one or the old bare ConnectionError/
TimeoutError it replaced) could escape this integration's own
reconnect/logging or a service call's translation layer entirely,
instead of being handled the way equivalent failures already are
elsewhere in this file:
- _attempt_observe_mode's own _connect_session() reconnect (fires only
when the session was closed out from under it concurrently) had no
try/except, and neither did either of its two call sites in
_async_update_data. A failure there escaped uncaught: HA's
DataUpdateCoordinator has its own final safety net so nothing crashed
the config entry, but non-TimeoutError failures logged a full ERROR
traceback instead of this integration's deliberately quiet "poll
failed, reconnecting" voice, and skipped its own reconnect bookkeeping
entirely. Fixed by catching around just the connect call (the only
unguarded raise path in the method -- subscribe_hrefs/
await_observe_notifies already handle their own failures), landing in
the same "give up on push this cycle" state abandon_observe_attempt()
already produces for the subscribe-failed and stale-session branches.
Deliberately does not touch _close_session() (self._session is
already None here -- _connect_session only ever publishes it after a
full success), _reconnect_is_frequent() (that window records the poll
path's own reconnects; feeding it a secondary path's failure would
over-trigger its warning threshold), or _resubscribe_due (that flag
means "a live session nothing has tried yet" -- setting it here would
re-enter the doomed handshake every cycle instead of letting
_last_observe_attempt_ts pace the retry).
- _enumerate_subdevices_blocking's _connect_session() call (first
discovery only) had the same gap. Fixed the same way: log and fall
through on the resources _poll_once already returned this cycle,
rather than losing first discovery over a failed subdevice probe.
- async_raw_read (backing the read_resource service) had no exception
handling at all -- a session/network failure during a live debug read
reached the service caller as a raw, untranslated library exception,
unlike write_resource's equivalent path. Now wrapped the same way
async_send_command/async_raw_write_sequence already are, raising
HomeAssistantError with a new debug_read_failed translation key
(added to all 7 shipped locales).
- async_raw_write_sequence's verify_after tail sat outside the method's
own try/except, so a failed confirmation read discarded the write
results that had already landed by throwing past them. Now caught
per-href inside the verify loop instead: a failed read is treated the
same as a 4.04/empty one (held=None, "couldn't verify" -- not lost or
misreported as a revert), and the rest of the batch still gets
checked.
Design for the first fix (the trickiest -- it's mid-lock, and has to
interact correctly with observe-mode state and the poll path's own
bookkeeping without corrupting either) was worked through with a
dedicated review pass before implementing.
Tests: new coverage for all four (test_coordinator.py's
test_attempt_observe_mode_survives_a_failed_reconnect, a new
test_coordinator_error_handling.py for the subdevice-enumeration case,
and two additions to test_services.py for the read-service and
verify_after cases). Full suite (1521 tests), ruff, and ty all pass.
Bumps the smartthings-local floor from >=0.1.2 to >=0.1.6 (manifest,
requirements-dev.txt, Dockerfile) and adopts the interface/behavior
changes introduced along the way:
- 0.1.3 ("redacted typed failures", PR #23) replaced connect()'s
ConnectionError(f"DTLS handshake error: {e}") with fixed, redacted
exceptions (SessionError, SessionTimeoutError, etc.) that never carry
backend text -- including the TLS alert name. The config flow's
_classify_handshake_failure relied on parsing that text out of the
exception (_alert_name) to tell a rejected certificate from any other
handshake failure; against a current library that regex never matches
again, silently downgrading every setup failure to the generic
"cannot_connect" message.
Fixed by adding _resolve_alert: it still tries _alert_name first (a
harmless fallback if it ever matches), then falls back to one bounded
smartthings_local.protocol.dtls_probe.diagnose_dtls_handshake() call
against the specific port that failed, which classifies the fatal
Alert straight from the raw record instead of an exception string.
_handshake_and_read now threads the resolved per-port alerts into
_classify_handshake_failure, so CertRejected vs. HandshakeFailed keeps
working the way it did before the redaction.
- 0.1.3 also moved the DTLS session onto a connected UDP socket (see
endpoint.py's open_connected_udp_socket), which changes why
coordinator._local_source_port needs a unique port per device -- the
kernel now demuxes by the full local-port/remote-peer tuple instead of
relying on an unconnected recvfrom(). Docstring updated to match.
- 0.1.6's reader-thread fail-fast fix (_check_live/_reader_running) makes
a dead reader raise SessionClosedError immediately instead of hanging
a request out to its timeout. No code change needed: SessionClosedError
is a ConnectionError subclass (not TimeoutError), so the coordinator's
existing _defer_reconnect_for/isinstance(e, TimeoutError) split already
routes it to the immediate-reconnect path.
Every other new/changed piece (endpoint.py, dtls_probe.py bounded
probing, auth.py's CertificateAuth/PskAuth providers) stays behind
compatible built-in exception types and unchanged get()/post()/
subscribe()/ping() signatures, per the library's own compatibility
table, so the coordinator's and observe.py's broad exception handling
needed no changes.
Tests: added coverage for _resolve_alert's exception-text vs.
diagnostic-handshake fallback, _classify_handshake_failure with a
resolved alerts mapping, and an end-to-end config-flow re-mint test
against a FakeSession that raises the new redacted SessionError instead
of the old text-bearing ConnectionError.
Full suite (1517 tests), ruff, and ty all pass against smartthings-local
0.1.6 installed from PyPI.
queue_get accepts dict | list since #335's Collection test started
queueing a batch list, but _get_reps was still typed list[dict] --
ty flagged the list.append() as invalid. No behavior change.
`_raw_read_blocking` decoded the CBOR body and kept it only when it was a
Property map, so a Collection -- which answers the `[devcol rep, {href,
rep}, ...]` batch `parse_device0_batch` reads -- came back as `2.05` with
`rep: {}`. That renders as "the resource exists and has nothing in it",
which is the opposite of what a populated batch means, and `/device/0`
itself would have read the same way.
It cost a real result: issue #335's board answers `/sec/devices` (the
`x.com.samsung.devcol` sibling of `/device/0`, and the one remaining place
a composite appliance could be enumerating its indoor units) with exactly
that empty-looking 2.05, and it was nearly written off as a dead end.
The read path now returns the decoded body alongside `rep`, and the service
response carries it as `body` whenever it isn't the map `rep` already has --
omitted for the ordinary case rather than duplicating every rep in every
response. Records the probe round this came out of: indexed leaves 4.04 on
that board, and the UUID prefix confirmed routable by a positive control, so
Patterns A/B/C are ruled out there on evidence rather than on absence.
Issue #335's board reports a sibling in subdeviceIdList and then 4.04s on
all 26 seeds enumerate_subdevices tries, which reads like "there is nothing
there". Comparing every captured /oic/res in the corpus says otherwise: only
the ARTIK051_DONGLE_FAC_18K board advertises its operational tree at all.
The other five list the onboarding surface and stop -- the range board hides
a live /device/1 behind a ten-link /oic/res -- so an href's absence from
/oic/res is not evidence, and Pattern A's index scan is dead weight
everywhere except the board it was written against.
What that leaves untried is the bare indexed leaf: every indexed href this
project has ever seen arrived inside a /device/<n> batch, and /device/1
4.04ing is evidence about the Collection, not about /mode/vs/1. Leaves
without their Collection is already confirmed BORA behavior in the other
namespace (issue #205). Records the probe list, the OCF composite-device
clause that suggests /sec/devices, a positive control for whether the UUID
prefix routes at all, and the dead ends worth not re-treading.
Adds washer_cycle_table_00 and dryer_cycle_table_00 translation catalog
entries, confirmed by the issue #357 reporter selecting each cycle on a
WF45R6300AW/US washer and DVE45R6300W/A3 dryer and reading back the raw
course code. Table_00 is a separate, older course-code family from the
existing Table_02/Table_03 catalogs -- laundry.cycle_select's table_href
scoping already keeps them apart, so this is a translations-only change.
Table_00 was previously used only as an example of an unconfirmed table in
tests; those now use Table_99 for that role, and new tests assert the
confirmed codes translate and that the resolved key routes to the new
table-scoped catalog entries.
Mirrored to all shipped languages (cs/de/es/it/ko/nl) to keep
tests/test_translations.py's key-for-key invariant.
The reporter's machine_state history for the same two cycles flips to
idle on the exact second progress reads 'Drying' (12:08:51 and 14:01:26),
which settles what the previous commits had to infer: rep_fn returns
'Idle' whenever state isn't active, so it cannot have produced that
value, and the only remaining path was the ungated sticky_live_fn the
bypass returned in its place. Replaying the sequence against the pre-fix
path reproduces the reported Cooling, Finish, Drying, Idle exactly; the
fix holds Finish through it.
Comments only -- swap the inference for the observation that confirms it.
Review of the previous commit caught that its docstring promised more
than the code did. Not restarting an *open* window still let a progress
that flapped out of and back into Finish re-arm a full fresh window once
the first had expired, so the value could be held well past
sticky_seconds from the first Finish. The new test passed only because
its final read left the sticky condition matching; ending the flap on a
non-matching read re-armed and would have failed it.
Make the guarantee real instead of weakening the claim: arming marks the
hold spent, and only sticky_bypass_fn -- a cycle actually running --
clears it. Expiry on its own no longer re-opens the door, because with no
cycle in between a second Finish is the same Finish, and re-arming on it
strobes the entity Finish -> Idle -> Finish once per window, re-firing
the announcements #345 and #358 are both about.
That subsumes the old _sticky_armed edge-trigger flag, which existed to
stop a stuck field extending the window; "spent until a new cycle" covers
that case and the flap case together, before or after expiry.
Also give the flap test real headroom -- it fitted 0.09s of sleeps into a
0.1s window and would have failed spuriously on a loaded runner.
Fixes#358, a regression from #346. That PR's sticky_bypass_fn released
the Finish/100 hold on any concrete non-Finish progress code, ungated on
machine_state, reasoning that a new cycle's own progress can appear
before state catches up. But the reporting DA_WM_TP1_21_COMMON dryer
replays a running stage on the way *out* of a cycle: the issue's history
shows Cooling -> +60s Finish -> +24s 'Drying' -> +4s settled, twice,
identically. The bypass read that tail as a new cycle, dropped the hold,
and republished 'Drying' -- so progress read Drying, Cooling, Finish,
Drying, Idle instead of ending at Finish, Idle.
The tail is not new: rep_fn has always masked progress while state isn't
active, which is why it was invisible before #346. What surfaced it was
sticky_live_fn, a second, ungated view of the same field that the bypass
returned in rep_fn's place -- letting the hold publish a value the entity
otherwise never shows.
Both halves are fixed:
- The bypass (now _new_cycle_running) requires state == 'active'
alongside the progress code. The arm condition stays ungated -- failing
to arm loses the Finish entirely (#345), while releasing late costs
nothing, since the hold expires on its own.
- sticky_live_fn is gone. rep_fn is the only definition of a live value;
the hold decides only whether to freeze, and the bypass returns rep_fn's
own result.
Also stop an already-open window from being restarted by a progress that
flaps in and out of Finish, so sticky_seconds is measured from the first
Finish of a cycle and the documented bound actually holds.
A paused new cycle no longer cuts the hold short (it did under the old
ungated bypass). Nothing live is withheld by that: rep_fn shows Idle
while paused with or without a hold, so the only change is a stale Finish
expiring on schedule -- and 'paused' cannot be told apart from this tail.
ObserveManager.apply() shallow-merges every incoming rep onto whatever's
already cached for that href (issue #27's fix for /mode/vs/0's partial
notifies). That assumes an absent key always means "unchanged, keep the
old value" -- true for /mode/vs/0's supportedOptions, but backwards for
/alarms/vs/0: entity.py already documents {} as this resource's
canonical no-alarm state, and a live read_resource GET on the reporter's
washer confirmed the board sends exactly that {} when an alarm clears.
Merging it onto the prior rep left the stale ErrorCode_DC entry in the
cache forever, surviving power cycles and only clearing on a full
integration reload (which rebuilds the cache from scratch instead of
merging).
Add _is_alarms_href() to recognize /alarms/vs/<index> across every
subdevice-translated shape (MAIN identity, indexed renumbering, prefixed
UUID -- Subdevice.to_actual never touches the 'alarms/vs' stem) and have
apply() fully replace the cache for that href instead of merging. This
is a global fix: every family with an alarm sensor shares this href
(common.ALARMS, range_hood's own copy), so they were all exposed.
0.21.0 was already bumped by #334 but never released; 0.22.0 double-bumped
past it. This release covers everything merged since v0.20.0 and should
just be 0.21.0.
SwitchDeviceClass only ever supported 'outlet'/'switch', not 'lock'.
switch.py passes desc.device_class straight to SwitchDeviceClass(...),
so any board with these hrefs raised ValueError during switch platform
setup and lost every switch entity for the device, not just the lock
ones (issue #349, TP2X_WATERPURIFIER_20K).
KIDS_LOCK_GENERIC/_VS_FALLBACK dodged this same bug (issues #181/#183)
by moving to a read-only BinarySensorDesc, but the water-purifier and
cooktop locks are genuinely writable, so they stay SwitchDesc and just
drop the invalid device_class (with an mdi:lock icon standing in for
the one entity_category=config gave them for free).
Added a registry-wide test that instantiates SwitchDeviceClass for
every SwitchDesc.device_class across every by_type registry, so a
future capability can't reintroduce the same crash.
The one that could run the wrong program: guided setup accepted a name
another program already had. The duplicate check only compared names within
the form it was handed, and guided setup submits one program at a time, so
it never saw the others. Two programs sharing a label resolve to whichever
option comes first, so picking the second would have run the first one's
payload -- the exact failure the check exists to prevent, working correctly
in the bulk form and blind in the guided one. The taken set now includes
every other named program, excluding the slot being edited so confirming an
unchanged name doesn't reject itself.
The rest:
- The timeout screen's copy interpolates the same counters as the other two
but was shown without placeholders, so it rendered literal braces.
- The probe reports whatever payload is loaded, while observe() declines one
whose slot the device doesn't advertise. Guided setup could reach the name
form for such a slot, take a name, and silently discard it -- there is no
record to hang it on and no payload to replay. It now waits instead.
- The prefilled Download course came from the raw candidate list while the
dropdown filters to courses the appliance still offers, so a stale
candidate prefilled a value the selector rejects and the form failed
validation on something the user never chose.
A write schedules a debounced refresh, which polled through a fake session
that only implements post(), crashed on the missing get(), and left its timer
running past the end of the test. CI's lingering-timer check caught it; it
passed locally only by timing luck.
Stubbed the same way test_coordinator_send_command's fixture already does,
which is what this test should have copied to begin with.
The Download-course candidate came from "Course_ read while a non-sentinel
one-time payload is loaded". On the first rep after any restart that is
indistinguishable from a payload left over from a previous run, so an
appliance holding cloud payloads while sitting on an ordinary course
proposed that ordinary course as the Download one. Accepting the prefill
would then make selecting a downloaded program start, say, a cotton wash.
Only a transition actually watched counts now. "Never observed" is a
distinct state from "observed, nothing loaded" -- absent-then-loaded is a
genuine selection and still counts -- so a restored store deliberately
re-enters the unobserved state, since a restart cannot tell the two apart.
Payloads are still learned from that first rep either way; which programs
exist is device fact regardless of when they were loaded. It is only the
inference about which course means Download that needs the timing.
Both corpus dumps taken off the Download course show the appliance clearing
its one-time token to the FFFF sentinel, so this may never fire on these
boards. That is a reason to expect them to behave, not to depend on it.
A program is only offerable once it has both a name and the Download course
code that goes in the Course_ token. Guided setup collected names and never
asked about the course, so a user could walk all nine programs, watch every
name save, and end up with nothing in the cycle list.
Worse, it actively cleared the course. _apply_cloud_course_names read
download_course out of the submitted form; the guided name form has no such
field, so it passed None and apply_cloud_courses stored that. Naming a
program therefore removed every previously-named program from the list.
Traced on the fixture: 87 -> name one -> None -> nothing offerable.
Two changes. apply_cloud_courses now defaults download_course to "leave it
alone" rather than None, so silence can't be mistaken for a clear, and the
guided path forwards the field only when its form actually carried it.
And the first guided name form now asks for the course, prefilled from what
was just observed, dropping the field once confirmed. Asking there rather
than up front is deliberate: it is the first moment there is evidence to
prefill, since the user has just loaded a program and the course showing
alongside it is the Download one. That makes the walk stand on its own,
which is the whole point of offering it as the primary path.
The bulk form's course dropdown now shares the guided one's builder.
Translations for the guided-setup strings are in for cs/de/es/it/ko/nl,
matching the vocabulary the earlier pass established. The new field on the
name form reuses each locale's existing label from the bulk form rather than
adding an untranslated string.
A nine-program walk is hard to keep your place in. The counter alone doesn't
say what you've already done, and the programs still to do can't be listed --
they're unnamed by definition, which is the whole premise. So "named so far"
is the only orientation available, and it now appears on all three guided
screens.
It doubles as duplicate avoidance on the naming form: a repeated name is
rejected, so seeing the others while typing beats being bounced afterwards.
Listed in the appliance's own advertised order rather than the order they
were named -- a stable order either way, and not numbered: whether it matches
the dial is plausible but unverified, and implying it would be worse than
saying nothing.
Naming downloaded programs from a list of hex slot ids was the weak part of
this feature: it asked about programs in the abstract, long after the user
had touched the appliance, and the per-slot fields rendered as raw keys
because Home Assistant can't translate dynamic ones.
Guided setup asks in the moment instead. It waits on a progress step while
the user selects a program on the appliance, then asks for that one's name --
so the field is a single static key, and "which one is this?" is answered by
the user having just turned the dial to it. The prompt also shows the
appliance's own reported remaining time, which differs per program and is
device-reported rather than decoded.
Two things it has to get right:
- It waits for a *transition*, not a state. After naming a program the
appliance is still sitting on it, so a loop keyed on "a known slot is
loaded" would re-offer the same one forever. Each round baselines on
whatever is loaded when it starts.
- Re-selecting an already-named program is not an error -- it is how someone
checks their work -- so it gets the existing name pre-filled and the
counter deliberately does not move, rather than a rejection.
Names persist as they are entered rather than batching to the end of the
flow, which makes closing the dialog a clean "save and exit" with nothing
pending to lose, and makes the flow resumable: reopening picks up from the
store. async_remove cancels an in-flight round, so walking away actually
stops the probing instead of holding the session lock every few seconds
until the timeout.
/course/vs/0 is cold-tier, so passively a selection can take a whole poll
interval to appear. async_probe_cloud_courses live-reads it through the
normal apply path, keeping learning and persistence in one place.
The bulk form stays, under its own step, as the way to rename things later --
which guided setup is bad at.
New strings ship in English in every catalog and need translating.
Its bytes are not payload slots everywhere. On the DW5000C dishwasher all
four (8E 8D 8F 02) are course codes in that device's own course list, three
already translated -- Plastic, Pots and pans, Baby Care. There the token
marks which ordinary courses came from the cloud; they select with a plain
Course_ write and need no payload, which is consistent with it carrying no
payload token at all. It also has a DownloadCourseList_ token the washers
lack. On both washers the slots share zero overlap with the course list and
a payload is required to select one.
So the "this is not washer-only" claim was wrong, and gating on
advertised_slots offered that dishwasher's owner a naming flow for programs
that already work and are already named. The Repairs card was spared only
because the payload gate added earlier happens to catch it.
cloud_slots() subtracts the device's own course list, which separates the two
readings without guessing at families: what remains is slots that cannot be
selected any other way, which is what this module is for. Everything
user-facing now gates on that -- the options menu entry, the naming flow, the
Repairs count. The dishwasher gets nothing, both washers are unchanged.
Found by reading the dishwasher fixture's options array while answering a
question about it, which is also why diagnostics now reports advertised and
cloud slots separately: the difference between them is the whole distinction.
The eight new strings shipped as English placeholders in every non-English
catalog. Translated for cs/de/es/it/ko/nl.
Where a locale's course table already names the Download course itself, that
existing term is reused rather than a fresh coinage -- Korean's 다운로드 코스
is the catalog's own translation of course code 17, so the options flow now
says what the appliance display says. German and Dutch had no equally
distinctive existing term and build on the adjective already used for the
downloaded state.
Counts are not pluralized. The strings have no plural support and the
placeholders are raw numbers, so Czech, Italian and Dutch use the plural form
regardless of count -- the same simplification the rest of these catalogs
already make.
Trimming the names out of the dump last commit was the wrong call. Half of
what goes wrong with this feature is a configuration question -- which
programs got named, which Download course was confirmed, whether a payload
was ever captured for a slot the device advertises -- and none of that is
answerable from the payloads alone. A report saying "my download cycle isn't
showing up" is exactly the case that needs it.
The names are still the user's own words, so this block stays the one place
they appear; they reach a dump only because its owner chose to download and
share it. `resources` is unaffected either way -- it goes on reporting
exactly what the appliance said, via device_resources().
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.
Also drops appliance-specific wording from the new user-facing strings. The
setup step said "your washer" and told people to "turn the dial", which is
wrong for the DW5000C dishwasher that advertises the same tokens.
The two that could have caused a wrong wash cycle:
- The Download-course candidate was counted on every poll that saw a loaded
one-time payload, not on the polls where one was actually loaded. Since a
stale token is never evicted, it keeps being reported through however long
the appliance then sits on some ordinary course -- so "most frequent"
ranked by dwell time. Reproduced: one poll on Course_87 then 200 on
Course_1B suggests 1B, and accepting the suggestion makes picking a
download program start a Cotton wash. Now only a change of the payload
counts, which is the moment the device is known to accept a program.
- The Download-course dropdown had custom_value=True, contradicting its own
comment, so a typed-in code went into the Course_ token of a real write
unchecked. Off now, plus a server-side check against the device's own
course list where the value is stored.
Two that quietly broke things beyond this feature:
- cycle_select now always supplies a display_fn (to label cloud programs),
which defeated select._display's "no state table and no fallback -> return
raw" exit. Every dryer, dishwasher and air dresser on an unrecognized
course table would have had its options and state reshaped from '0E' to
'0 E', breaking automations and recorder history. The exit now keys off
whether anything actually named the value, not whether a fallback existed.
- The synthetic cloud field reached diagnostics, which reads
canonical_resources -- publishing user-typed program names in a dump
people paste into issues, directly against the comment claiming it never
could. Dropped at the redaction boundary, with a matching strip for the
debug read service, which wants device state unredacted but shouldn't
present our bookkeeping as something the appliance said.
And two smaller ones:
- The repair fired on any device advertising slots, so the DW5000C -- four
advertised, none ever loaded -- got a permanent warning nothing the owner
did in Home Assistant could clear. It now waits until a payload has been
seen, which is the only evidence that household uses downloaded programs.
- The name-collision check read only the translation catalog, missing the
device's own personal-course labels, which the select renders identically.
A survey of every laundry diagnostics dump attached to an issue turned up 14
devices, 4 of which carry cloud-course tokens. Two were already known; the
two new ones are both useful, and one contradicts something the
investigation write-up asserted.
A DW5000C dishwasher (issues #113/#123) advertises four downloaded programs
and carries no payload token for any of them. That is a shape the corpus
didn't have: the feature is not washer-only (DA_DW, not DA_WM), and a device
can name programs whose payloads have never been observed. The existing code
already handles it correctly -- nothing learnable, nothing offered, gap still
counted for the Repairs issue -- so this adds the fixture, golden, and tests
that keep it that way.
A second WW5000C (issues #259/#343, firmware _B048) holds the same saved
program as the first one's captured "Towels", and the two payloads differ at
exactly one byte: byte 3, 04 against 06. Everything else -- id, slot, all
four varying tag values, the whole tail -- is identical. So byte 3 is neither
a per-board constant nor a property of the program, and the doc's claim that
it is always 04 on this board was wrong.
That is also the strongest argument yet for learning payloads per device: a
catalog keyed on program id would have shipped one unit's byte 3 to the
other. Nothing changes in the implementation as a result -- it never had a
catalog -- but the reasoning is now backed by evidence rather than caution.
Also recorded: both WW5000C units advertise the byte-identical slot list
despite different firmware, so the program set looks factory- or
region-assigned rather than user-curated; and a sentinel's byte 2 equals the
selected course on one dump but not the other, so it stays unused.
Byte-aligning the WA55A7700AV's 16-byte payload against the WW5000C's
20-byte one: identical header, and the first four tag/value pairs are the
same tags in the same order at the same offsets -- the part that carries
per-program data has one shape on both boards. The whole width difference
is two trailing pairs the WA55 doesn't carry, and on the WW5000C that
trailing section is byte-identical across all nine programs, so it isn't
program data at all.
Doesn't change the conclusion -- the four shared tags carry non-overlapping
value ranges between the boards, so the encoding is still board-specific and
blobs are still replayed whole. Also records why the WA55's /washer/vs/0
readings can't be used to confirm a decode: that unit is on a local course,
not its cloud course.
A washer whose course table includes "Download"/"Downloaded" runs whichever
program the SmartThings cloud last pushed down. Those programs are now
selectable from the ordinary cycle select, so a downloaded Jeans or Sports
cycle can be started without giving the appliance internet access.
The device turns out to enumerate them itself. `CloudExtraCourse_` on
/course/vs/0 lists one byte per downloaded program, and byte 2 of a
program's payload is exactly that slot id -- verified against all nine
programs on the reporter's WW5000C and against the WA55A7700AV dump already
in the corpus. So nothing here is hardcoded: the appliance says which
programs exist, the payloads are learned by watching what it reports, and
the names come from the user.
That last part is unavoidable rather than a shortcut. A payload is only
visible while its program is loaded, and the appliance never reports a name
for one. So cloudcourse.py persists what has been seen (same rationale as
learned.py's mode store), a Repairs issue tells the owner how many programs
are still unaccounted for, and an options-flow step collects the names. A
program appears in the cycle select only once it is both learned and named.
Selecting one issues the only two-token options write in the codebase --
the course token has to switch to Download in the same write, or the
appliance accepts the program token and silently ignores it (confirmed on
hardware). The Download course code is learned by observation but never
applied until the user confirms it: tokens in this array are replaced by
prefix and never evicted, so a stale program token can appear alongside an
unrelated course, and acting on that would start the wrong wash cycle. For
the same reason a stale token is never reported as the running program.
Also of note:
- There is no single "Download" course code. The WW5000C uses 87, the
WA55A7700AV uses 17 -- same Table_02. Any per-table lookup would have
been wrong on one of the only two devices available to check.
- Payloads are replayed byte-for-byte and never decomposed or rebuilt.
Bytes 5/7/9 do decode to temperature/rinse/spin on the WW5000C, 9 for 9,
and produce nonsense on the WA55A7700AV -- so that decode is written up
in docs/investigations/download-cycle.md and not shipped, and the
read-only sensors it would have enabled were dropped.
- The store reaches the registry as a namespaced synthetic field merged
onto /course/vs/0's rep at read time, so exists_fn/rep_fn/options/write_fn
all see it through their existing signatures. It never enters the state
cache, so it can't be polled over, written to the device, or land in a
diagnostics dump.
- A name that would render identically to another cycle in the same
dropdown is rejected in the flow: the select maps a chosen label back to
a raw value by matching display text.
Non-English catalogs carry the new strings in English for now; they need
real translations.
Fixes#345. progress and progress_percentage were gated on machine_state
alone, falling straight to "Idle"/0 the instant state left 'active' --
but a washer can flip state away from 'active' within the same poll
interval progress reaches 'Finish' (more reliably than the same-family
dryer, per the report), so an automation watching for a real 'Finish'
value could go a whole cycle without ever observing one.
Implemented read-side, per-entity (sensor.py's new _apply_sticky),
generalizing the existing _hysteresis_value/_apply_hysteresis pattern
finish_time already uses, rather than writing a synthetic override back
into the coordinator's cache. That cache is this integration's record of
what the device actually said, and is read by several unrelated
consumers -- write_fn, validate_fn, diagnostics, the observe-mode sweep
comparison against /device/0, _completion_minutes' own stale-remainingTime
workaround -- all of which would otherwise see fabricated state.
A first pass gated the hold's arm condition on state=='active' AND
progress=='Finish' occurring in the same rep, mirroring _is_active. A
second review caught that this could make the whole fix a no-op on the
one device #345 reports it for: if state has already reset by the time
progress is ever observed at 'Finish' -- exactly what #345 describes --
the arm condition never fires. _just_finished now arms on progress==
'Finish' alone. That reopens the staleness risk the state check existed
to guard against (a progress field stuck at 'Finish' forever would then
arm forever too), so _apply_sticky is edge-triggered: only a fresh
False->True transition (re)starts the window, and expiry is still
checked on every call even while the condition keeps matching -- a
stuck value still won't hold past sticky_seconds.
Dropping the state requirement also exposed a second gap: the bypass
that lets a new cycle's own real progress override a stale hold was
keyed on machine_state=='active', so it missed a new cycle immediately
paused (e.g. adding a sock) -- machine_state isn't 'active' while
paused. It's keyed on a live, non-Finish progress code instead
(_live_progress_code), independent of state, same reasoning as
_just_finished. And since the bypass needs the *real* live value, not
whatever rep_fn's own (differently gated) result says, SensorDesc grew
sticky_live_fn alongside sticky_value_fn: rep_fn's progress gate still
shows "Idle" while paused, but the real progress value read ungated
must win over the hold regardless.
registry/entities.py: SensorDesc gains sticky_fn/sticky_value_fn/
sticky_live_fn/sticky_bypass_fn/sticky_seconds -- see sensor.py's
_apply_sticky docstring for the full contract.
registry/capabilities/operational.py: progress/progress_percentage's
rep_fn is unchanged; they gain the sticky_* wiring above. machine_state
and the Running binary sensor are untouched -- still gated on real-time
state (cycle_active now shares _is_active's rep_fn directly rather than
a duplicate inline copy), so they never claim the appliance is still
running once it isn't.
async_send_command handed write_fn/validate_fn the raw cache snapshot
(real, on-the-wire hrefs), not a subdevice-scoped view. Every other
consumer of a full resources dict (exists_fn, rep_fn, is_legacy_board,
...) reads through coordinator.canonical_resources() specifically to
avoid this; write_fn/validate_fn didn't, so on a composite AC (issue
#177) _temperature_step's resources.get(HREF_TEMP_CONTROL) saw the
master's /temperature/control/vs/0 instead of the subdevice's own
/temperature/control/vs/1, silently rounding a subdevice's 0.5-degree
write to a whole degree. The remote-control gate stays on the raw
snapshot -- /remotectrl/* is a shared, MAIN-only resource a
subdevice's owned-hrefs-only canonical view would drop entirely.
climate.py's target_temperature_step duplicated this same
read-in-order logic; pointed it at airconditioner._temperature_step
so the read and write paths can't drift again.
Also, from the same review:
- _quantize_temperature rejects non-finite floats (nan/inf survive
float() but raise out of round()/division, escaping write_fn's
documented None-on-bad-payload contract).
- Deduplicated the quantize-and-check block shared by the
temperature_ocf/temperature branches of _climate_write.
- Added the /temperatures/vs/0 items[]-fallback test that was
previously unreachable (every increment-carrying fixture also has
/temperature/control/vs/0, which _temperature_step checks first).
- Added a coordinator-level test seeding an indexed subdevice with its
own step, distinct from the master's, covering the fix above.
- The Towels/Bedding regression test now checks all 7 locale catalogs,
not just English -- the bug is a code-mapping error, and
test_every_language_mirrors_the_english_catalog only checks key
topology, not values.
ClimateDesc.write_fn is typed as WriteFn (Callable[[Any, dict], ...]),
which only covers the (payload, rep) shape every other capability's
write_fn honors -- calling it through that alias with the climate-only
href/resources args, without first narrowing away the | None, failed
ty two ways: the missing "is not None" check and the extra positional
args past WriteFn's declared arity. Call _climate_write directly
instead, same as test_coordinator_send_command.py and
test_airconditioner_artik051_krac.py already do.
Issue #343: DA_WM_TP1_21_COMMON's washer_cycle_table_02 had course
codes 24 and 33 transposed -- selecting "Towels" in HA ran the
washer's Bedding cycle and vice versa (confirmed against the
reporter's diagnostics dump: Course_24 selected, courseTable
Table_02). Swapped both codes' labels back in line with the 69/6A-
79/88 family's own Bedding/Towels pair (6f/70), across every locale
catalog.
Issue #342: added the four course codes the reporter's editCourseList
carried with no catalog entry -- 06 (XXL Laundry), 08 (Rinse+Spin),
and a0 (15' Quick Wash) were missing outright; 74 (Drum Clean) turned
out to already be translated by the time this landed.
The download-course request in the same issue (selecting which
program a "Download" cycle fetches) is left for a follow-up -- still
waiting on a confirmed local write path before building anything on
top of the OneTimeCloudCourse/CloudCourse fields.
Samsung local AC temperature writes always rounded to the nearest
whole degree, dropping half-degree setpoints on boards that advertise
a 0.5 step (CAC and TP1X FAC). Squashed from moridew's PR #276 with
the review fixes applied:
- The /temperatures/vs/0 fallback never matched: its increment lives
inside the resource's items[] array, not at the top level (same
shape _temps_vs_item() already unwraps for current/unit). The
original fix only ever worked through /temperature/control/vs/0.
- With no increment advertised anywhere (e.g. ARTIK051), writes went
out unrounded instead of falling back to whole degrees the way
climate.py's target_temperature_step already does.
- A non-numeric payload now rejects the write (returns None) instead
of posting {"temperature": null} -- coordinator.py's
async_send_command already drops a write_fn result of None.
- int/float normalization now happens once, in _quantize_temperature,
instead of being duplicated (and skipped) per branch; a round(...,
2) guards against float division noise (e.g. 21.7 / 0.1).
Tests rebuilt against real fixture resources (airconditioner_cac,
airconditioner_artik051_krac_18k) instead of a fabricated flat
resource shape no device produces.
Marc Billow