bhetherman/localthings
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Trim the identity work's comments to the contributing guidelines

Browse Source 
CONTRIBUTING.md asks for one or two sentences over an essay, a pointer
rather than a re-derivation, and module docstrings that orient rather than
document the design. The identity change was written before that guidance
was read, and its comments narrate the reasoning at roughly twice the
length the conclusions need.

Cut to the conclusion and the evidence that makes it credible, keeping
every issue reference: _resolve_identity's docstring 41 lines -> 22 (now
shorter than the two longest already in that file), rekey_entry 23 -> 14,
its module docstring 21 -> 10, resolve_device_key 24 -> 16,
is_usable_device_id 15 -> 8, and the same treatment for the inline blocks
in _resolve_identity, the CONF_DEVICE_KEY note, the redaction rationale,
and the migration suite's module docstring.

Comments only; the suite and the thirteen-mutation sweep are unchanged.
This commit is contained in:
Marc Billow
2026-08-17 05:33:49 +00:00
parent 1b8e8368ca
commit ccb4a09c65
6 changed files with 93 additions and 193 deletions
Download Patch File Download Diff File Expand all files Collapse all files
custom_components/localthings/const.py
+4 -9
View File
@@ -30,15 +30,10 @@ CONF_LEAF_KEY_PEM = "leaf_key_pem"
# output, the host itself for a placeholder-serial board -- issues
# #83/#189), so it matches what _run_discovery computes on the first poll.
CONF_SERIAL = "serial"
# The identity this entry's devices and entities are actually keyed on --
# registry.identity.resolve_device_key's output, normally the OCF device
# UUID (issue #381). Distinct from CONF_SERIAL, which stays as (a) the key
# a pre-v4 entry was minted with, so the one-time re-key knows what to
# rewrite from, and (b) the corroborating identity that tells a device
# whose `di` rotated across a factory reset apart from a different
# appliance that moved onto this address. Absent on an entry that has not
# polled since upgrading: the OCF resources are only readable from the
# device, so the coordinator adopts this on the first live poll.
# What this entry's devices and entities are keyed on -- normally the OCF
# device UUID (issue #381). CONF_SERIAL stays alongside it as the pre-v4
# key to re-key from, and as what corroborates a later change of UUID.
# Absent until the first live poll, since only the device can report it.
CONF_DEVICE_KEY = "device_key"
CONF_MODEL = "model"
CONF_MANUFACTURER = "manufacturer"
custom_components/localthings/coordinator.py
+37 -78
View File
@@ -316,14 +316,9 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
# device_key mints permanent registry keys, so it must be correct
# before the first entity registers -- a placeholder corrected once
# the first poll lands orphans the first device/entity pair instead.
#
# CONF_DEVICE_KEY is what a v4 entry stores (the OCF device UUID,
# issue #381); CONF_SERIAL is both the pre-v4 key and the fallback
# for a board that answers no OCF identity resource; and the host
# covers a pre-migration entry, matching what resolve_serial itself
# returns for a placeholder-serial board (issues #83/#189). The
# chain is ordered so an entry that has not polled since upgrading
# keeps loading under the key its registry entries already carry.
# Ordered so an entry that has not polled since upgrading still
# loads under the key its registry rows already carry: the v4 UUID
# (issue #381), else the pre-v4 serial, else the host (#83/#189).
self.device_key = (
entry.data.get(CONF_DEVICE_KEY) or entry.data.get(CONF_SERIAL) or entry.data[CONF_HOST]
)
@@ -1107,44 +1102,25 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
"""The (key, serial) this entry should be registered under, re-keying
the registries if the key has moved.
Returns both together because they have to agree: the serial is what
corroborates a later change of key, so writing this poll's serial
while *defending* the stored key against a different appliance would
quietly hand that appliance the corroboration it needs to win the
next poll. The serial is therefore only adopted alongside a key we
accepted -- never on a poll whose identity we just rejected.
Key and serial are returned together because they have to agree: the
serial corroborates a later change of key, so adopting it while
*defending* the stored key would hand a different appliance the
corroboration it needs to win the next poll.
The stored key wins by default: re-keying an entry that already has
registry entries orphans them unless the rewrite goes with it
(issue #236), so nothing here changes a key without calling
`rekey_entry` in the same breath.
Nothing changes a key without calling `rekey_entry` in the same
breath, or the existing registry rows are orphaned (issue #236).
Two things are therefore never treated as a change of identity: a
snapshot replay (issue #295), which never reached the device, and a
poll that read no UUID, since the device saying nothing is not the
device saying something different.
Four things can happen:
* A snapshot replay (issue #295) never re-keys. That path did not
reach the device, so its "reported" identity is only whatever the
snapshot happened to preserve.
* The polled identity matches what this entry is already keyed on --
the overwhelmingly common case, and a no-op beyond recording it.
* A key is stored and this poll produced no UUID -- keep it. The
device saying nothing is not the device saying something
different, and demoting a UUID-keyed entry back onto its serial
because one reconnect couldn't read /oic/d would re-key every
entity the user has for the duration of an outage.
* The identity differs. Either this is the registered appliance
under a new identity -- the one-time move onto the OCF device UUID
(issue #381), or a `di` regenerated by an OCF hard reset -- or a
*different* appliance now answers on this address. The serialNum
is what tells those apart.
That last decision is deliberately the same whether or not this
entry has adopted a UUID yet. A pre-v4 entry is the population this
change exists to move, but it is also the population that has been
running longest, so it is the last one that should lose its
registry rows to an appliance that merely happens to share its
address -- a guard `_run_discovery` has had since issue #236 and
which an unconditional "adopt whatever the device says" would have
quietly dropped for exactly those users.
A genuine difference is either the registered appliance under a new
identity -- the move onto the OCF device UUID (issue #381), or a
`di` regenerated by a hard reset -- or a different appliance on this
address, and the serialNum is what tells them apart. That test is
deliberately the same before and after an entry has adopted a UUID:
a pre-v4 entry has been running longest, so it is the last one that
should lose its rows to whatever now answers at its address.
"""
host = self._entry.data[CONF_HOST]
stored_serial = self._entry.data.get(CONF_SERIAL)
@@ -1153,40 +1129,28 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
polled_ocf = ocf_device_key(self._identity)
stored_key = self._entry.data.get(CONF_DEVICE_KEY)
# What this entry's registry rows actually carry today. A pre-v4
# entry has no CONF_DEVICE_KEY, so it is whatever the coordinator
# has always fallen back to -- see this class's __init__.
# What this entry's rows carry today: a pre-v4 entry has no
# CONF_DEVICE_KEY, so it is whatever __init__ fell back to.
current_key = stored_key if stored_key is not None else (stored_serial or host)
# `polled_serial` is already resolve_serial's output, so this is
# exactly resolve_device_key's chain: the UUID where the device
# reports one, today's serial/host answer where it doesn't.
polled_key = polled_ocf or polled_serial
if polled_key == current_key:
# Returned rather than short-circuited so a pre-v4 entry records
# the key it has always had, which is what stops the next poll
# from treating this same answer as a change.
# the key it has always had, and the next poll sees no change.
return current_key, polled_serial
if stored_key is not None and polled_ocf is None:
# Nothing to compare against: this may or may not be the
# registered appliance, so neither half is updated.
return stored_key, stored_serial or polled_serial
# Excluding the host answer is what keeps this from firing on two
# *different* placeholder-serial units: `polled_serial` is already
# resolved, so anything that fell back to the address is not an
# identity and can't corroborate anything.
# Excluding the host answer keeps this from firing on two *different*
# placeholder-serial units: an address is not an identity and can't
# corroborate anything.
same_unit = polled_serial == stored_serial and polled_serial != host
# An entry keyed on the host never made an identity claim to defend
# -- it is registered against "whatever answers at this address"
# (issues #83/#189). Any real identity is a strict improvement on
# that, so it is adopted without needing the serial to corroborate
# it; requiring corroboration would strand exactly the
# placeholder-serial boards this whole change is meant to rescue,
# since their serial resolves to the host and can never match. An
# entry with no stored serial at all has nothing to corroborate
# against either, and is treated the same way.
# A host-keyed entry (issues #83/#189) is registered against whatever
# answers at this address, so it has no claim to defend and needs no
# corroboration -- demanding it would strand exactly the
# placeholder-serial boards this exists to rescue. Same for an entry
# with no stored serial to compare against.
if not (current_key == host or same_unit or stored_serial is None):
self._log.warning(
"device at %s identifies as %r but this entry is registered as %r "
@@ -1200,10 +1164,8 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
return current_key, stored_serial or polled_serial
# Corroborated: a pre-v4 entry moving onto its UUID, the same unit
# with a regenerated `di`, or an address-keyed entry finally
# learning a real identity. Following it keeps the user's
# entity_ids, history and automations rather than stranding them on
# a key the device will never report again.
# with a regenerated `di`, or a host-keyed entry learning a real
# identity. Following it keeps the user's entity_ids and history.
self._log.info(
"device %s (serial %r) changed key from %r to %r; following it",
host,
@@ -1499,13 +1461,10 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
manufacturer=mfr,
model=model,
)
# A snapshot replay passes None: it never reached the device, so it
# has no standing to write a device key. Persisting `self.device_key`
# there would freeze a pre-v4 entry's legacy key into CONF_DEVICE_KEY
# as if a poll had confirmed it, and _resolve_identity would then
# treat the real UUID -- when a live poll finally produced one -- as
# a changed identity to defend against rather than the one-time
# adoption it is.
# A snapshot replay passes None: it never reached the device, so
# writing a key here would freeze a pre-v4 entry's legacy key in as
# if a poll had confirmed it, and the real UUID would later look
# like an identity to defend against rather than one to adopt.
self._persist_identity(None if from_snapshot else key, serial, model, mfr, device_type_name)
if not from_snapshot:
# A coverage gap is a claim about what the device reports, so only
custom_components/localthings/registry/identity.py
+22 -42
View File
@@ -66,19 +66,12 @@ def resolve_serial(raw_serial: str | None, host: str) -> str:
def is_usable_device_id(value: str | None) -> bool:
"""True for an OCF `di`/`pi` that actually identifies one unit.
Firmware that never had a UUID assigned reports OCF's nil UUID
(all-zero, dashes aside), which is identical on every unit of the
family -- the #189 failure mode transplanted onto a new field, and not
something `is_placeholder_serial`'s repeated-hex-digit rule catches,
because the dashes make more than one distinct character.
Past that, the same known-junk rules that disqualify a serialNum
disqualify a UUID: a board firmware-flashed with 'Nothing(SVC)' in one
identity field is not a board to trust in another. Anything else is
accepted as-is. A *shared but well-formed* `di` would be undetectable
here, exactly as issue #381's shared-but-well-formed serialNum is --
heuristics can't see that, which is the whole reason this moves onto a
field the protocol itself has to keep distinct.
Rejects OCF's nil UUID, which firmware that never had one assigned
reports on every unit of the family -- the #189 failure mode on a new
field, and one `is_placeholder_serial`'s repeated-digit rule misses
because the dashes make more than one distinct character. Past that the
same known-junk rules apply: a board flashed with 'Nothing(SVC)' in one
identity field is not one to trust in another.
"""
s = (value or "").strip()
if not s:
@@ -92,16 +85,11 @@ def ocf_device_key(identity: DeviceIdentity | None) -> str | None:
"""The OCF-derived half of resolve_device_key's chain, or None when the
device reported no usable UUID.
Split out because "the device has no UUID" and "the device has this
UUID" are different answers to a caller holding an existing key: the
coordinator must never demote an entry from a UUID back onto a
serialNum just because one poll couldn't read /oic/d (a reconnect, a
timeout), which is exactly what it would do if it only saw the
collapsed string resolve_device_key returns.
Normalized because the result is compared against its stored form on
every poll; firmware that changes case between reads would otherwise
look like a different appliance.
Split out because "no UUID" and "this UUID" are different answers to a
caller holding an existing key: the coordinator must never demote an
entry off its UUID just because one poll couldn't read /oic/d.
Normalized so firmware that changes case between reads doesn't look
like a different appliance.
"""
if identity is None:
return None
@@ -116,26 +104,18 @@ def resolve_device_key(identity: DeviceIdentity | None, raw_serial: str | None,
Tried in order: /oic/d's `di`, /oic/p's `pi`, the serialNum, the host.
`di` leads because it is the identifier the protocol we are already
speaking uses to address this endpoint -- if it were wrong or shared,
OCF discovery and the DTLS association would not work in the first
place. serialNum, by contrast, is a vendor-populated string no part of
the stack depends on, which is why three separate firmware families
have shipped it unusable: 'Nothing(SVC)' (#83), a flash-unset sentinel
(#189), and -- unfixable by any heuristic -- a well-formed serial
duplicated across every unit of the model (#381).
`di` leads because it is what the protocol already uses to address this
endpoint: if it were wrong or shared, OCF discovery and the DTLS
association would not work at all. serialNum is a vendor-populated
string nothing depends on, which is why three firmware families have
shipped it unusable -- 'Nothing(SVC)' (#83), a flash-unset sentinel
(#189), and a well-formed serial duplicated across every unit (#381).
`pi` is only the fallback despite the OCF spec calling it immutable:
it identifies the *platform*, so a board hosting more than one logical
OCF device shares one `pi` across all of them, reintroducing the very
collision this exists to prevent. `di` is device-scoped, which is the
granularity of a config entry.
The serial stays in the chain below both so a board that answers
neither OCF resource lands exactly where it did before this existed,
and the host stays last for the same reason -- it is an address rather
than an identity (a new DHCP lease silently makes it someone else's),
so it is strictly a last resort.
`pi` is only the fallback despite the spec calling it immutable: it is
*platform*-scoped, so a board hosting several logical OCF devices
shares one across all of them. `di` is device-scoped, the granularity
of a config entry. The serial and host stay below both so a board
answering neither resource lands where it always did.
"""
return ocf_device_key(identity) or resolve_serial(raw_serial, host)
custom_components/localthings/registry/redact.py
+5 -11
View File
@@ -36,17 +36,11 @@ _SENSITIVE_SUBSTRINGS = (
# from the SmartThings app and can carry a person's name -- the device-type
# signal we actually want from that resource is `rt`, which is not redacted.
#
# OCF's /oic/d `di` and /oic/p `pi` used to be redacted here too. They are
# not account data: they're randomly-assigned per-unit UUIDs, carrying no
# more about their owner than the appliance-internal subdeviceIdList
# diagnostics already reports for the same reason (see diagnostics.py).
# Redacting them cost more than it bought -- issue #381 was two units
# colliding on a duplicated serialNum, and the first diagnostics download
# asking whether their `di`/`pi` differed came back with both values
# blanked, so the question could only be answered by walking the reporter
# through a manual read_resource call. They are now also what
# resolve_device_key mints registry keys from, so a report that hides them
# hides the identity every entity in it is named after.
# /oic/d's `di` and /oic/p's `pi` are deliberately not redacted: they're
# randomly-assigned per-unit UUIDs rather than account data, and they are
# what registry keys are minted from (issue #381), so blanking them hides
# the identity every entity in a report is named after -- which is exactly
# what made #381's first diagnostics download unable to answer it.
_SENSITIVE_EXACT = frozenset({"n"})
custom_components/localthings/rekey.py
+16 -36
View File
@@ -1,24 +1,13 @@
"""Move an entry's registry entries from one device key to another.
The key a device's registry entries are minted from (see
registry.identity.resolve_device_key) appears in three permanent places:
the config entry's unique_id, the device registry's identifiers, and every
entity's unique_id. Changing it therefore can't be a matter of writing a
new value and restarting -- everything already in the registries would be
orphaned, and the user would find a duplicate device whose entities all
carry a `_2` suffix, with their history, area and automations attached to
the dead copy.
The key (registry.identity.resolve_device_key) is permanent in three
places, so changing it means rewriting the registries rather than storing
a new value -- anything left behind is orphaned. Rewriting rather than
recreating is what keeps an entity's entity_id, and with it its history,
area and automations.
So the key change is performed *on* the registries instead. Rewriting
beats deleting: an entity keeps its entity_id, and with it its name, area,
long-term statistics and every automation and dashboard that references
it.
Lives in its own module rather than in __init__.py because both callers
need it and they sit on opposite sides of an import edge: the v1 -> v2
migration in __init__.py, and the coordinator's first-poll adoption of the
OCF device UUID (issue #381), which can only happen once the device has
actually been reached.
Its own module because both callers need it: the v1 -> v2 migration in
__init__.py and the coordinator's first-poll adoption (issue #381).
"""
from __future__ import annotations
@@ -39,25 +28,16 @@ _LOGGER = logging.getLogger(__name__)
def rekey_entry(hass: HomeAssistant, entry: ConfigEntry, old_key: str, new_key: str) -> None:
"""Rewrite everything this entry registered under `old_key` to `new_key`.
Covers all three places the key is permanent: the entity registry
(unique_ids are f"{DOMAIN}_{key}_{state_key}"), the device registry
(identifiers are (DOMAIN, key) for the device itself and
(DOMAIN, f"{key}_{subdevice}") for each subdevice of a composite
appliance -- see coordinator.device_info_for), and the config entry's
own unique_id.
All three permanent places move together: entity unique_ids
(f"{DOMAIN}_{key}_{state_key}"), device identifiers ((DOMAIN, key), plus
(DOMAIN, f"{key}_{subdevice}") per subdevice -- see device_info_for),
and the entry's own unique_id. Leaving that last one behind would let
the config flow's duplicate check wave through a re-add of this very
appliance.
Leaving the entry's unique_id behind would half-migrate it: the config
flow's duplicate check (_abort_if_unique_id_configured) would still be
comparing new devices against the key this entry no longer uses, so
re-adding this very appliance would be waved through as a second entry.
Idempotent: a second call finds nothing left under `old_key` and does
nothing, which is what makes it safe to attempt on every poll rather
than having to track whether it has already run.
Where both keys somehow already exist, the `old_key` copy is the dead
one -- unavailable since whichever restart created the split -- so it
is removed rather than rewritten over the live entry.
Idempotent, so it is safe to attempt on every poll rather than tracking
whether it has run. Where both keys already exist the `old_key` copy is
the dead one, so it is removed rather than rewritten over the live entry.
Must run on the event loop; the registry helpers require it.
"""
tests/localthings/test_identity_migration.py
+9 -17
View File
@@ -1,24 +1,16 @@
"""Moving an existing install onto the OCF device UUID (issue #381).
Every other migration this integration has done finishes inside
`async_migrate_entry`. This one can't: the UUID is only readable from the
appliance, and an entry can load entirely from its stored snapshot while
the appliance is off (issue #295). So the config-entry version bumps up
front and the coordinator adopts the UUID on the first *live* poll,
rewriting the entity registry, the device registry and the entry's own
This migration can't finish inside `async_migrate_entry` -- the UUID is
only readable from the appliance, and an entry can load from its snapshot
while that appliance is off (issue #295) -- so the coordinator adopts it
on the first live poll, rewriting both registries and the entry's
unique_id together.
That makes this the riskiest migration in the codebase: it rewrites the
identity of registry rows a user's automations, dashboards, history and
areas all hang off. The suite is therefore organised around what an
existing user must not lose, rather than around the functions involved:
* the entity_id, and every customization carried on that registry row
* long-term statistics (keyed by entity_id -- see the end-to-end test in
tests/test_rekey_statistics_end_to_end.py, which drives a real recorder)
* the device row, its area, and a composite appliance's subdevice links
* other config entries' rows, which this must never touch
* stability: an upgrade that happens twice, or offline, must not churn
That makes it the riskiest migration here: it rewrites the identity of
rows a user's automations, history and areas hang off. These tests are
organised around what must not break rather than around the functions
involved. Statistics are covered separately, against a real recorder, in
tests/test_rekey_statistics_end_to_end.py.
"""
from __future__ import annotations