diff --git a/src/controller/AutoCommissioner.cpp b/src/controller/AutoCommissioner.cpp index 62cf5ff16912da..e9cc301f45b904 100644 --- a/src/controller/AutoCommissioner.cpp +++ b/src/controller/AutoCommissioner.cpp @@ -203,6 +203,8 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio // skip scan step return CommissioningStage::kConfigRegulatory; case CommissioningStage::kScanNetworks: + return CommissioningStage::kNeedsNetworkCreds; + case CommissioningStage::kNeedsNetworkCreds: return CommissioningStage::kConfigRegulatory; case CommissioningStage::kConfigRegulatory: return CommissioningStage::kSendPAICertificateRequest; @@ -536,17 +538,6 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio } mParams.SetCompletionStatus(completionStatus); - if (mCommissioningPaused) - { - mPausedStage = nextStage; - - if (GetDeviceProxyForStep(nextStage) == nullptr) - { - ChipLogError(Controller, "Invalid device for commissioning"); - return CHIP_ERROR_INCORRECT_STATE; - } - return CHIP_NO_ERROR; - } return PerformStep(nextStage); } @@ -574,28 +565,6 @@ CHIP_ERROR AutoCommissioner::PerformStep(CommissioningStage nextStage) return CHIP_NO_ERROR; } -void AutoCommissioner::PauseCommissioning() -{ - mCommissioningPaused = true; -} - -CHIP_ERROR AutoCommissioner::ResumeCommissioning() -{ - VerifyOrReturnError(mCommissioningPaused, CHIP_ERROR_INCORRECT_STATE); - mCommissioningPaused = false; - - // if no new step was attempted - if (mPausedStage == CommissioningStage::kError) - { - return CHIP_NO_ERROR; - } - - CommissioningStage nextStage = mPausedStage; - mPausedStage = CommissioningStage::kError; - - return PerformStep(nextStage); -} - void AutoCommissioner::ReleaseDAC() { if (mDAC != nullptr) diff --git a/src/controller/AutoCommissioner.h b/src/controller/AutoCommissioner.h index 25932e213c69c8..4dbfa4332841bf 100644 --- a/src/controller/AutoCommissioner.h +++ b/src/controller/AutoCommissioner.h @@ -38,25 +38,6 @@ class AutoCommissioner : public CommissioningDelegate CHIP_ERROR CommissioningStepFinished(CHIP_ERROR err, CommissioningDelegate::CommissioningReport report) override; - /** - * @brief - * This function puts the AutoCommissioner in a paused state to prevent advancing to the next stage. - * It is expected that a DevicePairingDelegate may call this method when processing the - * OnCommissioningStatusUpdate, for example, in order to obtain network credentials from the user based - * upon the results of the NetworkScan. - * Use ResumeCommissioning to continue the commissioning process. - * - */ - void PauseCommissioning(); - - /** - * @brief - * An error return value means resume failed, for example: - * - AutoCommissioner was not in a paused state. - * - AutoCommissioner was unable to continue (no DeviceProxy) - */ - CHIP_ERROR ResumeCommissioning(); - protected: CommissioningStage GetNextCommissioningStage(CommissioningStage currentStage, CHIP_ERROR & lastErr); DeviceCommissioner * GetCommissioner() { return mCommissioner; } @@ -96,9 +77,6 @@ class AutoCommissioner : public CommissioningDelegate bool mNeedsNetworkSetup = false; ReadCommissioningInfo mDeviceCommissioningInfo; - CommissioningStage mPausedStage = CommissioningStage::kError; - bool mCommissioningPaused = false; - // TODO: Why were the nonces statically allocated, but the certs dynamically allocated? uint8_t * mDAC = nullptr; uint16_t mDACLen = 0; diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index d409d727f405dd..afdc4609fd8581 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -1833,13 +1833,15 @@ void DeviceCommissioner::OnScanNetworksFailure(void * context, CHIP_ERROR error) ChipLogProgress(Controller, "Received ScanNetworks failure response %" CHIP_ERROR_FORMAT, error.Format()); DeviceCommissioner * commissioner = static_cast(context); + + // advance to the kNeedsNetworkCreds waiting step + // clear error so that we don't abort the commissioning when ScanNetworks fails + commissioner->CommissioningStageComplete(CHIP_NO_ERROR); + if (commissioner->GetPairingDelegate() != nullptr) { commissioner->GetPairingDelegate()->OnScanNetworksFailure(error); } - // need to advance to next step - // clear error so that we don't abort the commissioning when ScanNetworks fails - commissioner->CommissioningStageComplete(CHIP_NO_ERROR); } void DeviceCommissioner::OnScanNetworksResponse(void * context, @@ -1853,12 +1855,23 @@ void DeviceCommissioner::OnScanNetworksResponse(void * context, : "none provided")); DeviceCommissioner * commissioner = static_cast(context); + // advance to the kNeedsNetworkCreds waiting step + commissioner->CommissioningStageComplete(CHIP_NO_ERROR); + if (commissioner->GetPairingDelegate() != nullptr) { commissioner->GetPairingDelegate()->OnScanNetworksSuccess(data); } +} + +CHIP_ERROR DeviceCommissioner::NetworkCredentialsReady() +{ + ReturnErrorCodeIf(mCommissioningStage != CommissioningStage::kNeedsNetworkCreds, CHIP_ERROR_INCORRECT_STATE); + // need to advance to next step - commissioner->CommissioningStageComplete(CHIP_NO_ERROR); + CommissioningStageComplete(CHIP_NO_ERROR); + + return CHIP_NO_ERROR; } void DeviceCommissioner::OnNetworkConfigResponse(void * context, @@ -2000,6 +2013,11 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio SendCommand(proxy, request, OnScanNetworksResponse, OnScanNetworksFailure, endpoint, timeout); break; } + case CommissioningStage::kNeedsNetworkCreds: { + // nothing to do, the OnScanNetworksSuccess and OnScanNetworksFailure callbacks provide indication to the + // DevicePairingDelegate that network credentials are needed. + break; + } case CommissioningStage::kConfigRegulatory: { // To set during config phase: // UTC time diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index ad357842685c94..78581dca42c657 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -547,6 +547,20 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, CommissioningStageComplete(CHIP_ERROR err, CommissioningDelegate::CommissioningReport report = CommissioningDelegate::CommissioningReport()); + /** + * @brief + * This function is called by the DevicePairingDelegate to indicate that network credentials have been set + * on the CommissioningParameters of the CommissioningDelegate using CommissioningDelegate.SetCommissioningParameters(). + * As a result, commissioning can advance to the next stage. + * + * The DevicePairingDelegate may call this method from the OnScanNetworksSuccess and OnScanNetworksFailure callbacks, + * or it may call this method after obtaining network credentials using asyncronous methods (prompting user, cloud API call, + * etc). + * + * @return CHIP_ERROR The return status. Returns CHIP_ERROR_INCORRECT_STATE if not in the correct state (kNeedsNetworkCreds). + */ + CHIP_ERROR NetworkCredentialsReady(); + #if CONFIG_NETWORK_LAYER_BLE #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE /** diff --git a/src/controller/CommissioningDelegate.h b/src/controller/CommissioningDelegate.h index 595978b3049646..cf1743c2d21bae 100644 --- a/src/controller/CommissioningDelegate.h +++ b/src/controller/CommissioningDelegate.h @@ -55,6 +55,7 @@ enum CommissioningStage : uint8_t // ScanNetworks can happen anytime after kArmFailsafe. // However, the circ tests fail if it is earlier in the list kScanNetworks, + kNeedsNetworkCreds, }; const char * StageToString(CommissioningStage stage); diff --git a/src/controller/DevicePairingDelegate.h b/src/controller/DevicePairingDelegate.h index 186e3dc94288d7..016fe344d8a412 100644 --- a/src/controller/DevicePairingDelegate.h +++ b/src/controller/DevicePairingDelegate.h @@ -86,7 +86,13 @@ class DLL_EXPORT DevicePairingDelegate /** * @brief - * Called with the NetworkScanResponse returned from the target + * Called with the NetworkScanResponse returned from the target. + * + * The DeviceCommissioner will be waiting in the kNeedsNetworkCreds step and not advancing the commissioning process. + * + * The implementation should set the network credentials on the CommissioningParameters of the CommissioningDelegate + * using CommissioningDelegate.SetCommissioningParameters(), and then call DeviceCommissioner.NetworkCredentialsReady() + * in order to resume the commissioning process. */ virtual void OnScanNetworksSuccess(const app::Clusters::NetworkCommissioning::Commands::ScanNetworksResponse::DecodableType & dataResponse) @@ -95,6 +101,12 @@ class DLL_EXPORT DevicePairingDelegate /** * @brief * Called when the NetworkScan request fails. + * + * The DeviceCommissioner will be waiting in the kNeedsNetworkCreds step and not advancing the commissioning process. + * + * The implementation should set the network credentials on the CommissioningParameters of the CommissioningDelegate + * using CommissioningDelegate.SetCommissioningParameters(), and then call DeviceCommissioner.NetworkCredentialsReady() + * in order to resume the commissioning process. */ virtual void OnScanNetworksFailure(CHIP_ERROR error) {} }; diff --git a/src/controller/java/CHIPDeviceController-JNI.cpp b/src/controller/java/CHIPDeviceController-JNI.cpp index 1cd4972db04a37..2020eb27e45d8a 100644 --- a/src/controller/java/CHIPDeviceController-JNI.cpp +++ b/src/controller/java/CHIPDeviceController-JNI.cpp @@ -513,26 +513,6 @@ JNI_METHOD(void, establishPaseConnectionByAddress) } } -JNI_METHOD(void, pauseCommissioning) -(JNIEnv * env, jobject self, jlong handle) -{ - ChipLogProgress(Controller, "pauseCommissioning() called"); - chip::DeviceLayer::StackLock lock; - AndroidDeviceControllerWrapper * wrapper = AndroidDeviceControllerWrapper::FromJNIHandle(handle); - - wrapper->GetAutoCommissioner()->PauseCommissioning(); -} - -JNI_METHOD(void, resumeCommissioning) -(JNIEnv * env, jobject self, jlong handle) -{ - ChipLogProgress(Controller, "resumeCommissioning() called"); - chip::DeviceLayer::StackLock lock; - AndroidDeviceControllerWrapper * wrapper = AndroidDeviceControllerWrapper::FromJNIHandle(handle); - - wrapper->GetAutoCommissioner()->ResumeCommissioning(); -} - JNI_METHOD(void, setUseJavaCallbackForNOCRequest) (JNIEnv * env, jobject self, jlong handle, jboolean useCallback) { diff --git a/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java b/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java index dbc0c54d53ab18..541f83e40aba99 100644 --- a/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java +++ b/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java @@ -188,14 +188,6 @@ public void commissionDevice( commissionDevice(deviceControllerPtr, deviceId, csrNonce, networkCredentials); } - public void pauseCommissioning() { - pauseCommissioning(deviceControllerPtr); - } - - public void resumeCommissioning() { - resumeCommissioning(deviceControllerPtr); - } - /** * When a NOCChainIssuer is set for this controller, then onNOCChainGenerationNeeded will be * called when the NOC CSR needs to be signed. This allows for custom credentials issuer @@ -670,10 +662,6 @@ private native boolean openPairingWindowWithPINCallback( private native byte[] getAttestationChallenge(long deviceControllerPtr, long devicePtr); - private native void pauseCommissioning(long deviceControllerPtr); - - private native void resumeCommissioning(long deviceControllerPtr); - private native void setUseJavaCallbackForNOCRequest( long deviceControllerPtr, boolean useCallback);