Import Thread datasets from Matter border routers - #179102
Conversation
|
Hey there @home-assistant/matter, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
912c749 to
083e1a9
Compare
There was a problem hiding this comment.
Pull request overview
Imports Thread datasets from Matter border routers into Home Assistant’s shared Thread dataset store.
Changes:
- Adds dataset discovery, import, timestamp tracking, and subscriptions.
- Adds Thread ordering as an optional after-dependency.
- Adds fixtures, tests, and a Matter test-helper fix.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
homeassistant/components/matter/adapter.py |
Schedules imports and subscribes to dataset changes. |
homeassistant/components/matter/thread_border_router.py |
Reads and stores border-router datasets. |
homeassistant/components/matter/manifest.json |
Adds the Thread after-dependency. |
tests/components/matter/common.py |
Corrects mocked node lookup. |
tests/components/matter/fixtures/nodes/thread_border_router.json |
Adds a border-router fixture. |
tests/components/matter/test_thread_border_router.py |
Tests import and coexistence behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
083e1a9 to
d781f6c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
homeassistant/components/matter/adapter.py:227
- Tie the retry timer to the config entry's unload lifecycle. Otherwise a pending timer can fire after Matter has unloaded, access the stopped client, and add a new background task after unload has already cancelled tracked tasks.
async_call_later(self.hass, THREAD_DATASET_RETRY_DELAY, _retry)
tests/components/matter/test_thread_border_router.py:34
- Stub the discovery timeout in this fixture as well as zeroconf. The mocked listener never reports a router, so each dataset addition leaves
_set_preferred_dataset_if_only_networkwaiting for the production 30-second timeout; subsequentasync_block_till_done()calls can make these tests take 30 seconds each.
@pytest.fixture(autouse=True)
def mock_thread_discovery(mock_async_zeroconf: MagicMock) -> MagicMock:
"""Adding a dataset starts Thread discovery, which would open a real socket."""
return mock_async_zeroconf
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
homeassistant/components/matter/adapter.py:227
- Register the retry timer with the config entry so it is canceled on unload. As written, a
NodeNotReadytimer survives a Matter reload, then uses the disconnected old client and can create a new background task after that entry's lifecycle has ended.
async_call_later(self.hass, THREAD_DATASET_RETRY_DELAY, _retry)
homeassistant/components/matter/adapter.py:217
- Add coverage for the
NodeNotReadyrecovery path. None of the new tests raises this exception, so timestamp clearing, delayed retry execution, and retry cleanup can regress without detection.
except NodeNotReady:
# The node is mid-resubscription after a restart; the availability
# event can even arrive while it is still not ready, so a plain
# retrigger is not enough. Try again once things have settled.
self._thread_dataset_timestamps.pop(key, None)
d781f6c to
23763a5
Compare
23763a5 to
c44cd86
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (3)
homeassistant/components/matter/adapter.py:176
- Invoke dataset scheduling from endpoint-added events as well. This path is only reached through
_setup_node, whileendpoint_added_callback(lines 84–93) only calls_setup_endpoint; anENDPOINT_ADDEDevent can be the sole notification for a new endpoint, so a newly exposed border-router endpoint is not imported or subscribed until an unrelated node update occurs.
try:
homeassistant/components/matter/adapter.py:175
- Replace this explanation with the actual reason for the call placement. The dataset read runs in a background task, so its exceptions cannot be caught by the preceding synchronous
tryregardless of this call's placement; the relevant behavior is that imports are still scheduled after entity setup fails.
def _setup_node(self, node: MatterNode) -> None:
"""Set up an node."""
LOGGER.debug("Setting up entities for node %s", node.node_id)
homeassistant/components/matter/adapter.py:227
- Cancel delayed retries when the config entry unloads.
async_call_laterreturns a cancellation callback, but it is dropped here; unloading Matter during this delay lets_retrylater access the disconnected client and create a background task after unload processing has already canceled tracked tasks.
dataset changes again.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
homeassistant/components/matter/adapter.py:113
- Clear the Thread import state in
node_removed_callbackbefore looking up the node. That callback returns when the client has already evicted the node, so without anENDPOINT_REMOVEDevent these entries remain and a later node reusing the same ID and timestamp can have its dataset import suppressed.
key = (data["node_id"], data["endpoint_id"])
if unsubscribe := self._thread_dataset_subscriptions.pop(key, None):
unsubscribe()
self._thread_dataset_timestamps.pop(key, None)
homeassistant/components/matter/adapter.py:209
- Re-import when the preferred border router's extended address changes, not only when the dataset timestamp changes.
DatasetStore._async_maybe_update_preferred_border_agentexplicitly refreshes this address because a router can regenerate it while keeping the same dataset and border-agent ID; the timestamp-only cache and subscription leave Matter-sourced entries stale until another dataset change or restart.
timestamp = get_active_dataset_timestamp(endpoint)
# _setup_node also runs on every node update; only re-read the
# dataset when its timestamp shows it actually changed.
if key in self._thread_dataset_timestamps and (
self._thread_dataset_timestamps[key] == timestamp
97c56b4 to
b7309c0
Compare
b7309c0 to
8bf3599
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
homeassistant/components/matter/thread_border_router.py:89
- Decode with
validate=Trueso malformed base64 is rejected. The default decoder silently discards non-alphabet characters, so a corrupted value containing an otherwise valid dataset can be imported instead of taking the error path.
return b64decode(raw)
homeassistant/components/matter/adapter.py:278
- Bound or availability-gate the
NodeNotReadyretries. Every retry failure schedules another timer, so an offline or persistently unready border router is polled every 30 seconds indefinitely; rely on the availability event after a bounded retry or track a retry limit.
if cancel_previous := self._thread_dataset_retries.pop(key, None):
cancel_previous()
self._thread_dataset_retries[key] = async_call_later(
self.hass, THREAD_DATASET_RETRY_DELAY, _retry
)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
homeassistant/components/matter/adapter.py:225
- Skip dataset imports while the node is unavailable.
setup_nodes()also visits cached offline nodes, and every resultingNodeNotReadycurrently rearms the 30-second callback, so an offline border router is polled indefinitely; the existingNODE_UPDATEDpath will schedule the import once it becomes available.
for endpoint in get_border_router_endpoints(node):
homeassistant/components/matter/thread_border_router.py:89
- Decode the response with strict base64 validation.
b64decode()defaults tovalidate=Falseand silently discards non-alphabet characters, so malformed values such as invalid characters prefixed to an otherwise valid dataset are imported instead of being rejected as this function intends.
return b64decode(raw)
8bf3599 to
f119e91
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
homeassistant/components/matter/thread_border_router.py:113
- Distinguish malformed responses from a legitimate empty dataset before marking the import complete.
_dataset_from_responsealso returnsNonefor missing, wrong-typed, or invalid-base64 payloads, so this normal return leaves the adapter's timestamp cached and suppresses every later read while the timestamp is unchanged. Propagate a parse-failure status or exception so the adapter clears/retries that state, while keeping empty bytes as the unprovisioned case.
dataset = _dataset_from_response(response)
if not dataset:
f119e91 to
f7ad0de
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
homeassistant/components/matter/thread_border_router.py:140
- Guard the delayed preferred-dataset selection before enabling this automatic add. On a first import,
DatasetStore.async_addstarts a discovery task that later assignspreferred_datasetwithout rechecking it (dataset_store.py:355-366, 492-496), so a user or migration choosing another network during that window is overwritten; merge #179098 or add its equivalent guard first.
await async_add_dataset(hass, DOMAIN, dataset.hex(), **preferred)
A router implementing the Network Infrastructure Manager device type carries the Thread Border Router Management cluster, which lets an authorised member of the Matter fabric read the active operational dataset over an authenticated session. That is an alternative to reading the same credentials from a vendor specific API such as the OpenThread Border Router REST interface. When a node exposes a border router endpoint, read its active dataset by command and add it to the Thread dataset store with source "matter". The read is keyed on the active dataset timestamp so node updates do not reissue the command while the dataset is unchanged, and an attribute subscription on that timestamp re-imports when the dataset changes underneath a quiet node, which is exactly what a scheduled migration does: without it the change would go unnoticed until the next interview. Three details are load bearing and are covered by tests: - The Matter client returns command responses as plain dicts with octet strings base64 encoded, so the dataset cannot be read off the response as an attribute. - A border router with no Thread stack running reports ExtAddress as NullValue rather than omitting it. NullValue is not falsy, and the store rejects a preferred border agent ID that is not accompanied by an extended address, so it has to be excluded explicitly. - The get_node mock in the Matter test helpers found the requested node and then returned None; this is its first caller, so the helper is fixed here. Thread is declared as an after dependency rather than a dependency. Matter does not require Thread, and making it a hard dependency would set up the Thread integration, and so zeroconf, for every Matter config entry. Assisted-By: Claude Opus 5
The store already handles two integrations writing to it: async_add() deduplicates on dataset and extended PAN ID, and the preference setter only acts when none is stored or the border agent ID matches. Assert that importing over Matter leaves an otbr-seeded entry alone, and cover the NodeNotReady retry (including cancellation on unload and replacement on re-arm), state cleanup on endpoint and node removal, and the response shapes the dataset read accepts. Assisted-By: Claude Opus 5
f7ad0de to
9199bfb
Compare
|
How can I test this PR? |
|
You can e.g. set up an OpenWrt device (with a Thread dongle/antenna) as TBR + Matter NIM. With that, one can then migrate devices between HA's internal TBR and the external OpenWrt one. |
Can OpenWRT be run in a VM with a Thread dongle/antenna? |
@lboue I don't see why not, but it's not something I've tested. |
Proposed change
A router implementing the Network Infrastructure Manager device type carries the Thread
Border Router Management cluster, which lets an authorised member of the Matter fabric
read the active operational dataset over an authenticated session -- an alternative to
reading the same credentials from a vendor-specific API such as the OTBR REST interface.
When a node exposes a border router endpoint, read its active dataset by command and add
it to the Thread dataset store with source "matter". The read is keyed on the active
dataset timestamp so node updates do not reissue the command while the dataset is
unchanged, and an attribute subscription on that timestamp re-imports when the dataset
changes underneath a quiet node -- which is exactly what a scheduled migration does.
Details covered by tests:
dataset cannot be read off the response as an attribute.
than omitting it; the store rejects a preferred border agent ID without an extended
address, so NullValue is excluded explicitly.
returned None; this is its first caller, so the helper is fixed here.
the same network over Matter, and asserts no duplicate entry and no change to the
entry's source or preferred border agent.
Thread is declared as an after dependency, not a dependency: Matter does not require
Thread, and a hard dependency would set up the Thread integration (and so zeroconf) for
every Matter config entry.
Type of change
Additional information
Should land after Don't overwrite a Thread preference chosen during discovery #179098, which guards the store's delayed preferred-dataset selection this import can trigger; the race predates this PR and every async_add_dataset caller is exposed to it.
Part of a series adding Matter Network Infrastructure Manager (NIM) support to the matter integration;
follow-ups will build on the import machinery added here. See dev...LorbusChris:homeassistant-core:ha6-matter-thread-migration for the full merge train.
Written with AI assistance (Claude Code); the commits carry
Assisted-Bytrailers.I've reviewed and understand all of it, and I'll be answering questions myself.
This PR fixes or closes issue: fixes #
This PR is related to issue:
Add pending-dataset TLV write support home-assistant-libs/python-otbr-api#269,
Refuse a channel change while a pending dataset is in place home-assistant-libs/python-otbr-api#272,
Add an OTBR action to migrate the whole Thread network #178291,
Report the outcome of adding a Thread dataset #178306,
Don't overwrite a Thread preference chosen during discovery #179098
Link to documentation pull request:
Link to developer documentation pull request:
Link to frontend pull request:
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: