From 11394062d11d32fa6583d3c862de99bbe985bf52 Mon Sep 17 00:00:00 2001 From: cdj <45139296+DejinChen@users.noreply.github.com> Date: Thu, 5 Sep 2024 12:54:00 +0800 Subject: [PATCH] Auto-commissioner: remove primary network config if failed in commissioning SecondaryNetworkInterface device (#35255) --- src/controller/AutoCommissioner.cpp | 15 ++++++-- src/controller/CHIPDeviceController.cpp | 46 ++++++++++++++++-------- src/controller/CHIPDeviceController.h | 2 -- src/controller/CommissioningDelegate.cpp | 7 ++-- src/controller/CommissioningDelegate.h | 7 ++-- 5 files changed, 53 insertions(+), 24 deletions(-) diff --git a/src/controller/AutoCommissioner.cpp b/src/controller/AutoCommissioner.cpp index 0e83d7125cf175..64f132c3124cda 100644 --- a/src/controller/AutoCommissioner.cpp +++ b/src/controller/AutoCommissioner.cpp @@ -502,8 +502,16 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio case CommissioningStage::kEvictPreviousCaseSessions: return CommissioningStage::kFindOperationalForStayActive; case CommissioningStage::kPrimaryOperationalNetworkFailed: - return CommissioningStage::kDisablePrimaryNetworkInterface; - case CommissioningStage::kDisablePrimaryNetworkInterface: + if (mDeviceCommissioningInfo.network.wifi.endpoint == kRootEndpointId) + { + return CommissioningStage::kRemoveWiFiNetworkConfig; + } + else + { + return CommissioningStage::kRemoveThreadNetworkConfig; + } + case CommissioningStage::kRemoveWiFiNetworkConfig: + case CommissioningStage::kRemoveThreadNetworkConfig: return GetNextCommissioningStageNetworkSetup(currentStage, lastErr); case CommissioningStage::kFindOperationalForStayActive: return CommissioningStage::kICDSendStayActive; @@ -567,7 +575,8 @@ EndpointId AutoCommissioner::GetEndpoint(const CommissioningStage & stage) const case CommissioningStage::kThreadNetworkSetup: case CommissioningStage::kThreadNetworkEnable: return mDeviceCommissioningInfo.network.thread.endpoint; - case CommissioningStage::kDisablePrimaryNetworkInterface: + case CommissioningStage::kRemoveWiFiNetworkConfig: + case CommissioningStage::kRemoveThreadNetworkConfig: return kRootEndpointId; default: return kRootEndpointId; diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 955e36bfb0d8a2..7c43d50c082389 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -1878,12 +1878,6 @@ void DeviceCommissioner::OnBasicSuccess(void * context, const chip::app::DataMod commissioner->CommissioningStageComplete(CHIP_NO_ERROR); } -void DeviceCommissioner::OnInterfaceEnableWriteSuccessResponse(void * context) -{ - DeviceCommissioner * commissioner = static_cast(context); - commissioner->CommissioningStageComplete(CHIP_NO_ERROR); -} - void DeviceCommissioner::OnBasicFailure(void * context, CHIP_ERROR error) { ChipLogProgress(Controller, "Received failure response %s\n", chip::ErrorStr(error)); @@ -3536,19 +3530,43 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio } break; case CommissioningStage::kPrimaryOperationalNetworkFailed: { - // nothing to do. This stage indicates that the primary operational network failed and the network interface should be - // disabled later. + // nothing to do. This stage indicates that the primary operational network failed and the network config should be + // removed later. break; } - case CommissioningStage::kDisablePrimaryNetworkInterface: { - NetworkCommissioning::Attributes::InterfaceEnabled::TypeInfo::Type request = false; - CHIP_ERROR err = SendCommissioningWriteRequest(proxy, endpoint, NetworkCommissioning::Id, - NetworkCommissioning::Attributes::InterfaceEnabled::Id, request, - OnInterfaceEnableWriteSuccessResponse, OnBasicFailure); + case CommissioningStage::kRemoveWiFiNetworkConfig: { + NetworkCommissioning::Commands::RemoveNetwork::Type request; + request.networkID = params.GetWiFiCredentials().Value().ssid; + request.breadcrumb.Emplace(breadcrumb); + CHIP_ERROR err = SendCommissioningCommand(proxy, request, OnNetworkConfigResponse, OnBasicFailure, endpoint, timeout); + if (err != CHIP_NO_ERROR) + { + // We won't get any async callbacks here, so just complete our stage. + ChipLogError(Controller, "Failed to send RemoveNetwork command: %" CHIP_ERROR_FORMAT, err.Format()); + CommissioningStageComplete(err); + return; + } + break; + } + case CommissioningStage::kRemoveThreadNetworkConfig: { + ByteSpan extendedPanId; + chip::Thread::OperationalDataset operationalDataset; + if (!params.GetThreadOperationalDataset().HasValue() || + operationalDataset.Init(params.GetThreadOperationalDataset().Value()) != CHIP_NO_ERROR || + operationalDataset.GetExtendedPanIdAsByteSpan(extendedPanId) != CHIP_NO_ERROR) + { + ChipLogError(Controller, "Unable to get extended pan ID for thread operational dataset\n"); + CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT); + return; + } + NetworkCommissioning::Commands::RemoveNetwork::Type request; + request.networkID = extendedPanId; + request.breadcrumb.Emplace(breadcrumb); + CHIP_ERROR err = SendCommissioningCommand(proxy, request, OnNetworkConfigResponse, OnBasicFailure, endpoint, timeout); if (err != CHIP_NO_ERROR) { // We won't get any async callbacks here, so just complete our stage. - ChipLogError(Controller, "Failed to send InterfaceEnabled write request: %" CHIP_ERROR_FORMAT, err.Format()); + ChipLogError(Controller, "Failed to send RemoveNetwork command: %" CHIP_ERROR_FORMAT, err.Format()); CommissioningStageComplete(err); return; } diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 4b876156199735..2ec020340ee98d 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -980,8 +980,6 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, OnICDManagementStayActiveResponse(void * context, const app::Clusters::IcdManagement::Commands::StayActiveResponse::DecodableType & data); - static void OnInterfaceEnableWriteSuccessResponse(void * context); - /** * @brief * This function processes the CSR sent by the device. diff --git a/src/controller/CommissioningDelegate.cpp b/src/controller/CommissioningDelegate.cpp index 85ea5e86c5e3a6..d6acac6bc00bd4 100644 --- a/src/controller/CommissioningDelegate.cpp +++ b/src/controller/CommissioningDelegate.cpp @@ -139,8 +139,11 @@ const char * StageToString(CommissioningStage stage) case kPrimaryOperationalNetworkFailed: return "PrimaryOperationalNetworkFailed"; - case kDisablePrimaryNetworkInterface: - return "DisablePrimaryNetworkInterface"; + case kRemoveWiFiNetworkConfig: + return "RemoveWiFiNetworkConfig"; + + case kRemoveThreadNetworkConfig: + return "RemoveThreadNetworkConfig"; default: return "???"; diff --git a/src/controller/CommissioningDelegate.h b/src/controller/CommissioningDelegate.h index 7a96939cf49434..f0f98961adf3c3 100644 --- a/src/controller/CommissioningDelegate.h +++ b/src/controller/CommissioningDelegate.h @@ -79,9 +79,10 @@ enum CommissioningStage : uint8_t /// Call CHIPDeviceController::NetworkCredentialsReady() when CommissioningParameters is populated with /// network credentials to use in kWiFiNetworkSetup or kThreadNetworkSetup steps. kNeedsNetworkCreds, - kPrimaryOperationalNetworkFailed, ///< Indicate that the primary operational network (on root endpoint) failed, should disable - ///< the primary network interface later. - kDisablePrimaryNetworkInterface, ///< Send InterfaceEnabled write request to the device to disable network interface. + kPrimaryOperationalNetworkFailed, ///< Indicate that the primary operational network (on root endpoint) failed, should remove + ///< the primary network config later. + kRemoveWiFiNetworkConfig, ///< Remove Wi-Fi network config. + kRemoveThreadNetworkConfig ///< Remove Thread network config. }; enum class ICDRegistrationStrategy : uint8_t