Retry /<uuid>/information/vs/0 in the flat fallback so siblings get their own model/serial
Cloning the master's hrefs/values verbatim (issue #265) also clones its /information/vs/0 -- so a sibling's HA device page showed the master's own model and serial number, reading as a duplicate device even though the two devices' registry identifiers are genuinely distinct (they're derived from the subdevice's UUID, not from this resource). /information/vs/0 is also the one href this pattern has ever actually been confirmed to answer on its own (issue #177 comment 5113518087), so the fallback now retries that one href specifically -- a single bounded RETRIEVE per candidate, not the ~30 that caused the original hang -- and uses the real reply in place of the clone when it answers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MuiJvThhi4WkNj2xb9xo8o
This commit is contained in:
@@ -47,10 +47,17 @@ trying to re-confirm that claim one href at a time bought unreliable
|
||||
confirmation at a cost that could take the integration down with it. So the
|
||||
fallback now trusts the claim and clones the master's current hrefs *and*
|
||||
values verbatim under the prefix as this cycle's assumed state for the
|
||||
sibling -- no extra RETRIEVEs at all. `coordinator._poll_subdevice_flat_hrefs`
|
||||
still re-GETs each of those hrefs individually, under the prefix, on every
|
||||
later summary poll (unchanged by this), correcting the assumption toward
|
||||
the sibling's real values as responses arrive; an href the firmware never
|
||||
sibling, with one bounded exception: it still retries a single individual
|
||||
`GET /<uuid>/information/vs/0` -- the one href this pattern has ever
|
||||
actually confirmed to answer on its own, per the paragraph above -- so the
|
||||
sibling's HA device shows its own model/serial rather than the master's
|
||||
cloned ones (identical model/serial across two otherwise-distinct devices
|
||||
reads as a duplicate, even though their device-registry identifiers are
|
||||
genuinely different). That's one extra RETRIEVE per candidate, not the ~30
|
||||
that caused issue #265. `coordinator._poll_subdevice_flat_hrefs` still
|
||||
re-GETs every cloned href individually, under the prefix, on every later
|
||||
summary poll (unchanged by this), correcting the assumption toward the
|
||||
sibling's real values as responses arrive; an href the firmware never
|
||||
answers under that prefix just keeps mirroring the master indefinitely --
|
||||
which, for a board proven not to expose that href at all, is what "assume
|
||||
it exists" has to mean in practice. See `Subdevice.flat_hrefs`.
|
||||
@@ -420,8 +427,8 @@ def enumerate_subdevices(
|
||||
# there, so trust it outright rather than spend an unbounded,
|
||||
# firmware-dependent number of RETRIEVEs re-confirming it href by
|
||||
# href: clone the master's current hrefs and values verbatim under
|
||||
# this prefix as the sibling's assumed state, no further RETRIEVEs
|
||||
# at all. coordinator._poll_subdevice_flat_hrefs re-GETs each of
|
||||
# this prefix as the sibling's assumed state, no per-href probing
|
||||
# loop at all. coordinator._poll_subdevice_flat_hrefs re-GETs each of
|
||||
# these hrefs individually, under the prefix, on every later summary
|
||||
# poll (unchanged by this), correcting the clone toward the
|
||||
# sibling's real values as responses arrive; an href the firmware
|
||||
@@ -433,6 +440,25 @@ def enumerate_subdevices(
|
||||
return
|
||||
for href in flat_hrefs:
|
||||
fetched[f"/{sub_id}{href}"] = resources[href]
|
||||
# One exception, worth its own bounded RETRIEVE rather than trusting
|
||||
# the clone: /information/vs/0 is what device_info_for() reads for
|
||||
# this subdevice's own model/serial. Left cloned from the master,
|
||||
# the sibling's HA device page would show the *master's* model and
|
||||
# serial verbatim -- distinct device-registry identifiers (derived
|
||||
# from this UUID, see device_info_for), but reading as a duplicate
|
||||
# of the master to anyone looking at the two devices' info. This is
|
||||
# also the one href ever actually confirmed to answer under this
|
||||
# pattern's prefix on real hardware (issue #177 comment 5113518087),
|
||||
# so it's worth retrying here even though the rest of the fallback
|
||||
# no longer probes anything -- one extra RETRIEVE per candidate,
|
||||
# not the ~30 that caused issue #265.
|
||||
info_seed = (sub_id, "information", "vs", "0")
|
||||
info = _get_property(sess, info_seed)
|
||||
_probed(_seed_href(info_seed), info)
|
||||
if info:
|
||||
fetched[f"/{sub_id}/information/vs/0"] = info
|
||||
if "/information/vs/0" not in flat_hrefs:
|
||||
flat_hrefs = tuple(sorted({*flat_hrefs, "/information/vs/0"}))
|
||||
subdevices.append(
|
||||
Subdevice(
|
||||
kind="prefixed",
|
||||
|
||||
@@ -285,11 +285,12 @@ async def test_fac_bora_205_flat_fallback_clones_master_and_materializes(
|
||||
(issue #205), but issue #265 replaced the old per-href confirmation loop
|
||||
-- which used to hold this candidate back with only /information/vs/0
|
||||
ever confirmed live under the prefix -- with an unconditional clone of
|
||||
the master's own hrefs and values under the prefix. So the sibling now
|
||||
materializes immediately with the master's own climate state as its
|
||||
assumed starting point, rather than waiting on per-href confirmation
|
||||
that this firmware may never give (this test used to assert the
|
||||
opposite: that the liveness gate correctly held the candidate back)."""
|
||||
the master's own hrefs and values under the prefix, plus one bounded
|
||||
retry of /information/vs/0 itself. So the sibling now materializes
|
||||
immediately with the master's own climate state as its assumed starting
|
||||
point, but its own real, distinct model/serial (this test used to
|
||||
assert the opposite: that the liveness gate correctly held the
|
||||
candidate back)."""
|
||||
coordinator = _coordinator(hass)
|
||||
resources, _oic_res, _seeds = _load_device_full("airconditioner_fac_bora_205_flat")
|
||||
await _discover(coordinator, "airconditioner_fac_bora_205_flat")
|
||||
@@ -301,16 +302,26 @@ async def test_fac_bora_205_flat_fallback_clones_master_and_materializes(
|
||||
assert subdevice.seed_path == ()
|
||||
assert subdevice.flat_hrefs == tuple(sorted(resources))
|
||||
|
||||
# Only the seed Collection itself was ever probed over the network --
|
||||
# no more per-href confirmation loop to log.
|
||||
# The seed Collection and the one bounded /information/vs/0 retry were
|
||||
# probed over the network -- nothing else, no per-href confirmation loop.
|
||||
assert coordinator._subdevice_probes[f"/{_SUB_UUID}/device/0"] is False
|
||||
assert coordinator._subdevice_probes[f"/{_SUB_UUID}/information/vs/0"] is True
|
||||
allowed = {f"/{_SUB_UUID}/device/0", f"/{_SUB_UUID}/information/vs/0"}
|
||||
assert not any(
|
||||
href.startswith(f"/{_SUB_UUID}/") and href != f"/{_SUB_UUID}/device/0"
|
||||
href.startswith(f"/{_SUB_UUID}/") and href not in allowed
|
||||
for href in coordinator._subdevice_probes
|
||||
)
|
||||
|
||||
assert _climate_bound(coordinator, _SUB_UUID) is not None
|
||||
|
||||
# The confirmed /information/vs/0 reply -- not the master's cloned one --
|
||||
# is what device_info_for() reads, so the sibling's HA device shows its
|
||||
# own real model/serial rather than looking like a duplicate of the
|
||||
# master (issue #177 comment 5113518087's real hand-read capture).
|
||||
info = coordinator.device_info_for(subdevice)
|
||||
assert info["model"] == "TP2X_FAC_BORA_RAC_21K"
|
||||
assert info["model"] != coordinator.device_info.get("model")
|
||||
|
||||
# Confirms the master itself is completely unaffected by its sibling's
|
||||
# Collection endpoint not answering -- same guarantee every other
|
||||
# subdevice test in this file relies on.
|
||||
|
||||
@@ -302,9 +302,12 @@ def test_enumerate_prefixed_falls_back_to_cloning_master_state_when_device0_coll
|
||||
master href individually under the prefix to confirm one used to be the
|
||||
fallback, but a firmware that drops packets instead of 4.04ing turned
|
||||
that into a ~300s hang that took the whole config entry's setup down
|
||||
with it. So the fallback no longer touches the network at all -- it
|
||||
with it. So the fallback no longer loops over every master href -- it
|
||||
trusts the device's own subdeviceIdList claim and clones the master's
|
||||
current hrefs and values verbatim under the prefix."""
|
||||
current hrefs and values verbatim under the prefix, plus one bounded
|
||||
retry of /information/vs/0 specifically (see the dedicated test below);
|
||||
that retry also fails to answer here, so the clone is all this
|
||||
candidate ends up with."""
|
||||
resources = {
|
||||
"/subdevices/vs/0": {"x.com.samsung.da.subdeviceIdList": [_UUID]},
|
||||
"/mode/vs/0": {"m": "cool"},
|
||||
@@ -324,6 +327,38 @@ def test_enumerate_prefixed_falls_back_to_cloning_master_state_when_device0_coll
|
||||
}
|
||||
|
||||
|
||||
def test_enumerate_prefixed_flat_fallback_confirms_its_own_information():
|
||||
"""When the one bounded retry of /<uuid>/information/vs/0 *does* answer,
|
||||
its real reply overrides the master's cloned /information/vs/0 -- so the
|
||||
sibling's device_info shows its own model/serial instead of the
|
||||
master's, which would otherwise read as a duplicate device in the UI
|
||||
even though the two devices' registry identifiers are genuinely
|
||||
distinct (see device_info_for)."""
|
||||
resources = {
|
||||
"/subdevices/vs/0": {"x.com.samsung.da.subdeviceIdList": [_UUID]},
|
||||
"/information/vs/0": {"x.com.samsung.da.modelNum": "MASTER_MODEL"},
|
||||
"/mode/vs/0": {"m": "cool"},
|
||||
}
|
||||
sess = _FakeSession(
|
||||
{
|
||||
# (_UUID, 'device', '0') absent -> Collection probe fails.
|
||||
(_UUID, "information", "vs", "0"): {
|
||||
"x.com.samsung.da.modelNum": "SIBLING_MODEL",
|
||||
"x.com.samsung.da.serialNum": "SIBLING-SERIAL",
|
||||
},
|
||||
}
|
||||
)
|
||||
subdevices, extra = enumerate_subdevices(sess, resources, oic_res_links=[])
|
||||
assert len(subdevices) == 1
|
||||
subdevice = subdevices[0]
|
||||
assert subdevice.flat_hrefs == ("/information/vs/0", "/mode/vs/0", "/subdevices/vs/0")
|
||||
assert extra[f"/{_UUID}/information/vs/0"] == {
|
||||
"x.com.samsung.da.modelNum": "SIBLING_MODEL",
|
||||
"x.com.samsung.da.serialNum": "SIBLING-SERIAL",
|
||||
}
|
||||
assert extra[f"/{_UUID}/mode/vs/0"] == {"m": "cool"}
|
||||
|
||||
|
||||
def test_enumerate_prefixed_flat_fallback_is_a_no_op_with_no_master_hrefs_to_clone():
|
||||
"""A degenerate `resources` (nothing at all, not even /subdevices/vs/0 --
|
||||
can't happen via the subdeviceIdList path but shared by _probe_prefixed
|
||||
@@ -338,9 +373,11 @@ def test_enumerate_prefixed_flat_fallback_is_a_no_op_with_no_master_hrefs_to_clo
|
||||
assert extra == {}
|
||||
|
||||
|
||||
def test_enumerate_prefixed_flat_fallback_probe_log_only_reports_the_seed():
|
||||
"""Only /<uuid>/device/0 is ever probed over the network now -- no more
|
||||
per-href probing to log."""
|
||||
def test_enumerate_prefixed_flat_fallback_probe_log_only_reports_seed_and_information():
|
||||
"""Only /<uuid>/device/0 (the seed) and /<uuid>/information/vs/0 (the one
|
||||
bounded exception -- so the sibling gets its own model/serial rather
|
||||
than the master's cloned ones) are ever probed over the network now --
|
||||
no more per-href probing loop to log."""
|
||||
resources = {
|
||||
"/subdevices/vs/0": {"x.com.samsung.da.subdeviceIdList": [_UUID]},
|
||||
"/mode/vs/0": {"m": "cool"},
|
||||
@@ -350,9 +387,9 @@ def test_enumerate_prefixed_flat_fallback_probe_log_only_reports_the_seed():
|
||||
_FakeSession({}), resources, oic_res_links=[], probe_log=probes.__setitem__
|
||||
)
|
||||
assert probes[f"/{_UUID}/device/0"] is False
|
||||
assert not any(
|
||||
href.startswith(f"/{_UUID}/") and href != f"/{_UUID}/device/0" for href in probes
|
||||
)
|
||||
assert probes[f"/{_UUID}/information/vs/0"] is False
|
||||
allowed = {f"/{_UUID}/device/0", f"/{_UUID}/information/vs/0"}
|
||||
assert not any(href.startswith(f"/{_UUID}/") and href not in allowed for href in probes)
|
||||
|
||||
|
||||
def test_enumerate_prefixed_flat_fallback_does_not_cross_contaminate_a_second_uuid():
|
||||
|
||||
Reference in New Issue
Block a user