Don't stop trying BLE if mDNS commissioning fails #fixes 43364#43527
Open
woody-apple wants to merge 8 commits intoproject-chip:masterfrom
Open
Don't stop trying BLE if mDNS commissioning fails #fixes 43364#43527woody-apple wants to merge 8 commits intoproject-chip:masterfrom
woody-apple wants to merge 8 commits intoproject-chip:masterfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request improves the reliability of the commissioning process by allowing discovery on other transports (like BLE) to continue even if one transport (like mDNS) fails or is slow to establish a PASE session. The changes correctly modify the discovery timeout and error handling logic to be more resilient. A new helper function StopPairingIfTransportsExhausted is introduced to centralize the logic for failing the pairing process, which is a good improvement. My only feedback is a minor one on a comment that could be more accurate.
Note: Security Review is unavailable for this PR.
|
PR #43527: Size comparison from 6d0caa2 to 400474a Full report (6 builds for cc32xx, nrfconnect, realtek, stm32)
|
jtung-apple
force-pushed
the
issue/43364
branch
from
0e7795e to
239c702
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Updates controller setup-code pairing logic so a DNS-SD discovery timeout won’t prematurely stop physical-proximity transports (BLE / Wi‑Fi PAF / NFC) when a PASE attempt is already underway, improving commissioning reliability when mDNS-based attempts fail.
Changes:
- Adjust discovery-timeout handling to stop DNS-SD only (when PASE is in progress) and introduce a helper to fail only when all transports are exhausted.
- Stop DNS-SD after a DNS-SD-triggered PASE attempt completes to avoid discovery being “stuck on” indefinitely.
- Add unit tests and a test-access helper to validate timeout behavior and transport state preservation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/controller/SetUpCodePairer.cpp | Refines timeout/transport-stop logic; adds “transports exhausted” failure path; stops DNS-SD after IP PASE attempts. |
| src/controller/SetUpCodePairer.h | Adds friend test accessor and declares StopPairingIfTransportsExhausted. |
| src/controller/tests/BUILD.gn | Registers new SetUpCodePairer unit test and test-access header. |
| src/controller/tests/TestSetUpCodePairer.cpp | Adds tests for timeout behavior with/without PASE in progress. |
| src/controller/tests/SetUpCodePairerTestAccess.h | Provides controlled access to SetUpCodePairer internals for tests. |
You can also share your feedback on Copilot code review. Take the survey.
jtung-apple
force-pushed
the
issue/43364
branch
from
a069fdc to
f50bfdb
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
You can also share your feedback on Copilot code review. Take the survey.
|
PR #43527: Size comparison from e39dcc1 to 3731169 Full report (5 builds for cc32xx, realtek, stm32)
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
You can also share your feedback on Copilot code review. Take the survey.
|
PR #43527: Size comparison from e39dcc1 to 85ece64 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #43527 +/- ##
==========================================
+ Coverage 54.07% 54.16% +0.08%
==========================================
Files 1548 1548
Lines 106701 106709 +8
Branches 13309 13281 -28
==========================================
+ Hits 57699 57799 +100
+ Misses 49002 48910 -92 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
You can also share your feedback on Copilot code review. Take the survey.
|
PR #43527: Size comparison from e39dcc1 to 2aa13fd Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
jtung-apple
approved these changes
Mar 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Starting BLE scan:
2026-02-27 09:23:47.274055-0500 0x97808 Default 0x227995 201 0 homed: (Matter) [com.csa.matter:Default] Pre-warming done
Appears over mDNS
2026-02-27 09:23:49.363666-0500 0x97813 Default 0x0 201 0 homed: (Matter) [com.csa.matter:Discovery] Resolve type=_matterc._udp name=27A648528C4C6BBC domain=local. interface=15
Appears over BLE
2026-02-27 09:23:53.831386-0500 0x9755E Default 0x0 201 0 homed: (Matter) [com.csa.matter:Controller] Discovered device to be commissioned over BLE[11:22 AM]The SDK waits a little bit for BLE, and we also do some “pre-warming” on the iOS side, to bring the BLE radio up as soon as the UI appears, instead of when we just start startcommissioning (to give BLE even more time).
Proposal:
Basically, if we fail-out DNS-SD, and then the discriminator is avaiable on BLE, try BLE. This is the same as saying "try all colliding discriminator/VID/PID over every transport" which is supposed to help reliability
Related issues
#43364
Testing
Manual
Readability checklist
The checklist below will help the reviewer finish PR review in time and keep the
code readable:
descriptive
“When in Rome…”
rule (coding style)
See: Pull Request Guidelines