[Telink] CNET-4-12 fix on non-concurrent mode#43562
Open
interfer wants to merge 4 commits intoproject-chip:masterfrom
Open
[Telink] CNET-4-12 fix on non-concurrent mode#43562interfer wants to merge 4 commits intoproject-chip:masterfrom
interfer wants to merge 4 commits intoproject-chip:masterfrom
Conversation
This commit aimed to fix Thread network switch (7+ step of the test)
Now it accomplishes all the steps of CNET 4.12.
![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 introduces a new Kconfig option CHIP_TELINK_OPENTHREAD_NETWORK_SWITCH_PATH to allow Thread to remain enabled when switching between commissioned datasets, reducing network detachment time. It also refactors the NetworkCommissioningCluster to handle ConnectNetwork responses differently based on the session type (PASE/BLE vs. CASE) in non-concurrent mode, sending responses early for PASE/BLE requests to prevent commissioning transport teardown issues. Additionally, it enhances logging for Thread network IDs and introduces a new member mConnectNetworkResponseSentEarly to manage response sending state.
interfer
force-pushed
the
telink_non-concurrect-cnet-4-12-fix
branch
from
d42c612 to
62230e6
Compare
|
PR #43562: Size comparison from e39dcc1 to 62230e6 Full report (5 builds for esp32, nxp, realtek, stm32)
|
interfer
force-pushed
the
telink_non-concurrect-cnet-4-12-fix
branch
from
62230e6 to
b72af96
Compare
|
PR #43562: Size comparison from e39dcc1 to b72af96 Full report (13 builds for cc13x4_26x4, cc32xx, psoc6, realtek, stm32)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #43562 +/- ##
==========================================
- Coverage 54.07% 54.07% -0.01%
==========================================
Files 1548 1548
Lines 106701 106709 +8
Branches 13309 13314 +5
==========================================
Hits 57699 57699
- Misses 49002 49010 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
interfer
force-pushed
the
telink_non-concurrect-cnet-4-12-fix
branch
from
b72af96 to
89b0f38
Compare
|
PR #43562: Size comparison from e39dcc1 to 89b0f38 Full report (26 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32)
|
s07641069
approved these changes
Mar 12, 2026
interfer
force-pushed
the
telink_non-concurrect-cnet-4-12-fix
branch
from
89b0f38 to
0b7cc90
Compare
|
PR #43562: Size comparison from e39dcc1 to 0b7cc90 Full report (48 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
andy31415
reviewed
Mar 12, 2026
src/app/clusters/network-commissioning/NetworkCommissioningCluster.cpp
Outdated
Show resolved
Hide resolved
andy31415
reviewed
Mar 12, 2026
andy31415
reviewed
Mar 12, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses TC-CNET-4.12 failures in non-concurrent networking mode by making ConnectNetwork response handling aware of the secure session type (PASE vs CASE) and by introducing a Telink-configurable OpenThread “fast dataset switch” path to reduce disruption during Thread-to-Thread transitions.
Changes:
- Adjust non-concurrent
ConnectNetworkhandling to send the response either early (PASE) or after attach completes (CASE), preventing timeouts and improving sequencing. - Add explicit state tracking (
mConnectNetworkResponseSentEarly) to avoid duplicate/missing responses in the updated flow. - Add an optional Telink Kconfig feature to switch commissioned Thread datasets without a full Thread disable/enable cycle.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp | Adds an optional “fast dataset switch” path when switching between commissioned datasets under a Telink-controlled config. |
| src/app/clusters/network-commissioning/NetworkCommissioningCluster.h | Adds a non-concurrent-only flag to track whether ConnectNetworkResponse was sent early. |
| src/app/clusters/network-commissioning/NetworkCommissioningCluster.cpp | Makes ConnectNetwork response emission PASE/CASE-aware; improves Thread network ID logging with hex formatting. |
| config/telink/chip-module/Kconfig | Introduces CHIP_OPENTHREAD_NETWORK_SWITCH_PATH to enable the fast dataset switch behavior by default for Telink. |
src/app/clusters/network-commissioning/NetworkCommissioningCluster.cpp
Outdated
Show resolved
Hide resolved
src/app/clusters/network-commissioning/NetworkCommissioningCluster.cpp
Outdated
Show resolved
Hide resolved
src/app/clusters/network-commissioning/NetworkCommissioningCluster.cpp
Outdated
Show resolved
Hide resolved
src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp
Outdated
Show resolved
Hide resolved
Contributor
Contributor
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a fix for the CNET-4-12 test case in non-concurrent mode by making the ConnectNetwork response transport-aware (PASE vs. CASE) and adding a fast path for Thread-to-Thread network switching. The changes in NetworkCommissioningCluster.cpp to handle response timing based on the session type are well-implemented and improve robustness. The addition of a fast dataset switch path in GenericThreadStackManagerImpl_OpenThread.hpp is a good optimization to reduce network downtime. I have one suggestion to ensure the fast path is correctly triggered for ConnectNetwork commands.
removed obsolete calls and added comments
|
PR #43562: Size comparison from e39dcc1 to 46d3a9b Full report (48 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
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
Since #41702 has been introduced we've migrated our platform to using non-concurrent. But CNET-4.12 has been failing on step 8 and later after little fix on our platform side it got stuck on 12th, as well as Nordic does, for comparison.
NetworkCommissioningCluster.cpp handles ble-thread transition fine but as far as I understand there's no thread-thread path, as we need it in this test case.
And I've investigated response was missing for that "being in 1st thread network attach to 2nd network" step so
networkcommissioning connect-networkcommand would finish properly and DUT could attach indeed. It did sent response in step 7 but actually in never communicated on 2nd network in further steps.Changes overview
ConnectNetworkresponse is now transport-aware (PASE vs CASE) inNetworkCommissioningCluster.cpp. Previously non-concurrent flow assumed response was already sent and could skip sending in cases where it was actually still required.Now:
OnResult()after attach completes.This way it avoids “missing response / timeout” behavior when the command is issued over CASE.
Added explicit state tracking for early response in
NetworkCommissioningClusterwith new flagmConnectNetworkResponseSentEarly. So it prevents double responses and ensures exactly one validConnectNetworkResponseis emitted.Added Telink-only fast dataset switch path in OpenThread attach. When enabled, and only for thread-to-thread switch with no callback path(not as for ble-thread), code updates dataset without full Thread disable/enable cycle.
This way it reduces detached window during rollback/switch, improving chances to keep command/session flow stable in non-concurrent mode.
Improved connect logging for Thread, Thread path now logs Extended PAN ID (hex) instead of SSID-style text.
So Thread network ID diagnostics become accurate and easier to correlate with datasets.
Related issues
#40370
#40629
project-chip/matter-test-scripts#583
Hi @emessina-max , I just wanted to reach you out to inform I'd like to apply changes amongst those you did earlier in #41702.
Maybe you could give some advice on what I'm trying to do here (if you have time of course)?
Testing
src/python_testing/TC_CNET_4_12.pyhaving two OTBR (RPI based) with separate networks, as test prerequisites demand