Skip to content

Commit

Permalink
Move callback to params
Browse files Browse the repository at this point in the history
Co-authored-by: Andrei Litvin <[email protected]>
  • Loading branch information
samadDotDev and andy31415 committed Jul 12, 2024
1 parent b154a1b commit 8862cac
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -54,8 +54,9 @@ CHIP_ERROR OpenCommissioningWindowCommand::RunCommand()
.SetIteration(mIteration)
.SetDiscriminator(mDiscriminator)
.SetSalt(mSalt)
.SetReadVIDPIDAttributes(true),
&mOnOpenCommissioningWindowCallback, ignored);
.SetReadVIDPIDAttributes(true)
.SetCallback(&mOnOpenCommissioningWindowCallback),
ignored);
}
}

Expand Down
13 changes: 6 additions & 7 deletions src/controller/CommissioningWindowOpener.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<OnOpenCommissioningWindow> * callback,
SetupPayload & payload)
{
VerifyOrReturnError(mNextStep == Step::kAcceptCommissioningStart, CHIP_ERROR_INCORRECT_STATE);
Expand Down Expand Up @@ -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();
Expand All @@ -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<OnOpenCommissioningWindowWithVerifier> * callback)
CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(const CommissioningWindowVerifierParams & params)
{
VerifyOrReturnError(mNextStep == Step::kAcceptCommissioningStart, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(kSpake2p_Min_PBKDF_Iterations <= params.GetIteration() &&
Expand All @@ -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();
Expand Down
17 changes: 2 additions & 15 deletions src/controller/CommissioningWindowOpener.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -118,18 +112,14 @@ 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
* provided to the callback function, unlike this
* out parameter, will include the VID/PID bits if
* readVIDPIDAttributes is true.
*/
CHIP_ERROR OpenCommissioningWindow(const CommissioningWindowPasscodeParams & params,
Callback::Callback<OnOpenCommissioningWindow> * callback, SetupPayload & payload);
CHIP_ERROR OpenCommissioningWindow(const CommissioningWindowPasscodeParams & params, SetupPayload & payload);

/**
* @brief
Expand All @@ -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<OnOpenCommissioningWindowWithVerifier> * callback);
CHIP_ERROR OpenCommissioningWindow(const CommissioningWindowVerifierParams & params);

private:
enum class Step : uint8_t
Expand Down
32 changes: 29 additions & 3 deletions src/controller/CommissioningWindowParams.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename Derived>
class CommissioningWindowCommonParams
{
Expand Down Expand Up @@ -121,10 +127,20 @@ class CommissioningWindowPasscodeParams : public CommissioningWindowCommonParams
return *this;
}

Callback::Callback<OnOpenCommissioningWindow> * 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<OnOpenCommissioningWindow> * callback)
{
mCallback = callback;
return *this;
}

private:
Optional<uint32_t> mSetupPIN = NullOptional;
Optional<ByteSpan> mSalt = NullOptional;
bool mReadVIDPIDAttributes = false;
Optional<uint32_t> mSetupPIN = NullOptional;
Optional<ByteSpan> mSalt = NullOptional;
bool mReadVIDPIDAttributes = false;
Callback::Callback<OnOpenCommissioningWindow> * mCallback = nullptr;
};

class CommissioningWindowVerifierParams : public CommissioningWindowCommonParams<CommissioningWindowVerifierParams>
Expand All @@ -150,9 +166,19 @@ class CommissioningWindowVerifierParams : public CommissioningWindowCommonParams
return *this;
}

Callback::Callback<OnOpenCommissioningWindowWithVerifier> * 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<OnOpenCommissioningWindowWithVerifier> * callback)
{
mCallback = callback;
return *this;
}

private:
ByteSpan mVerifier;
ByteSpan mSalt;
Callback::Callback<OnOpenCommissioningWindowWithVerifier> * mCallback = nullptr;
};

} // namespace Controller
Expand Down
14 changes: 8 additions & 6 deletions src/controller/python/ChipDeviceController-ScriptBinding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -714,12 +714,14 @@ PyChipError pychip_DeviceController_OpenCommissioningWindow(chip::Controller::De
SetupPayload payload;
auto opener =
Platform::New<Controller::CommissioningWindowOpener>(static_cast<chip::Controller::DeviceController *>(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;
}

Expand Down
41 changes: 24 additions & 17 deletions src/controller/tests/TestCommissioningWindowOpener.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -164,18 +166,23 @@ 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);
}

TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowPasscode_Failure_InvalidIteration)
{
SetupPayload ignored;
Callback::Callback<Controller::OnOpenCommissioningWindow> 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);
}

Expand Down

0 comments on commit 8862cac

Please sign in to comment.