From fadaf48f9308fb2b028f921b0cce7eeaab5b3f4a Mon Sep 17 00:00:00 2001 From: Song Guo Date: Wed, 11 May 2022 08:57:35 +0800 Subject: [PATCH 1/4] [nwprov] Breadcrumb support --- .../network-commissioning.cpp | 51 +++++++++++++++++-- .../network-commissioning.h | 4 ++ .../test_scripts/network_commissioning.py | 37 +++++++++++--- 3 files changed, 80 insertions(+), 12 deletions(-) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index 784a6c77ac6fb4..393bbfe235f041 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -18,6 +18,7 @@ #include "network-commissioning.h" +#include #include #include #include @@ -267,12 +268,14 @@ void Instance::HandleScanNetworks(HandlerContext & ctx, const Commands::ScanNetw ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::InvalidCommand); return; } - mAsyncCommandHandle = CommandHandler::Handle(&ctx.mCommandHandler); + mLastOperationBreadcrumb = req.breadcrumb; + mAsyncCommandHandle = CommandHandler::Handle(&ctx.mCommandHandler); mpDriver.Get()->ScanNetworks(ssid, this); } else if (mFeatureFlags.Has(NetworkCommissioningFeature::kThreadNetworkInterface)) { - mAsyncCommandHandle = CommandHandler::Handle(&ctx.mCommandHandler); + mLastOperationBreadcrumb = req.breadcrumb; + mAsyncCommandHandle = CommandHandler::Handle(&ctx.mCommandHandler); mpDriver.Get()->ScanNetworks(this); } else @@ -362,6 +365,10 @@ void Instance::HandleAddOrUpdateWiFiNetwork(HandlerContext & ctx, const Commands mpDriver.Get()->AddOrUpdateNetwork(req.ssid, req.credentials, debugText, outNetworkIndex); FillDebugTextAndNetworkIndex(response, debugText, outNetworkIndex); ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response); + if (response.networkingStatus == NetworkCommissioningStatus::kSuccess) + { + KickBreadcrumbChange(req.breadcrumb); + } } void Instance::HandleAddOrUpdateThreadNetwork(HandlerContext & ctx, const Commands::AddOrUpdateThreadNetwork::DecodableType & req) @@ -381,6 +388,22 @@ void Instance::HandleAddOrUpdateThreadNetwork(HandlerContext & ctx, const Comman mpDriver.Get()->AddOrUpdateNetwork(req.operationalDataset, debugText, outNetworkIndex); FillDebugTextAndNetworkIndex(response, debugText, outNetworkIndex); ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response); + if (response.networkingStatus == NetworkCommissioningStatus::kSuccess) + { + KickBreadcrumbChange(req.breadcrumb); + } +} + +void Instance::KickBreadcrumbChange(const Optional & breadcrumb) +{ + VerifyOrReturn(breadcrumb.HasValue()); + GeneralCommissioning::Attributes::Breadcrumb::Set(0, breadcrumb.Value()); +} + +void Instance::KickBreadcrumbChange() +{ + KickBreadcrumbChange(mLastOperationBreadcrumb); + mLastOperationBreadcrumb.ClearValue(); } void Instance::HandleRemoveNetwork(HandlerContext & ctx, const Commands::RemoveNetwork::DecodableType & req) @@ -399,6 +422,10 @@ void Instance::HandleRemoveNetwork(HandlerContext & ctx, const Commands::RemoveN response.networkingStatus = mpWirelessDriver->RemoveNetwork(req.networkID, debugText, outNetworkIndex); FillDebugTextAndNetworkIndex(response, debugText, outNetworkIndex); ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response); + if (response.networkingStatus == NetworkCommissioningStatus::kSuccess) + { + KickBreadcrumbChange(req.breadcrumb); + } } void Instance::HandleConnectNetwork(HandlerContext & ctx, const Commands::ConnectNetwork::DecodableType & req) @@ -414,8 +441,8 @@ void Instance::HandleConnectNetwork(HandlerContext & ctx, const Commands::Connec mConnectingNetworkIDLen = static_cast(req.networkID.size()); memcpy(mConnectingNetworkID, req.networkID.data(), mConnectingNetworkIDLen); - - mAsyncCommandHandle = CommandHandler::Handle(&ctx.mCommandHandler); + mAsyncCommandHandle = CommandHandler::Handle(&ctx.mCommandHandler); + mLastOperationBreadcrumb = req.breadcrumb; mpWirelessDriver->ConnectNetwork(req.networkID, this); } @@ -431,6 +458,10 @@ void Instance::HandleReorderNetwork(HandlerContext & ctx, const Commands::Reorde response.networkingStatus = mpWirelessDriver->ReorderNetwork(req.networkID, req.networkIndex, debugText); FillDebugTextAndNetworkIndex(response, debugText, req.networkIndex); ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response); + if (response.networkingStatus == NetworkCommissioningStatus::kSuccess) + { + KickBreadcrumbChange(req.breadcrumb); + } } void Instance::OnResult(Status commissioningError, CharSpan debugText, int32_t interfaceStatus) @@ -467,6 +498,10 @@ void Instance::OnResult(Status commissioningError, CharSpan debugText, int32_t i mLastNetworkingStatusValue.SetNonNull(commissioningError); commandHandle->AddResponse(mPath, response); + if (commissioningError == NetworkCommissioningStatus::kSuccess) + { + KickBreadcrumbChange(); + } } void Instance::OnFinished(Status status, CharSpan debugText, ThreadScanResponseIterator * networks) @@ -529,6 +564,10 @@ void Instance::OnFinished(Status status, CharSpan debugText, ThreadScanResponseI { ChipLogError(Zcl, "Failed to encode response: %s", err.AsString()); } + if (status == NetworkCommissioningStatus::kSuccess) + { + KickBreadcrumbChange(); + } networks->Release(); } @@ -588,6 +627,10 @@ void Instance::OnFinished(Status status, CharSpan debugText, WiFiScanResponseIte { ChipLogError(Zcl, "Failed to encode response: %s", err.AsString()); } + if (status == NetworkCommissioningStatus::kSuccess) + { + KickBreadcrumbChange(); + } if (networks != nullptr) { networks->Release(); diff --git a/src/app/clusters/network-commissioning/network-commissioning.h b/src/app/clusters/network-commissioning/network-commissioning.h index 3edee1924b098e..e18eee18332bac 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.h +++ b/src/app/clusters/network-commissioning/network-commissioning.h @@ -100,6 +100,10 @@ class Instance : public CommandHandlerInterface, uint8_t mLastNetworkID[DeviceLayer::NetworkCommissioning::kMaxNetworkIDLen]; uint8_t mLastNetworkIDLen = 0; + Optional mLastOperationBreadcrumb; + void KickBreadcrumbChange(const Optional & breadcrumbValue); + void KickBreadcrumbChange(); + // Actual handlers of the commands void HandleScanNetworks(HandlerContext & ctx, const Commands::ScanNetworks::DecodableType & req); void HandleAddOrUpdateWiFiNetwork(HandlerContext & ctx, const Commands::AddOrUpdateWiFiNetwork::DecodableType & req); diff --git a/src/controller/python/test/test_scripts/network_commissioning.py b/src/controller/python/test/test_scripts/network_commissioning.py index 64f7f7fd7f8ffd..1b13928358c0a1 100644 --- a/src/controller/python/test/test_scripts/network_commissioning.py +++ b/src/controller/python/test/test_scripts/network_commissioning.py @@ -21,6 +21,7 @@ from chip.clusters.Types import NullValue import chip.interaction_model import asyncio +import random import base @@ -55,6 +56,18 @@ class NetworkCommissioningTests: def __init__(self, devCtrl, nodeid): self._devCtrl = devCtrl self._nodeid = nodeid + self._last_breadcrumb = random.randint(1, 1 << 48) + + async def must_verify_breadcrumb(self): + res = await self._devCtrl.ReadAttribute(nodeid=self._nodeid, attributes=[(0, Clusters.GeneralCommissioning.Attributes.Breadcrumb)], returnClusterObject=True) + if self._last_breadcrumb is not None: + if self._last_breadcrumb != res[0][Clusters.GeneralCommissioning].breadcrumb: + raise AssertionError( + f"Breadcrumb attribute mismatch! Expect {self._last_breadcrumb} got {res[0][Clusters.GeneralCommissioning].breadcrumb}") + + def with_breadcrumb(self) -> int: + self._last_breadcrumb += 1 + return self._last_breadcrumb def log_interface_basic_info(self, values): logger.info(f"The interface supports {values.maxNetworks} networks.") @@ -129,11 +142,12 @@ async def test_wifi(self, endpointId): # Scan networks logger.info(f"Scan networks") req = Clusters.NetworkCommissioning.Commands.ScanNetworks( - ssid=b'', breadcrumb=0) + ssid=b'', breadcrumb=self.with_breadcrumb()) res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req) logger.info(f"Received response: {res}") if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess: raise AssertionError(f"Unexpected result: {res.networkingStatus}") + await self.must_verify_breadcrumb() # Arm the failsafe before making network config changes logger.info(f"Arming the failsafe") @@ -149,17 +163,18 @@ async def test_wifi(self, endpointId): if len(networkList) != 0: logger.info(f"Removing existing network") req = Clusters.NetworkCommissioning.Commands.RemoveNetwork( - networkID=networkList[0].networkID, breadcrumb=0) + networkID=networkList[0].networkID, breadcrumb=self.with_breadcrumb()) res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req) logger.info(f"Received response: {res}") if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess: raise AssertionError( f"Unexpected result: {res.networkingStatus}") + await self.must_verify_breadcrumb() # Add first network logger.info(f"Adding first test network") req = Clusters.NetworkCommissioning.Commands.AddOrUpdateWiFiNetwork( - ssid=TEST_WIFI_SSID.encode(), credentials=TEST_WIFI_PASS.encode(), breadcrumb=0) + ssid=TEST_WIFI_SSID.encode(), credentials=TEST_WIFI_PASS.encode(), breadcrumb=self.with_breadcrumb()) res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req) logger.info(f"Received response: {res}") if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess: @@ -167,6 +182,7 @@ async def test_wifi(self, endpointId): if res.networkIndex != 0: raise AssertionError( f"Unexpected result: {res.networkIndex} (should be 0)") + await self.must_verify_breadcrumb() logger.info(f"Check network list") res = await self._devCtrl.ReadAttribute(nodeid=self._nodeid, attributes=[(endpointId, Clusters.NetworkCommissioning.Attributes.Networks)], returnClusterObject=True) @@ -181,12 +197,13 @@ async def test_wifi(self, endpointId): logger.info(f"Connect to a network") req = Clusters.NetworkCommissioning.Commands.ConnectNetwork( - networkID=TEST_WIFI_SSID.encode(), breadcrumb=0) + networkID=TEST_WIFI_SSID.encode(), breadcrumb=self.with_breadcrumb()) res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req) logger.info(f"Got response: {res}") if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess: raise AssertionError(f"Unexpected result: {res.networkingStatus}") logger.info(f"Device connected to a network.") + await self.must_verify_breadcrumb() # Disarm the failsafe logger.info(f"Disarming the failsafe") @@ -259,11 +276,12 @@ async def test_thread(self, endpointId): # Scan networks logger.info(f"Scan networks") req = Clusters.NetworkCommissioning.Commands.ScanNetworks( - ssid=b'', breadcrumb=0) + ssid=b'', breadcrumb=self.with_breadcrumb()) res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req) logger.info(f"Received response: {res}") if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess: raise AssertionError(f"Unexpected result: {res.networkingStatus}") + await self.must_verify_breadcrumb() # Arm the failsafe before making network config changes logger.info(f"Arming the failsafe") @@ -279,17 +297,18 @@ async def test_thread(self, endpointId): if len(networkList) != 0: logger.info(f"Removing existing network") req = Clusters.NetworkCommissioning.Commands.RemoveNetwork( - networkID=networkList[0].networkID, breadcrumb=0) + networkID=networkList[0].networkID, breadcrumb=self.with_breadcrumb()) res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req) logger.info(f"Received response: {res}") if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess: raise AssertionError( f"Unexpected result: {res.networkingStatus}") + await self.must_verify_breadcrumb() # Add first network logger.info(f"Adding first test network") req = Clusters.NetworkCommissioning.Commands.AddOrUpdateThreadNetwork( - operationalDataset=TEST_THREAD_NETWORK_DATASET_TLVS[0], breadcrumb=0) + operationalDataset=TEST_THREAD_NETWORK_DATASET_TLVS[0], breadcrumb=self.with_breadcrumb()) res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req) logger.info(f"Received response: {res}") if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess: @@ -297,6 +316,7 @@ async def test_thread(self, endpointId): if res.networkIndex != 0: raise AssertionError( f"Unexpected result: {res.networkIndex} (should be 0)") + await self.must_verify_breadcrumb() logger.info(f"Check network list") res = await self._devCtrl.ReadAttribute(nodeid=self._nodeid, attributes=[(endpointId, Clusters.NetworkCommissioning.Attributes.Networks)], returnClusterObject=True) @@ -311,12 +331,13 @@ async def test_thread(self, endpointId): logger.info(f"Connect to a network") req = Clusters.NetworkCommissioning.Commands.ConnectNetwork( - networkID=TEST_THREAD_NETWORK_IDS[0], breadcrumb=0) + networkID=TEST_THREAD_NETWORK_IDS[0], breadcrumb=self.with_breadcrumb()) res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req) logger.info(f"Got response: {res}") if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess: raise AssertionError(f"Unexpected result: {res.networkingStatus}") logger.info(f"Device connected to a network.") + await self.must_verify_breadcrumb() # Disarm the failsafe logger.info(f"Disarming the failsafe") From 01a009e60532539fc7a4686ceefff60a6675ef6f Mon Sep 17 00:00:00 2001 From: Song Guo Date: Thu, 12 May 2022 10:51:05 +0800 Subject: [PATCH 2/4] Add interface to generalcommissioning cluster to touch breadcrumb --- .../general-commissioning-server.cpp | 14 ++++++++ .../general-commissioning-server.h | 32 +++++++++++++++++++ .../network-commissioning.cpp | 3 +- 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 src/app/clusters/general-commissioning-server/general-commissioning-server.h diff --git a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp index 5dbee6f93045eb..d86487cc9a6778 100644 --- a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp +++ b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp @@ -310,3 +310,17 @@ void MatterGeneralCommissioningPluginServerInitCallback() registerAttributeAccessOverride(&gAttrAccess); DeviceLayer::PlatformMgrImpl().AddEventHandler(OnPlatformEventHandler); } + +namespace chip { +namespace app { +namespace Clusters { +namespace GeneralCommissioning { +void SetBreadcrumb(Attributes::Breadcrumb::TypeInfo::Type breadcrumb) +{ + VerifyOrReturn(DeviceLayer::DeviceControlServer::DeviceControlSvr().GetFailSafeContext().IsFailSafeArmed()); + Breadcrumb::Set(0, breadcrumb); +} +} // namespace GeneralCommissioning +} // namespace Clusters +} // namespace app +} // namespace chip diff --git a/src/app/clusters/general-commissioning-server/general-commissioning-server.h b/src/app/clusters/general-commissioning-server/general-commissioning-server.h new file mode 100644 index 00000000000000..4c66aada325787 --- /dev/null +++ b/src/app/clusters/general-commissioning-server/general-commissioning-server.h @@ -0,0 +1,32 @@ +/** + * + * Copyright (c) 2021 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +namespace chip { +namespace app { +namespace Clusters { +namespace GeneralCommissioning { + +/** + * Sets the breadcrumb attribute. If failsafe is not currently armed, this function will not change the breadcrumb attribute. + */ +void SetBreadcrumb(Attributes::Breadcrumb::TypeInfo::Type breadcrumb); +} // namespace GeneralCommissioning +} // namespace Clusters +} // namespace app +} // namespace chip diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index 393bbfe235f041..b6397b432fc7c2 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -397,7 +398,7 @@ void Instance::HandleAddOrUpdateThreadNetwork(HandlerContext & ctx, const Comman void Instance::KickBreadcrumbChange(const Optional & breadcrumb) { VerifyOrReturn(breadcrumb.HasValue()); - GeneralCommissioning::Attributes::Breadcrumb::Set(0, breadcrumb.Value()); + GeneralCommissioning::SetBreadcrumb(breadcrumb.Value()); } void Instance::KickBreadcrumbChange() From 496db2970d4475684c8dfd42c5091625e26ba4a4 Mon Sep 17 00:00:00 2001 From: Song Guo Date: Thu, 12 May 2022 14:06:43 +0800 Subject: [PATCH 3/4] Update --- .../general-commissioning-server.cpp | 1 - .../general-commissioning-server.h | 6 ++-- .../network-commissioning.cpp | 36 ++++++++++--------- .../network-commissioning.h | 6 ++-- 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp index d86487cc9a6778..fa8d755fc23467 100644 --- a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp +++ b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp @@ -317,7 +317,6 @@ namespace Clusters { namespace GeneralCommissioning { void SetBreadcrumb(Attributes::Breadcrumb::TypeInfo::Type breadcrumb) { - VerifyOrReturn(DeviceLayer::DeviceControlServer::DeviceControlSvr().GetFailSafeContext().IsFailSafeArmed()); Breadcrumb::Set(0, breadcrumb); } } // namespace GeneralCommissioning diff --git a/src/app/clusters/general-commissioning-server/general-commissioning-server.h b/src/app/clusters/general-commissioning-server/general-commissioning-server.h index 4c66aada325787..dc1cc8c7e23c63 100644 --- a/src/app/clusters/general-commissioning-server/general-commissioning-server.h +++ b/src/app/clusters/general-commissioning-server/general-commissioning-server.h @@ -1,6 +1,6 @@ /** * - * Copyright (c) 2021 Project CHIP Authors + * Copyright (c) 2022 Project CHIP Authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,10 +22,8 @@ namespace app { namespace Clusters { namespace GeneralCommissioning { -/** - * Sets the breadcrumb attribute. If failsafe is not currently armed, this function will not change the breadcrumb attribute. - */ void SetBreadcrumb(Attributes::Breadcrumb::TypeInfo::Type breadcrumb); + } // namespace GeneralCommissioning } // namespace Clusters } // namespace app diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index b6397b432fc7c2..c47452d45c23ea 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -269,14 +269,14 @@ void Instance::HandleScanNetworks(HandlerContext & ctx, const Commands::ScanNetw ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::InvalidCommand); return; } - mLastOperationBreadcrumb = req.breadcrumb; - mAsyncCommandHandle = CommandHandler::Handle(&ctx.mCommandHandler); + mCurrentOperationBreadcrumb = req.breadcrumb; + mAsyncCommandHandle = CommandHandler::Handle(&ctx.mCommandHandler); mpDriver.Get()->ScanNetworks(ssid, this); } else if (mFeatureFlags.Has(NetworkCommissioningFeature::kThreadNetworkInterface)) { - mLastOperationBreadcrumb = req.breadcrumb; - mAsyncCommandHandle = CommandHandler::Handle(&ctx.mCommandHandler); + mCurrentOperationBreadcrumb = req.breadcrumb; + mAsyncCommandHandle = CommandHandler::Handle(&ctx.mCommandHandler); mpDriver.Get()->ScanNetworks(this); } else @@ -368,7 +368,7 @@ void Instance::HandleAddOrUpdateWiFiNetwork(HandlerContext & ctx, const Commands ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response); if (response.networkingStatus == NetworkCommissioningStatus::kSuccess) { - KickBreadcrumbChange(req.breadcrumb); + UpdateBreadcrumb(req.breadcrumb); } } @@ -391,20 +391,22 @@ void Instance::HandleAddOrUpdateThreadNetwork(HandlerContext & ctx, const Comman ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response); if (response.networkingStatus == NetworkCommissioningStatus::kSuccess) { - KickBreadcrumbChange(req.breadcrumb); + UpdateBreadcrumb(req.breadcrumb); } } -void Instance::KickBreadcrumbChange(const Optional & breadcrumb) +void Instance::UpdateBreadcrumb(const Optional & breadcrumb) { VerifyOrReturn(breadcrumb.HasValue()); GeneralCommissioning::SetBreadcrumb(breadcrumb.Value()); } -void Instance::KickBreadcrumbChange() +void Instance::UpdateBreadcrumb() { - KickBreadcrumbChange(mLastOperationBreadcrumb); - mLastOperationBreadcrumb.ClearValue(); + // We rejected the command when there is another ongoing command, so mCurrentOperationBreadcrumb reflects the breadcrumb + // argument in the only background command. + UpdateBreadcrumb(mCurrentOperationBreadcrumb); + mCurrentOperationBreadcrumb.ClearValue(); } void Instance::HandleRemoveNetwork(HandlerContext & ctx, const Commands::RemoveNetwork::DecodableType & req) @@ -425,7 +427,7 @@ void Instance::HandleRemoveNetwork(HandlerContext & ctx, const Commands::RemoveN ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response); if (response.networkingStatus == NetworkCommissioningStatus::kSuccess) { - KickBreadcrumbChange(req.breadcrumb); + UpdateBreadcrumb(req.breadcrumb); } } @@ -442,8 +444,8 @@ void Instance::HandleConnectNetwork(HandlerContext & ctx, const Commands::Connec mConnectingNetworkIDLen = static_cast(req.networkID.size()); memcpy(mConnectingNetworkID, req.networkID.data(), mConnectingNetworkIDLen); - mAsyncCommandHandle = CommandHandler::Handle(&ctx.mCommandHandler); - mLastOperationBreadcrumb = req.breadcrumb; + mAsyncCommandHandle = CommandHandler::Handle(&ctx.mCommandHandler); + mCurrentOperationBreadcrumb = req.breadcrumb; mpWirelessDriver->ConnectNetwork(req.networkID, this); } @@ -461,7 +463,7 @@ void Instance::HandleReorderNetwork(HandlerContext & ctx, const Commands::Reorde ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response); if (response.networkingStatus == NetworkCommissioningStatus::kSuccess) { - KickBreadcrumbChange(req.breadcrumb); + UpdateBreadcrumb(req.breadcrumb); } } @@ -501,7 +503,7 @@ void Instance::OnResult(Status commissioningError, CharSpan debugText, int32_t i commandHandle->AddResponse(mPath, response); if (commissioningError == NetworkCommissioningStatus::kSuccess) { - KickBreadcrumbChange(); + UpdateBreadcrumb(); } } @@ -567,7 +569,7 @@ void Instance::OnFinished(Status status, CharSpan debugText, ThreadScanResponseI } if (status == NetworkCommissioningStatus::kSuccess) { - KickBreadcrumbChange(); + UpdateBreadcrumb(); } networks->Release(); } @@ -630,7 +632,7 @@ void Instance::OnFinished(Status status, CharSpan debugText, WiFiScanResponseIte } if (status == NetworkCommissioningStatus::kSuccess) { - KickBreadcrumbChange(); + UpdateBreadcrumb(); } if (networks != nullptr) { diff --git a/src/app/clusters/network-commissioning/network-commissioning.h b/src/app/clusters/network-commissioning/network-commissioning.h index e18eee18332bac..690789aa9584b7 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.h +++ b/src/app/clusters/network-commissioning/network-commissioning.h @@ -100,9 +100,9 @@ class Instance : public CommandHandlerInterface, uint8_t mLastNetworkID[DeviceLayer::NetworkCommissioning::kMaxNetworkIDLen]; uint8_t mLastNetworkIDLen = 0; - Optional mLastOperationBreadcrumb; - void KickBreadcrumbChange(const Optional & breadcrumbValue); - void KickBreadcrumbChange(); + Optional mCurrentOperationBreadcrumb; + void UpdateBreadcrumb(const Optional & breadcrumbValue); + void UpdateBreadcrumb(); // Actual handlers of the commands void HandleScanNetworks(HandlerContext & ctx, const Commands::ScanNetworks::DecodableType & req); From 3f46d3976c1ebc2e2e1481cab115fda000285867 Mon Sep 17 00:00:00 2001 From: Song Guo Date: Tue, 17 May 2022 09:47:49 +0800 Subject: [PATCH 4/4] Rename UpdateBreadcrumb --- .../network-commissioning/network-commissioning.cpp | 8 ++++---- .../network-commissioning/network-commissioning.h | 7 ++++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index c47452d45c23ea..99c586f9c99678 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -401,7 +401,7 @@ void Instance::UpdateBreadcrumb(const Optional & breadcrumb) GeneralCommissioning::SetBreadcrumb(breadcrumb.Value()); } -void Instance::UpdateBreadcrumb() +void Instance::CommitSavedBreadcrumb() { // We rejected the command when there is another ongoing command, so mCurrentOperationBreadcrumb reflects the breadcrumb // argument in the only background command. @@ -503,7 +503,7 @@ void Instance::OnResult(Status commissioningError, CharSpan debugText, int32_t i commandHandle->AddResponse(mPath, response); if (commissioningError == NetworkCommissioningStatus::kSuccess) { - UpdateBreadcrumb(); + CommitSavedBreadcrumb(); } } @@ -569,7 +569,7 @@ void Instance::OnFinished(Status status, CharSpan debugText, ThreadScanResponseI } if (status == NetworkCommissioningStatus::kSuccess) { - UpdateBreadcrumb(); + CommitSavedBreadcrumb(); } networks->Release(); } @@ -632,7 +632,7 @@ void Instance::OnFinished(Status status, CharSpan debugText, WiFiScanResponseIte } if (status == NetworkCommissioningStatus::kSuccess) { - UpdateBreadcrumb(); + CommitSavedBreadcrumb(); } if (networks != nullptr) { diff --git a/src/app/clusters/network-commissioning/network-commissioning.h b/src/app/clusters/network-commissioning/network-commissioning.h index 690789aa9584b7..fddd41098147c4 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.h +++ b/src/app/clusters/network-commissioning/network-commissioning.h @@ -101,8 +101,13 @@ class Instance : public CommandHandlerInterface, uint8_t mLastNetworkIDLen = 0; Optional mCurrentOperationBreadcrumb; + + // Commits the breadcrumb value saved in mCurrentOperationBreadcrumb to the breadcrumb attribute in GeneralCommissioning + // cluster. Will set mCurrentOperationBreadcrumb to NullOptional. + void CommitSavedBreadcrumb(); + + // Sets the breadcrumb attribute in GeneralCommissioning cluster, no-op when breadcrumbValue is NullOptional. void UpdateBreadcrumb(const Optional & breadcrumbValue); - void UpdateBreadcrumb(); // Actual handlers of the commands void HandleScanNetworks(HandlerContext & ctx, const Commands::ScanNetworks::DecodableType & req);