Skip to content

Commit bdf3885

Browse files
[Telink] CNET-4-12 fix on non-concurrent mode (project-chip#43562)
* [Telink] fix for CNET 4.12 This commit aimed to fix Thread network switch (7+ step of the test) * [Telink] modified non-concurrent commissioning path Now it accomplishes all the steps of CNET 4.12. * Restyled by clang-format * [Telink] addressed review issues removed obsolete calls and added comments * [Telink] moved config definition to common Zephyr config If you need to use it in your platform, override default --------- Co-authored-by: Restyled.io <commits@restyled.io>
1 parent 77b70cf commit bdf3885

5 files changed

Lines changed: 96 additions & 16 deletions

File tree

config/telink/chip-module/Kconfig.defaults

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,9 @@ config OPENTHREAD_IP6_MAX_EXT_MCAST_ADDRS
364364
default 2 if PM
365365
default 8
366366

367+
config CHIP_OPENTHREAD_NETWORK_SWITCH_PATH
368+
default y
369+
367370
endif # NET_L2_OPENTHREAD
368371

369372
config NET_TX_STACK_SIZE

config/zephyr/Kconfig

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,13 @@ config CHIP_FACTORY_RESET_ERASE_NVS
170170
configuration is supposed to be cleared on a factory reset, including
171171
device-specific entries.
172172

173+
config CHIP_OPENTHREAD_NETWORK_SWITCH_PATH
174+
bool "Keep Thread enabled when switching between commissioned datasets"
175+
default n
176+
help
177+
Keep Thread enabled while switching between commissioned
178+
datasets to reduce detached window during fail-safe rollback/connect.
179+
173180
module = MATTER
174181
module-str = Matter
175182
source "${ZEPHYR_BASE}/subsys/logging/Kconfig.template.log_config"

src/app/clusters/network-commissioning/NetworkCommissioningCluster.cpp

Lines changed: 70 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
#include <lib/core/CHIPError.h>
4040
#include <lib/core/DataModelTypes.h>
4141
#include <lib/support/AutoRelease.h>
42+
#include <lib/support/BytesToHex.h>
4243
#include <lib/support/CodeUtils.h>
4344
#include <lib/support/SafeInt.h>
4445
#include <lib/support/SortUtils.h>
@@ -49,6 +50,7 @@
4950
#include <platform/internal/DeviceNetworkInfo.h>
5051
#include <protocols/interaction_model/StatusCode.h>
5152
#include <tracing/macros.h>
53+
#include <transport/SecureSession.h>
5254

5355
namespace chip {
5456
namespace app {
@@ -61,6 +63,17 @@ using namespace chip::app::Clusters::NetworkCommissioning;
6163

6264
namespace {
6365

66+
#if !CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
67+
68+
bool IsConnectNetworkRequestOverPASE(CommandHandler & handler)
69+
{
70+
Messaging::ExchangeContext * exchangeCtx = handler.GetExchangeContext();
71+
return exchangeCtx && exchangeCtx->HasSessionHandle() && exchangeCtx->GetSessionHandle()->IsSecureSession() &&
72+
exchangeCtx->GetSessionHandle()->AsSecureSession()->GetSecureSessionType() == Transport::SecureSession::Type::kPASE;
73+
}
74+
75+
#endif // CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
76+
6477
// Note: CHIP_CONFIG_NETWORK_COMMISSIONING_DEBUG_TEXT_BUFFER_SIZE can be 0, this disables debug text
6578
using DebugTextStorage = std::array<char, CHIP_CONFIG_NETWORK_COMMISSIONING_DEBUG_TEXT_BUFFER_SIZE>;
6679

@@ -638,20 +651,49 @@ NetworkCommissioningCluster::HandleConnectNetwork(CommandHandler & handler, cons
638651

639652
mpWirelessDriver->ConnectNetwork(req.networkID, this);
640653
#else
641-
// In Non-concurrent mode postpone the final execution of ConnectNetwork until the operational
642-
// network has been fully brought up and kOperationalNetworkStarted is delivered.
643-
// mConnectingNetworkIDLen and mConnectingNetworkID contain the received SSID
644-
// As per spec, send the ConnectNetworkResponse(Success) prior to releasing the commissioning channel
645-
SendNonConcurrentConnectNetworkResponse();
654+
mConnectNetworkResponseSentEarly = false;
655+
// In non-concurrent mode there are two execution paths:
656+
// 1) PASE/BLE: send ConnectNetworkResponse early before tearing down the commissioning
657+
// transport; actual connect is started later from OnPlatformEventHandler.
658+
// 2) CASE: start connect immediately here and send ConnectNetworkResponse from OnResult
659+
// after attach finishes.
660+
if (IsConnectNetworkRequestOverPASE(handler))
661+
{
662+
// PASE path must respond before commissioning transport is torn down.
663+
mConnectNetworkResponseSentEarly = true;
664+
SendNonConcurrentConnectNetworkResponse();
665+
}
666+
else
667+
{
668+
HandleNonConcurrentConnectNetwork();
669+
}
646670
#endif
647671
return std::nullopt;
648672
}
649673

650674
std::optional<ActionReturnStatus> NetworkCommissioningCluster::HandleNonConcurrentConnectNetwork()
651675
{
652676
ByteSpan nonConcurrentNetworkID = ByteSpan(mConnectingNetworkID, mConnectingNetworkIDLen);
653-
ChipLogProgress(NetworkProvisioning, "Non-concurrent mode, Connect to Network SSID=%s",
654-
NullTerminated(mConnectingNetworkID, mConnectingNetworkIDLen).c_str());
677+
if (mFeatureFlags.Has(Feature::kThreadNetworkInterface))
678+
{
679+
constexpr size_t kThreadNetworkIdHexMax = (2 * kMaxNetworkIDLen) + 1;
680+
char threadNetworkIdHex[kThreadNetworkIdHexMax];
681+
if (Encoding::BytesToUppercaseHexString(nonConcurrentNetworkID.data(), nonConcurrentNetworkID.size(), threadNetworkIdHex,
682+
sizeof(threadNetworkIdHex)) == CHIP_NO_ERROR)
683+
{
684+
ChipLogProgress(NetworkProvisioning, "Non-concurrent mode, Connect to Thread Network ID=%s", threadNetworkIdHex);
685+
}
686+
else
687+
{
688+
ChipLogProgress(NetworkProvisioning, "Non-concurrent mode, Connect to Thread Network ID (len=%u)",
689+
static_cast<unsigned>(nonConcurrentNetworkID.size()));
690+
}
691+
}
692+
else
693+
{
694+
ChipLogProgress(NetworkProvisioning, "Non-concurrent mode, Connect to Wi-Fi SSID=%s",
695+
NullTerminated(mConnectingNetworkID, mConnectingNetworkIDLen).c_str());
696+
}
655697
mpWirelessDriver->ConnectNetwork(nonConcurrentNetworkID, this);
656698
return std::nullopt;
657699
}
@@ -790,12 +832,11 @@ void NetworkCommissioningCluster::DisconnectLingeringConnection()
790832
void NetworkCommissioningCluster::OnResult(Status commissioningError, CharSpan debugText, int32_t interfaceStatus)
791833
{
792834
auto commandHandleRef = std::move(mAsyncCommandHandle);
793-
835+
auto commandHandle = commandHandleRef.Get();
794836
// In Non-concurrent mode the commandHandle will be null here, the ConnectNetworkResponse
795837
// has already been sent and the BLE will have been stopped, however the other functionality
796838
// is still required
797839
#if CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
798-
auto commandHandle = commandHandleRef.Get();
799840
if (commandHandle == nullptr)
800841
{
801842
// When the platform shut down, interaction model engine will invalidate all commandHandle to avoid dangling references.
@@ -825,14 +866,23 @@ void NetworkCommissioningCluster::OnResult(Status commissioningError, CharSpan d
825866
SetLastNetworkId(ByteSpan{ mConnectingNetworkID, mConnectingNetworkIDLen });
826867
SetLastNetworkingStatusValue(MakeNullable(commissioningError));
827868

869+
bool shouldSendConnectNetworkResponse = true;
828870
#if (CONFIG_NETWORK_LAYER_BLE || CHIP_DEVICE_CONFIG_ENABLE_THREAD_MESHCOP) && !CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
829-
ChipLogProgress(NetworkProvisioning, "Non-concurrent mode, ConnectNetworkResponse will NOT be sent");
830-
// Do not send the ConnectNetworkResponse if in non-concurrent mode
831-
// TODO(#30576) raised to modify CommandHandler to notify it if no response required
832-
// -----> Is this required here: commandHandle->FinishCommand();
833-
#else
834-
commandHandle->AddResponse(mAsyncCommandPath, response);
835-
#endif // CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
871+
if (mConnectNetworkResponseSentEarly)
872+
{
873+
ChipLogProgress(NetworkProvisioning, "Non-concurrent mode, ConnectNetworkResponse was already sent");
874+
shouldSendConnectNetworkResponse = false;
875+
}
876+
#endif
877+
878+
if (shouldSendConnectNetworkResponse && commandHandle != nullptr)
879+
{
880+
commandHandle->AddResponse(mAsyncCommandPath, response);
881+
}
882+
883+
#if !CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
884+
mConnectNetworkResponseSentEarly = false;
885+
#endif
836886

837887
if (commissioningError == Status::kSuccess)
838888
{
@@ -932,6 +982,10 @@ void NetworkCommissioningCluster::OnFailSafeTimerExpired()
932982
{
933983
VerifyOrReturn(mpWirelessDriver != nullptr);
934984

985+
#if !CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
986+
mConnectNetworkResponseSentEarly = false;
987+
#endif
988+
935989
ChipLogDetail(Zcl, "Failsafe timeout, tell platform driver to revert network credentials.");
936990
TEMPORARY_RETURN_IGNORED mpWirelessDriver->RevertConfiguration();
937991
mAsyncCommandHandle.Release();

src/app/clusters/network-commissioning/NetworkCommissioningCluster.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,12 @@ class NetworkCommissioningCluster : private NetworkCommissioningLogicListNode,
240240
uint8_t mLastNetworkIDLen = 0;
241241
Optional<uint64_t> mCurrentOperationBreadcrumb;
242242
bool mScanningWasDirected = false;
243+
#if !CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
244+
// Tracks whether ConnectNetworkResponse was already sent from HandleConnectNetwork()
245+
// on the PASE/BLE path. OnResult() uses this to avoid sending a duplicate response
246+
// after the attach callback, and the flag is reset at command start / fail-safe expiry.
247+
bool mConnectNetworkResponseSentEarly = false;
248+
#endif
243249
Context mClusterContext;
244250

245251
void SetLastNetworkingStatusValue(NetworkCommissioning::Attributes::LastNetworkingStatus::TypeInfo::Type networkingStatusValue);

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,16 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_AttachToThreadN
391391

392392
// Reset the previously set callback since it will never be called in case incorrect dataset was supplied.
393393
mpConnectCallback = nullptr;
394+
395+
#if defined(CONFIG_CHIP_OPENTHREAD_NETWORK_SWITCH_PATH) && CONFIG_CHIP_OPENTHREAD_NETWORK_SWITCH_PATH
396+
if (callback == nullptr && dataset.IsCommissioned() && current_dataset.IsCommissioned() &&
397+
!dataset.AsByteSpan().data_equal(current_dataset.AsByteSpan()) && Impl()->IsThreadEnabled())
398+
{
399+
ReturnErrorOnFailure(Impl()->SetThreadProvision(dataset.AsByteSpan()));
400+
return CHIP_NO_ERROR;
401+
}
402+
#endif
403+
394404
ReturnErrorOnFailure(Impl()->SetThreadEnabled(false));
395405
ReturnErrorOnFailure(Impl()->SetThreadProvision(dataset.AsByteSpan()));
396406

0 commit comments

Comments
 (0)