Commit Graph
6 Commits
Author SHA1 Message Date
Marc Billow ca27bf7f6e docs: de-identify reporter usernames repo-wide, add PII rule to skill
Extended the earlier de-identification (jhkwon19) to the other issue #177
reporter (HJcom), who was named in ~20 spots across coordinator.py,
diagnostics.py, subdevices.py, identity.py, capabilities/airconditioner.py,
a fixture's seeds_note, and several test docstrings/function names. Swapped
all of it for "the reporter"/"the Pattern A reporter", matching the
convention already established for the other reporter.

Added a rule to the adding-device-support skill (end of "Lock it in"):
don't put a reporter's name or GitHub username in code comments,
docstrings, seeds_note, or test/function names -- that prose ships and
sits in git history indefinitely, unlike an issue thread or a
release-notes thank-you. Use "the reporter" / "issue #NNN's reporter" /
a pattern label instead.
2026-07-30 03:06:06 +00:00
Marc Billow ce92d71a92 docs: de-identify the reporter's username in code comments
Comments/docstrings across subdevices.py, coordinator.py, the two fac_bora
fixtures, and their tests named the issue #177/#205 reporter directly.
Swapped to "the reporter"/"the Pattern B reporter" throughout -- no
behavior or test-assertion changes, string content only. HJcom (the
Pattern A reporter) is unaffected.
2026-07-30 02:50:49 +00:00
Marc Billow 8600985a36 review: address Opus findings on the subdevice flat-href fallback
- coordinator: _poll_subdevice_flat_hrefs called sess.pace() outside its
  try block and re-read self._session instead of using the caller's
  already-None-checked reference -- a session closed mid-poll (async_close()
  doesn't hold _session_lock) could crash the whole poll cycle instead of
  just dropping that one sibling. Now takes sess from the caller and guards
  pace() the same as get().
- coordinator: skip flat hrefs already covered by the hot/warm sub-poll
  tiers -- those are refreshed every few seconds by _run_subpolls already,
  so re-fetching them again on the once-per-summary-poll flat pass only adds
  GETs, not freshness. Matters because, unlike the Collection path (always
  one GET), a flat subdevice's summary-poll cost scales with its href count.
- subdevices.py module docstring: corrected an overclaim inherited from PR
  #199 that GET /<uuid>/device/0 had been "confirmed live" on jhkwon19's
  unit. Only an individual /information/vs/0 read was ever actually
  confirmed; the Collection endpoint itself has never been observed to
  answer on any known unit, which is exactly what issue #205 exposes.
- SKILL.md: fixed the numofsubdevice cross-check formula to match what
  coordinator.py actually compares (len(materialized) + 1, not
  len(subdevices) + len(subdevices_skipped)).
- Documented, not yet guarded against: a firmware that echoes state back
  under any unrecognized prefix instead of 4.04ing could pass the flat probe
  and the liveness gate, materializing a phantom duplicate of the master.
  Every board seen so far genuinely 4.04s on paths it doesn't own.
- Added test coverage the review flagged as missing: a materialized (not
  just skipped) flat subdevice re-polling end-to-end through to
  canonical_resources, the hot/warm skip itself, zero-master-hrefs and
  two-UUID no-cross-contamination edge cases, and a golden file for the
  #205 fixture (SKILL.md's "fixture + golden + test" discipline).
2026-07-30 02:44:26 +00:00
Marc Billow e78d941af3 fix(subdevices): fall back to per-href probing when a prefixed subdevice has no /device/0 Collection
Issue #205 shows the UUID-prefixed pattern's own reference device
(TP2X_FAC_BORA_21K) doesn't always answer /<uuid>/device/0, contrary to
what the pattern was built against. When that Collection GET comes back
empty, enumerate_subdevices now probes every href the master itself
answered this cycle individually under the UUID prefix, keeping whichever
ones respond. Subdevice gains a flat_hrefs field for this, and the
coordinator re-polls those hrefs individually each cycle instead of
re-fetching a Collection batch that doesn't exist.

Built a fixture from the reporter's real #205 diagnostics dump: the
fallback finds a candidate through the one href already confirmed live
under this UUID (/information/vs/0, from the #177 thread), and
discover_partitioned's liveness gate correctly holds it back since that
href alone binds no entity -- honest current state, not a guessed
resolution.
2026-07-30 02:25:26 +00:00
Marc Billow 9e85395b44 rename: unit/sub-unit -> subdevice, matching OCF terminology
"Unit"/"sub-unit" from #199's multi-indoor-device support wasn't OCF
idiomatic -- OCF calls each component of a composite device a
"subdevice" (see subdeviceIdList), so rename SubUnit -> Subdevice
throughout: the registry module, coordinator state, BoundEntity's
subdevice field, diagnostics keys (subdevices/subdevices_skipped/
subdevice_probes), entity unique_id prefixes (unit1_/sub_<uuid>_ ->
subdevice1_/subdevice_<uuid>_), device-name fallback labels, golden
fixtures, tests, README, and the adding-device-support skill.

Breaking change to entity unique_ids and diagnostics keys, acceptable
since this hasn't been released yet.
2026-07-29 22:57:11 +00:00
Marc Billow c181a8b447 feat(subdevices): support multi-indoor-unit systems (#177)
Samsung 2-in-1 air conditioners put more than one logical indoor unit
behind a single IP and a single DTLS session. Only the unit the config
entry was set up against was ever discovered; the second one -- a whole
physical appliance the user can see in SmartThings -- had no entities at
all. Two reporters turned out to have two different mechanisms:

  ARTIK051_DONGLE_FAC_18K -- indexed siblings. /oic/res registers the
  whole tree discoverable and lists three complete parallel resource
  sets whose trailing path segment is the index (/mode/vs/0, /mode/vs/1,
  ...), on OCF-standard and vendor hrefs alike. /device/0's batch
  carries only the index-0 hrefs, so a sibling is reachable only through
  its own /device/<n> collection.

  TP2X_FAC_BORA_21K -- UUID-prefixed tree. /oic/res hides the appliance
  tree entirely (which is why a direct /device/1 probe returns nothing
  on this board). /subdevices/vs/0 carries subdeviceIdList instead, and
  that UUID appears as a literal href prefix; /<uuid>/information/vs/0
  was confirmed live to return the wall unit's own model and serial
  (TP2X_FAC_BORA_RAC_21K) against the master's TP2X_FAC_BORA_21K.

The detection signals don't overlap on either board, so no
disambiguation is needed -- enumeration checks both and takes what
answers.

Both patterns are the same thing underneath: a logical unit is a seed
collection path to poll plus an href transform between the canonical
href the registry knows and the actual on-the-wire href. That is the
whole abstraction (SubUnit), applied at four boundaries -- discovery,
the coordinator, the adapter, and the platforms. Capabilities, the
registry and the climate composite stay written against canonical hrefs
and are untouched.

Uniqueness comes from a key_prefix inside the flattened state key, so
the master unit's keys are byte-identical to every release before this
and every existing golden file is an unchanged regression guard. Each
sub-unit gets its own device-registry entry linked by via_device and
named from its own /information/vs/<n>, so it lands in its own room
rather than crowding the master's device page.

A sub-unit materializes only when it yields at least one primary
(non-diagnostic) entity with a populated value. That gate is not
decoration: the reporter's /device/2 is an unused slot that SmartThings
shows disabled, yet it answers with a full 14-href batch, and it
flattens to exactly one non-None value -- a diagnostic alarm_code
derived from an empty /alarms/vs/2. Without the entity-category filter
it becomes a phantom third climate card. The rule is deliberately
domain-agnostic rather than a list of HVAC hrefs, so a multi-drum
washer (#19) gets the same treatment with no new curation. Units that
answer but fail the gate are logged and reported in diagnostics, so a
genuinely missing unit stays diagnosable from a dump.

Enumeration fetches things that must not then be treated as appliance
state. A rejected candidate's seed has to be read to evaluate the gate,
but only units that pass are polled again, and StateCache has no
eviction -- so discovery runs before the first cache apply and those
reps are held aside for diagnostics rather than frozen into the cache
forever. /multidevice/vs/0 is probed on every device regardless of
family, so merging it into the resources dict would have reached
discovery on any board whose registry doesn't ignore that href -- only
the air conditioner one does -- raising a spurious coverage-gap repair
for a washer or fridge whose firmware answers it. It is corroborating
metadata (numofsubdevice, confirmed read-only) and now lives beside the
resources rather than in them.

Diagnostics reports each unit separately: top-level `resources` is this
unit's own and only its own, which is what the module docstring and the
adding-device-support skill have always claimed it was, and each
sibling or rejected candidate carries its own reps canonicalized so a
block reads exactly like the master's instead of needing to be
de-indexed by hand.

Fixtures are real captures. The ARTIK051_DONGLE_FAC_18K one is entirely
verbatim, both sibling seeds and the hand-read /multidevice/vs/0
included. The TP2X_FAC_BORA one has a real device0, oic_res and
sub-unit /information/vs/0, with the remainder of that unit's tree
constructed and documented as such in seeds_note; /<uuid>/device/0 is
the one part of that pattern still inferred rather than observed, and
can't be tested through the debug panel because a Collection returns a
list.
2026-07-29 19:14:51 +00:00