Marc Billow
c181a8b447
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.
83 lines
4.0 KiB
Python
83 lines
4.0 KiB
Python
"""Corpus-wide invariant: adapter._key must be unique across every bound
|
|
entity that would actually be registered as an HA entity, for every fixture
|
|
in the corpus -- issue #177's whole SubUnit/key_prefix design exists to
|
|
protect this (see DESIGN-177.md section 3/6). Run over the entire fixture
|
|
set, not just the two new sub-unit fixtures, so a future dump -- sub-unit-
|
|
capable or not -- exercises it automatically.
|
|
"""
|
|
from collections import Counter
|
|
|
|
import pytest
|
|
|
|
from custom_components.localthings.entity import _is_included
|
|
from custom_components.localthings.registry.adapter import _key
|
|
from custom_components.localthings.registry.entities import PLATFORM_OF
|
|
|
|
from tests.conftest import FIXTURES, _discover_full, _load_device_full
|
|
|
|
_FIXTURE_NAMES = sorted(
|
|
p.name[:-len('_device.json')] for p in FIXTURES.glob('*_device.json')
|
|
)
|
|
|
|
|
|
class _FakeCoordinator:
|
|
"""Just enough of LocalThingsCoordinator's surface for entity.py's
|
|
_is_included -- the same one-time entity-creation gate every platform's
|
|
async_setup_entry runs (see entity.py's own module docstring)."""
|
|
|
|
def __init__(self, resources: dict[str, dict], sub_units):
|
|
self.last_resources = resources
|
|
self._sub_units = list(sub_units)
|
|
|
|
def canonical_resources(self, sub_unit):
|
|
from custom_components.localthings.registry.subunits import canonical_view
|
|
return canonical_view(sub_unit, self.last_resources, self._sub_units)
|
|
|
|
|
|
@pytest.mark.parametrize('name', _FIXTURE_NAMES)
|
|
def test_key_is_unique_across_all_bound_entities(name):
|
|
"""`_key(b)` only has to be unique among entities `_is_included` would
|
|
actually register -- discover() alone can (deliberately) produce two
|
|
BoundEntity rows sharing a key on the same href when they're gated by
|
|
mutually-exclusive `exists_fn`s (see fridge.py's
|
|
REFRIGERATION_FALLBACK.defrost_active, which only exists when
|
|
DEFROST_BLOCK_STATUS's own `/defrost/block/vs/0` href is absent) --
|
|
only one of the two is ever actually included for a real device, so
|
|
checking the raw `bound` list would flag devices that have always
|
|
worked correctly. It also only has to be unique *within one HA
|
|
platform* -- unique_id collisions are scoped by (integration, platform)
|
|
in HA's entity registry (see entity.py's `_attr_unique_id`, which
|
|
doesn't itself encode the platform), and several devices in this corpus
|
|
deliberately bind the same key to the same href on two different
|
|
platforms discriminated the same way (e.g. a SwitchDesc and a
|
|
BinarySensorDesc both named 'power_switch' on /power/0).
|
|
"""
|
|
resources, oic_res, seeds = _load_device_full(name)
|
|
bound, materialized, skipped, full_resources, device_type_name = _discover_full(
|
|
resources, oic_res, seeds,
|
|
)
|
|
coordinator = _FakeCoordinator(full_resources, materialized)
|
|
included = [b for b in bound if _is_included(b, coordinator)]
|
|
keys = [(PLATFORM_OF[type(b.desc)], _key(b)) for b in included]
|
|
dupes = {k: n for k, n in Counter(keys).items() if n > 1}
|
|
assert not dupes, (
|
|
f"{name}: duplicate (platform, _key) values across {len(included)} "
|
|
f"included entities (materialized sub-units: "
|
|
f"{[su.key for su in materialized]}): {dupes}"
|
|
)
|
|
|
|
|
|
def test_sub_unit_capable_fixtures_actually_exercise_a_sub_unit():
|
|
"""A meta-check on the test above: if both sub-unit fixtures somehow
|
|
stopped materializing any sub-unit (a regression in enumeration or the
|
|
materialization gate), the corpus-wide uniqueness test above would keep
|
|
passing vacuously -- it never gets to check a single collision. Assert
|
|
the two fixtures this design added actually produce a materialized
|
|
sub-unit, so that silent-vacuous-pass failure mode is caught here
|
|
instead."""
|
|
for name in ('airconditioner_artik051_dongle_fac_18k',
|
|
'airconditioner_fac_bora_2in1'):
|
|
resources, oic_res, seeds = _load_device_full(name)
|
|
_, materialized, _, _, _ = _discover_full(resources, oic_res, seeds)
|
|
assert materialized, f"{name}: expected at least one materialized sub-unit"
|