Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@yufengwangca
yufengwangca committed Oct 3, 2024
1 parent 4ff623b commit 78aac89
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 69 deletions.
58 changes: 38 additions & 20 deletions examples/fabric-admin/device_manager/PairingManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ CHIP_ERROR PairingManager::Init(Controller::DeviceCommissioner * commissioner, C
VerifyOrReturnError(commissioner != nullptr, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(credIssuerCmds != nullptr, CHIP_ERROR_INCORRECT_STATE);

mCommissioner = commissioner;
mCommissioner = commissioner;
mCredIssuerCmds = credIssuerCmds;

return CHIP_NO_ERROR;
Expand Down Expand Up @@ -253,7 +253,6 @@ void PairingManager::OnStatusUpdate(DevicePairingDelegate::Status status)
switch (status)
{
case DevicePairingDelegate::Status::SecurePairingSuccess:
ChipLogProgress(NotSpecified, "Secure Pairing Success");
ChipLogProgress(NotSpecified, "CASE establishment successful");
break;
case DevicePairingDelegate::Status::SecurePairingFailed:
Expand All @@ -266,14 +265,12 @@ void PairingManager::OnPairingComplete(CHIP_ERROR err)
{
if (err == CHIP_NO_ERROR)
{
ChipLogProgress(NotSpecified, "Pairing Success");
ChipLogProgress(NotSpecified, "PASE establishment successful");
}
else
{
ChipLogProgress(NotSpecified, "Pairing Failure: %s", ErrorStr(err));
}

}

void PairingManager::OnPairingDeleted(CHIP_ERROR err)
Expand All @@ -293,8 +290,9 @@ void PairingManager::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err)
if (err == CHIP_NO_ERROR)
{
// print to console
fprintf(stderr, "New device with Node ID: 0x%lx has been successfully added.\n", nodeId);
// CurrentCommissioner() has a lifetime that is the entire life of the application itself
fprintf(stderr, "New device with Node ID: " ChipLogFormatX64 "has been successfully added.\n", ChipLogValueX64(nodeId));

// mCommissioner has a lifetime that is the entire life of the application itself
// so it is safe to provide to StartDeviceSynchronization.
DeviceSynchronizer::Instance().StartDeviceSynchronization(mCommissioner, nodeId, mDeviceIsICD);
}
Expand Down Expand Up @@ -418,6 +416,7 @@ void PairingManager::OnDiscoveredDevice(const Dnssd::CommissionNodeData & nodeDa
err = Pair(mNodeId, peerAddress);
if (CHIP_NO_ERROR != err)
{
ChipLogProgress(NotSpecified, "Failed to pair device: " ChipLogFormatX64 " %s", ChipLogValueX64(mNodeId), ErrorStr(err));
}
}

Expand Down Expand Up @@ -473,17 +472,19 @@ void PairingManager::OnDeviceAttestationCompleted(Controller::DeviceCommissioner
}

// NOTE: This will log errors even if the attestion was successful.
auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult);
CHIP_ERROR err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult);
if (CHIP_NO_ERROR != err)
{
ChipLogError(NotSpecified, "Failed to continue commissioning after device attestation, error: %s", ErrorStr(err));
}
return;
}

// Don't bypass attestation, continue with error.
auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult);
CHIP_ERROR err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult);
if (CHIP_NO_ERROR != err)
{
ChipLogError(NotSpecified, "Failed to continue commissioning after device attestation, error: %s", ErrorStr(err));
}
}

Expand Down Expand Up @@ -548,11 +549,13 @@ void PairingManager::OnCurrentFabricRemove(void * context, NodeId nodeId, CHIP_E
if (err == CHIP_NO_ERROR)
{
// print to console
fprintf(stderr, "Device with Node ID: 0x%lx has been successfully removed.\n", nodeId);
fprintf(stderr, "Device with Node ID: " ChipLogFormatX64 "has been successfully removed.\n", ChipLogValueX64(nodeId));

#if defined(PW_RPC_ENABLED)
app::InteractionModelEngine::GetInstance()->ShutdownSubscriptions(self->CurrentCommissioner().GetFabricIndex(), nodeId);
RemoveSynchronizedDevice(nodeId);
FabricIndex fabricIndex = self->CurrentCommissioner().GetFabricIndex();
app::InteractionModelEngine::GetInstance()->ShutdownSubscriptions(fabricIndex, nodeId);
ScopedNodeId scopedNodeId(nodeId, fabricIndex);
RemoveSynchronizedDevice(scopedNodeId);
#endif
}
else
Expand All @@ -571,16 +574,17 @@ void PairingManager::InitPairingCommand()

CHIP_ERROR PairingManager::PairDeviceWithCode(NodeId nodeId, const char * payload)
{
if (payload == nullptr || strlen(payload) > kMaxManualCodeLength)
if (payload == nullptr || strlen(payload) > kMaxManualCodeLength + 1)
{
ChipLogError(NotSpecified, "PairDeviceWithCode failed: Invalid pairing payload");
return CHIP_ERROR_INVALID_STRING_LENGTH;
}

auto params = Platform::MakeUnique<PairDeviceWithCodeParams>();
params->nodeId = nodeId;
auto params = Platform::MakeUnique<PairDeviceWithCodeParams>();
VerifyOrReturnError(params != nullptr, CHIP_ERROR_NO_MEMORY);

Platform::CopyString(params->payloadBuffer, kMaxManualCodeLength, payload);
params->nodeId = nodeId;
Platform::CopyString(params->payloadBuffer, sizeof(params->payloadBuffer), payload);

// Schedule work on the Matter thread
return DeviceLayer::PlatformMgr().ScheduleWork(OnPairDeviceWithCode, reinterpret_cast<intptr_t>(params.release()));
Expand Down Expand Up @@ -615,12 +619,14 @@ CHIP_ERROR PairingManager::PairDevice(chip::NodeId nodeId, uint32_t setupPINCode
return CHIP_ERROR_INVALID_STRING_LENGTH;
}

auto params = Platform::MakeUnique<PairDeviceParams>();
auto params = Platform::MakeUnique<PairDeviceParams>();
VerifyOrReturnError(params != nullptr, CHIP_ERROR_NO_MEMORY);

params->nodeId = nodeId;
params->setupPINCode = setupPINCode;
params->deviceRemotePort = deviceRemotePort;

Platform::CopyString(params->ipAddrBuffer, Inet::IPAddress::kMaxStringLength, deviceRemoteIp);
Platform::CopyString(params->ipAddrBuffer, sizeof(params->ipAddrBuffer), deviceRemoteIp);

// Schedule work on the Matter thread
return DeviceLayer::PlatformMgr().ScheduleWork(OnPairDevice, reinterpret_cast<intptr_t>(params.release()));
Expand Down Expand Up @@ -652,19 +658,31 @@ void PairingManager::OnPairDevice(intptr_t context)

CHIP_ERROR PairingManager::UnpairDevice(NodeId nodeId)
{
auto params = Platform::MakeUnique<UnpairDeviceParams>();
VerifyOrReturnError(params != nullptr, CHIP_ERROR_NO_MEMORY);

params->nodeId = nodeId;

// Schedule work on the Matter thread
return DeviceLayer::PlatformMgr().ScheduleWork(OnUnpairDevice, static_cast<intptr_t>(nodeId));
return DeviceLayer::PlatformMgr().ScheduleWork(OnUnpairDevice, reinterpret_cast<intptr_t>(params.release()));
}

void PairingManager::OnUnpairDevice(intptr_t context)
{
NodeId nodeId = static_cast<NodeId>(context);
Platform::UniquePtr<PairDeviceParams> params(reinterpret_cast<PairDeviceParams *>(context));
PairingManager & self = PairingManager::Instance();

self.InitPairingCommand();

self.mCurrentFabricRemover = Platform::MakeUnique<Controller::CurrentFabricRemover>(self.mCommissioner);
CHIP_ERROR err = self.mCurrentFabricRemover->RemoveCurrentFabric(nodeId, &self.mCurrentFabricRemoveCallback);

if (!self.mCurrentFabricRemover)
{
ChipLogError(NotSpecified, "Failed to unpair device, mCurrentFabricRemover is null");
return;
}

CHIP_ERROR err = self.mCurrentFabricRemover->RemoveCurrentFabric(params->nodeId, &self.mCurrentFabricRemoveCallback);
if (err != CHIP_NO_ERROR)
{
ChipLogError(NotSpecified, "Failed to unpair device, error: %s", ErrorStr(err));
Expand Down
100 changes: 51 additions & 49 deletions examples/fabric-admin/device_manager/PairingManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#include <crypto/CHIPCryptoPAL.h>

// Constants
constexpr uint16_t kMaxManualCodeLength = 21;
constexpr uint16_t kMaxManualCodeLength = 22;

class CommissioningWindowDelegate
{
Expand Down Expand Up @@ -140,14 +140,6 @@ class PairingManager : public chip::Controller::DevicePairingDelegate,
CHIP_ERROR UnpairDevice(chip::NodeId nodeId);

private:
PairingManager();
PairingManager(const PairingManager &) = delete;
PairingManager & operator=(const PairingManager &) = delete;

chip::Controller::DeviceCommissioner * mCommissioner = nullptr;
CredentialIssuerCommands * mCredIssuerCmds;

/////////// Open Commissioning Window Command Interface /////////
struct CommissioningWindowParams
{
chip::NodeId nodeId;
Expand All @@ -162,21 +154,34 @@ class PairingManager : public chip::Controller::DevicePairingDelegate,
chip::ByteSpan salt;
};

CommissioningWindowDelegate * mCommissioningWindowDelegate = nullptr;
struct PairDeviceWithCodeParams
{
chip::NodeId nodeId;
char payloadBuffer[kMaxManualCodeLength + 1];
};

/**
* Holds the unique_ptr to the current CommissioningWindowOpener.
* Only one commissioning window opener can be active at a time.
* The pointer is reset when the commissioning window is closed or when an error occurs.
*/
chip::Platform::UniquePtr<chip::Controller::CommissioningWindowOpener> mWindowOpener;
chip::Callback::Callback<chip::Controller::OnOpenCommissioningWindow> mOnOpenCommissioningWindowCallback;
chip::Callback::Callback<chip::Controller::OnOpenCommissioningWindowWithVerifier> mOnOpenCommissioningWindowVerifierCallback;
struct PairDeviceParams
{
chip::NodeId nodeId;
uint32_t setupPINCode;
uint16_t deviceRemotePort;
char ipAddrBuffer[chip::Inet::IPAddress::kMaxStringLength];
};

static void OnOpenCommissioningWindow(intptr_t context);
static void OnOpenCommissioningWindowResponse(void * context, chip::NodeId deviceId, CHIP_ERROR status,
chip::SetupPayload payload);
static void OnOpenCommissioningWindowVerifierResponse(void * context, chip::NodeId deviceId, CHIP_ERROR status);
struct UnpairDeviceParams
{
chip::NodeId nodeId;
};

// Constructors
PairingManager();
PairingManager(const PairingManager &) = delete;
PairingManager & operator=(const PairingManager &) = delete;

// Private member functions (static and non-static)
chip::Controller::CommissioningParameters GetCommissioningParameters();
void InitPairingCommand();
CHIP_ERROR Pair(chip::NodeId remoteId, chip::Transport::PeerAddress address);

/////////// DevicePairingDelegate Interface /////////
void OnStatusUpdate(chip::Controller::DevicePairingDelegate::Status status) override;
Expand All @@ -197,31 +202,28 @@ class PairingManager : public chip::Controller::DevicePairingDelegate,
const chip::Credentials::DeviceAttestationVerifier::AttestationDeviceInfo & info,
chip::Credentials::AttestationVerificationResult attestationResult) override;

/////////// Pairing Command Interface /////////
struct PairDeviceWithCodeParams
{
chip::NodeId nodeId;
char payloadBuffer[kMaxManualCodeLength + 1];
};
static void OnOpenCommissioningWindow(intptr_t context);
static void OnOpenCommissioningWindowResponse(void * context, chip::NodeId deviceId, CHIP_ERROR status,
chip::SetupPayload payload);
static void OnOpenCommissioningWindowVerifierResponse(void * context, chip::NodeId deviceId, CHIP_ERROR status);
static void OnCurrentFabricRemove(void * context, chip::NodeId remoteNodeId, CHIP_ERROR status);
static void OnPairDeviceWithCode(intptr_t context);
static void OnPairDevice(intptr_t context);
static void OnUnpairDevice(intptr_t context);

struct PairDeviceParams
{
chip::NodeId nodeId;
uint32_t setupPINCode;
uint16_t deviceRemotePort;
char ipAddrBuffer[chip::Inet::IPAddress::kMaxStringLength];
};
// Private data members
chip::Controller::DeviceCommissioner * mCommissioner = nullptr;
CredentialIssuerCommands * mCredIssuerCmds = nullptr;

CommissioningDelegate * mCommissioningDelegate = nullptr;
PairingDelegate * mPairingDelegate = nullptr;
CommissioningWindowDelegate * mCommissioningWindowDelegate = nullptr;
CommissioningDelegate * mCommissioningDelegate = nullptr;
PairingDelegate * mPairingDelegate = nullptr;

chip::NodeId mNodeId = chip::kUndefinedNodeId;
uint16_t mDiscriminator = 0;
uint32_t mSetupPINCode = 0;
const char * mOnboardingPayload = nullptr;

// For ICD
bool mDeviceIsICD = false;
bool mDeviceIsICD = false;
uint8_t mRandomGeneratedICDSymmetricKey[chip::Crypto::kAES_CCM128_Key_Length];

chip::Optional<bool> mICDRegistration;
Expand All @@ -231,16 +233,16 @@ class PairingManager : public chip::Controller::DevicePairingDelegate,
chip::Optional<uint64_t> mICDMonitoredSubject;
chip::Optional<uint32_t> mICDStayActiveDurationMsec;

/**
* Holds the unique_ptr to the current CommissioningWindowOpener.
* Only one commissioning window opener can be active at a time.
* The pointer is reset when the commissioning window is closed or when an error occurs.
*/
chip::Platform::UniquePtr<chip::Controller::CommissioningWindowOpener> mWindowOpener;
chip::Callback::Callback<chip::Controller::OnOpenCommissioningWindow> mOnOpenCommissioningWindowCallback;
chip::Callback::Callback<chip::Controller::OnOpenCommissioningWindowWithVerifier> mOnOpenCommissioningWindowVerifierCallback;

// For Unpair
chip::Platform::UniquePtr<chip::Controller::CurrentFabricRemover> mCurrentFabricRemover;
chip::Callback::Callback<chip::Controller::OnCurrentFabricRemove> mCurrentFabricRemoveCallback;

chip::Controller::CommissioningParameters GetCommissioningParameters();
void InitPairingCommand();
CHIP_ERROR Pair(chip::NodeId remoteId, chip::Transport::PeerAddress address);

static void OnCurrentFabricRemove(void * context, chip::NodeId remoteNodeId, CHIP_ERROR status);
static void OnPairDeviceWithCode(intptr_t context);
static void OnPairDevice(intptr_t context);
static void OnUnpairDevice(intptr_t context);
};

0 comments on commit 78aac89

Please sign in to comment.