diff --git a/examples/chip-tool/commands/pairing/OpenCommissioningWindowCommand.cpp b/examples/chip-tool/commands/pairing/OpenCommissioningWindowCommand.cpp index 2894ec79e8b16d..bc80e568b2bd7f 100644 --- a/examples/chip-tool/commands/pairing/OpenCommissioningWindowCommand.cpp +++ b/examples/chip-tool/commands/pairing/OpenCommissioningWindowCommand.cpp @@ -39,8 +39,9 @@ CHIP_ERROR OpenCommissioningWindowCommand::RunCommand() .SetTimeout(mCommissioningWindowTimeout) .SetIteration(mIteration) .SetDiscriminator(mDiscriminator) - .SetReadVIDPIDAttributes(true), - &mOnOpenCommissioningWindowCallback, ignored); + .SetReadVIDPIDAttributes(true) + .SetCallback(&mOnOpenCommissioningWindowCallback), + ignored); } ChipLogError(chipTool, "Unknown commissioning window option: %d", to_underlying(mCommissioningWindowOption)); diff --git a/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.cpp b/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.cpp index c60c742c564ecb..480cfbca35a20b 100644 --- a/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.cpp +++ b/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.cpp @@ -42,8 +42,8 @@ CHIP_ERROR OpenCommissioningWindowCommand::RunCommand() .SetIteration(mIteration) .SetDiscriminator(mDiscriminator) .SetVerifier(mVerifier.Value()) - .SetSalt(mSalt.Value()), - &mOnOpenCommissioningWindowVerifierCallback); + .SetSalt(mSalt.Value()) + .SetCallback(&mOnOpenCommissioningWindowVerifierCallback)); } else { @@ -54,8 +54,9 @@ CHIP_ERROR OpenCommissioningWindowCommand::RunCommand() .SetIteration(mIteration) .SetDiscriminator(mDiscriminator) .SetSalt(mSalt) - .SetReadVIDPIDAttributes(true), - &mOnOpenCommissioningWindowCallback, ignored); + .SetReadVIDPIDAttributes(true) + .SetCallback(&mOnOpenCommissioningWindowCallback), + ignored); } } diff --git a/src/controller/CommissioningWindowOpener.cpp b/src/controller/CommissioningWindowOpener.cpp index a2b32a277348b3..b0c219f4699c80 100644 --- a/src/controller/CommissioningWindowOpener.cpp +++ b/src/controller/CommissioningWindowOpener.cpp @@ -67,12 +67,12 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(NodeId deviceId, S .SetDiscriminator(discriminator) .SetSetupPIN(setupPIN) .SetSalt(salt) - .SetReadVIDPIDAttributes(readVIDPIDAttributes), - callback, payload); + .SetReadVIDPIDAttributes(readVIDPIDAttributes) + .SetCallback(callback), + payload); } CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(const CommissioningWindowPasscodeParams & params, - Callback::Callback * callback, SetupPayload & payload) { VerifyOrReturnError(mNextStep == Step::kAcceptCommissioningStart, CHIP_ERROR_INCORRECT_STATE); @@ -118,7 +118,7 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(const Commissionin PASESession::GeneratePASEVerifier(mVerifier, mPBKDFIterations, mPBKDFSalt, randomSetupPIN, mSetupPayload.setUpPINCode)); payload = mSetupPayload; - mCommissioningWindowCallback = callback; + mCommissioningWindowCallback = params.GetCallback(); mBasicCommissioningWindowCallback = nullptr; mCommissioningWindowVerifierCallback = nullptr; mNodeId = params.GetNodeId(); @@ -136,8 +136,7 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(const Commissionin return mController->GetConnectedDevice(mNodeId, &mDeviceConnected, &mDeviceConnectionFailure); } -CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(const CommissioningWindowVerifierParams & params, - Callback::Callback * callback) +CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(const CommissioningWindowVerifierParams & params) { VerifyOrReturnError(mNextStep == Step::kAcceptCommissioningStart, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(kSpake2p_Min_PBKDF_Iterations <= params.GetIteration() && @@ -150,7 +149,7 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(const Commissionin mPBKDFSalt = ByteSpan(mPBKDFSaltBuffer, params.GetSalt().size()); ReturnErrorOnFailure(mVerifier.Deserialize(params.GetVerifier())); - mCommissioningWindowVerifierCallback = callback; + mCommissioningWindowVerifierCallback = params.GetCallback(); mBasicCommissioningWindowCallback = nullptr; mCommissioningWindowCallback = nullptr; mNodeId = params.GetNodeId(); diff --git a/src/controller/CommissioningWindowOpener.h b/src/controller/CommissioningWindowOpener.h index 10bdae48703c43..2fb97d424fc06a 100644 --- a/src/controller/CommissioningWindowOpener.h +++ b/src/controller/CommissioningWindowOpener.h @@ -31,12 +31,6 @@ namespace chip { namespace Controller { -// Passing SetupPayload by value on purpose, in case a consumer decides to reuse -// this object from inside the callback. -typedef void (*OnOpenCommissioningWindow)(void * context, NodeId deviceId, CHIP_ERROR status, SetupPayload payload); -typedef void (*OnOpenCommissioningWindowWithVerifier)(void * context, NodeId deviceId, CHIP_ERROR status); -typedef void (*OnOpenBasicCommissioningWindow)(void * context, NodeId deviceId, CHIP_ERROR status); - /** * A helper class to open a commissioning window given some parameters. */ @@ -118,9 +112,6 @@ class CommissioningWindowOpener * * @param[in] params The parameters required to open an enhanced commissioning window * with the provided or generated passcode. - * @param[in] callback The function to be called on success or failure of opening the - * commissioning window. This will include the SetupPayload - * generated from provided parameters. * @param[out] payload The setup payload, not including the VID/PID bits, * even if those were asked for, that is generated * based on the passed-in information. The payload @@ -128,8 +119,7 @@ class CommissioningWindowOpener * out parameter, will include the VID/PID bits if * readVIDPIDAttributes is true. */ - CHIP_ERROR OpenCommissioningWindow(const CommissioningWindowPasscodeParams & params, - Callback::Callback * callback, SetupPayload & payload); + CHIP_ERROR OpenCommissioningWindow(const CommissioningWindowPasscodeParams & params, SetupPayload & payload); /** * @brief @@ -141,11 +131,8 @@ class CommissioningWindowOpener * * @param[in] params The parameters required to open an enhanced commissioning window * with the provided PAKE passcode verifier. - * @param[in] callback The function to be called on success or failure of opening the - * commissioning window. This will NOT include the SetupPayload. */ - CHIP_ERROR OpenCommissioningWindow(const CommissioningWindowVerifierParams & params, - Callback::Callback * callback); + CHIP_ERROR OpenCommissioningWindow(const CommissioningWindowVerifierParams & params); private: enum class Step : uint8_t diff --git a/src/controller/CommissioningWindowParams.h b/src/controller/CommissioningWindowParams.h index 2c375d96672661..a653560e6923d5 100644 --- a/src/controller/CommissioningWindowParams.h +++ b/src/controller/CommissioningWindowParams.h @@ -28,6 +28,12 @@ namespace chip { namespace Controller { +// Passing SetupPayload by value on purpose, in case a consumer decides to reuse +// this object from inside the callback. +typedef void (*OnOpenCommissioningWindow)(void * context, NodeId deviceId, CHIP_ERROR status, SetupPayload payload); +typedef void (*OnOpenCommissioningWindowWithVerifier)(void * context, NodeId deviceId, CHIP_ERROR status); +typedef void (*OnOpenBasicCommissioningWindow)(void * context, NodeId deviceId, CHIP_ERROR status); + template class CommissioningWindowCommonParams { @@ -121,10 +127,20 @@ class CommissioningWindowPasscodeParams : public CommissioningWindowCommonParams return *this; } + Callback::Callback * GetCallback() const { return mCallback; } + // The function to be called on success or failure of opening the commissioning window. + // This will include the SetupPayload generated from provided parameters. + CommissioningWindowPasscodeParams & SetCallback(Callback::Callback * callback) + { + mCallback = callback; + return *this; + } + private: - Optional mSetupPIN = NullOptional; - Optional mSalt = NullOptional; - bool mReadVIDPIDAttributes = false; + Optional mSetupPIN = NullOptional; + Optional mSalt = NullOptional; + bool mReadVIDPIDAttributes = false; + Callback::Callback * mCallback = nullptr; }; class CommissioningWindowVerifierParams : public CommissioningWindowCommonParams @@ -150,9 +166,19 @@ class CommissioningWindowVerifierParams : public CommissioningWindowCommonParams return *this; } + Callback::Callback * GetCallback() const { return mCallback; } + // The function to be called on success or failure of opening the + // commissioning window. This will NOT include the SetupPayload. + CommissioningWindowVerifierParams & SetCallback(Callback::Callback * callback) + { + mCallback = callback; + return *this; + } + private: ByteSpan mVerifier; ByteSpan mSalt; + Callback::Callback * mCallback = nullptr; }; } // namespace Controller diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp index 95df2249a8f4ac..556ae2c6943325 100644 --- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp +++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp @@ -714,12 +714,14 @@ PyChipError pychip_DeviceController_OpenCommissioningWindow(chip::Controller::De SetupPayload payload; auto opener = Platform::New(static_cast(devCtrl)); - PyChipError err = ToPyChipError(opener->OpenCommissioningWindow(Controller::CommissioningWindowPasscodeParams() - .SetNodeId(nodeid) - .SetTimeout(timeout) - .SetIteration(iteration) - .SetDiscriminator(discriminator), - pairingDelegate->GetOpenWindowCallback(opener), payload)); + PyChipError err = + ToPyChipError(opener->OpenCommissioningWindow(Controller::CommissioningWindowPasscodeParams() + .SetNodeId(nodeid) + .SetTimeout(timeout) + .SetIteration(iteration) + .SetDiscriminator(discriminator) + .SetCallback(pairingDelegate->GetOpenWindowCallback(opener)), + payload)); return err; } diff --git a/src/controller/tests/TestCommissioningWindowOpener.cpp b/src/controller/tests/TestCommissioningWindowOpener.cpp index 52f1f4194aed1e..dbaba8016eb53c 100644 --- a/src/controller/tests/TestCommissioningWindowOpener.cpp +++ b/src/controller/tests/TestCommissioningWindowOpener.cpp @@ -77,8 +77,8 @@ TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowVerifier_Success) .SetIteration(sTestSpake2p01_IterationCount) .SetDiscriminator(3840) .SetSalt(ByteSpan(sTestSpake2p01_Salt)) - .SetVerifier(ByteSpan(sTestSpake2p01_SerializedVerifier)), - &callback); + .SetVerifier(ByteSpan(sTestSpake2p01_SerializedVerifier)) + .SetCallback(&callback)); EXPECT_EQ(err, CHIP_NO_ERROR); } @@ -91,8 +91,8 @@ TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowVerifier_Failure_No .SetTimeout(300) .SetIteration(sTestSpake2p01_IterationCount) .SetDiscriminator(3840) - .SetVerifier(ByteSpan(sTestSpake2p01_SerializedVerifier)), - &callback); + .SetVerifier(ByteSpan(sTestSpake2p01_SerializedVerifier)) + .SetCallback(&callback)); EXPECT_EQ(err, CHIP_ERROR_INVALID_ARGUMENT); } @@ -105,8 +105,8 @@ TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowVerifier_Failure_No .SetTimeout(300) .SetIteration(sTestSpake2p01_IterationCount) .SetDiscriminator(3840) - .SetSalt(ByteSpan(sTestSpake2p01_Salt)), - &callback); + .SetSalt(ByteSpan(sTestSpake2p01_Salt)) + .SetCallback(&callback)); EXPECT_EQ(err, CHIP_ERROR_INVALID_ARGUMENT); } @@ -120,8 +120,8 @@ TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowVerifier_Failure_In .SetIteration(0) .SetDiscriminator(3840) .SetSalt(ByteSpan(sTestSpake2p01_Salt)) - .SetVerifier(ByteSpan(sTestSpake2p01_SerializedVerifier)), - &callback); + .SetVerifier(ByteSpan(sTestSpake2p01_SerializedVerifier)) + .SetCallback(&callback)); EXPECT_EQ(err, CHIP_ERROR_INVALID_ARGUMENT); } @@ -136,8 +136,9 @@ TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowPasscode_Success) .SetDiscriminator(3840) .SetSetupPIN(sTestSpake2p01_PinCode) .SetReadVIDPIDAttributes(true) - .SetSalt(ByteSpan(sTestSpake2p01_Salt)), - &callback, ignored); + .SetSalt(ByteSpan(sTestSpake2p01_Salt)) + .SetCallback(&callback), + ignored); EXPECT_EQ(err, CHIP_NO_ERROR); } @@ -150,8 +151,9 @@ TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowPasscode_Success_No .SetTimeout(300) .SetIteration(sTestSpake2p01_IterationCount) .SetDiscriminator(3840) - .SetSalt(ByteSpan(sTestSpake2p01_Salt)), - &callback, ignored); + .SetSalt(ByteSpan(sTestSpake2p01_Salt)) + .SetCallback(&callback), + ignored); EXPECT_EQ(err, CHIP_NO_ERROR); } @@ -164,8 +166,9 @@ TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowPasscode_Success_No .SetTimeout(300) .SetIteration(sTestSpake2p01_IterationCount) .SetDiscriminator(3840) - .SetSetupPIN(sTestSpake2p01_PinCode), - &callback, ignored); + .SetSetupPIN(sTestSpake2p01_PinCode) + .SetCallback(&callback), + ignored); EXPECT_EQ(err, CHIP_NO_ERROR); } @@ -173,9 +176,13 @@ TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowPasscode_Failure_In { SetupPayload ignored; Callback::Callback callback(OCWPasscodeCallback, this); - CHIP_ERROR err = opener.OpenCommissioningWindow( - Controller::CommissioningWindowPasscodeParams().SetNodeId(0x1234).SetTimeout(300).SetIteration(0).SetDiscriminator(3840), - &callback, ignored); + CHIP_ERROR err = opener.OpenCommissioningWindow(Controller::CommissioningWindowPasscodeParams() + .SetNodeId(0x1234) + .SetTimeout(300) + .SetIteration(0) + .SetDiscriminator(3840) + .SetCallback(&callback), + ignored); EXPECT_EQ(err, CHIP_ERROR_INVALID_ARGUMENT); }