From 45a68f88939dec2f2372a7e37abeeae627f81411 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Sun, 6 Oct 2024 19:39:53 -0700 Subject: [PATCH] Use ScheduleLambda() instead of PlatformMgr().ScheduleWork to excute command (#35926) * Use ScheduleLambda() instead of PlatformMgr().ScheduleWork to excute command * Restyled by clang-format --------- Co-authored-by: Restyled.io --- .../fabric-sync/FabricSyncCommand.cpp | 11 +- .../device_manager/PairingManager.cpp | 269 ++++++++---------- .../device_manager/PairingManager.h | 52 +--- 3 files changed, 129 insertions(+), 203 deletions(-) diff --git a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp index c8d9f3da96b323..879d28cd156956 100644 --- a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp +++ b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp @@ -30,15 +30,6 @@ using namespace ::chip; -namespace { - -void CheckFabricBridgeSynchronizationSupport(intptr_t ignored) -{ - DeviceMgr().ReadSupportedDeviceCategories(); -} - -} // namespace - void FabricSyncAddBridgeCommand::OnCommissioningComplete(NodeId deviceId, CHIP_ERROR err) { if (mBridgeNodeId != deviceId) @@ -73,7 +64,7 @@ void FabricSyncAddBridgeCommand::OnCommissioningComplete(NodeId deviceId, CHIP_E // // Note: The Fabric-Admin MUST NOT send the RequestCommissioningApproval command // if the remote Fabric-Bridge lacks Fabric Synchronization support. - DeviceLayer::PlatformMgr().ScheduleWork(CheckFabricBridgeSynchronizationSupport, 0); + DeviceLayer::SystemLayer().ScheduleLambda([]() { DeviceMgr().ReadSupportedDeviceCategories(); }); } } else diff --git a/examples/fabric-admin/device_manager/PairingManager.cpp b/examples/fabric-admin/device_manager/PairingManager.cpp index c46a0ca000897b..41b4c8744e16b3 100644 --- a/examples/fabric-admin/device_manager/PairingManager.cpp +++ b/examples/fabric-admin/device_manager/PairingManager.cpp @@ -131,98 +131,97 @@ CHIP_ERROR PairingManager::OpenCommissioningWindow(NodeId nodeId, EndpointId end return CHIP_ERROR_INCORRECT_STATE; } - auto params = Platform::MakeUnique(); - params->nodeId = nodeId; - params->endpointId = endpointId; - params->commissioningWindowTimeout = commissioningTimeoutSec; - params->iteration = iterations; - params->discriminator = discriminator; + // Ensure salt and verifier sizes are valid + if (!salt.empty() && salt.size() > chip::Crypto::kSpake2p_Max_PBKDF_Salt_Length) + { + ChipLogError(NotSpecified, "Salt size exceeds buffer capacity"); + return CHIP_ERROR_BUFFER_TOO_SMALL; + } - if (!salt.empty()) + if (!verifier.empty() && verifier.size() > chip::Crypto::kSpake2p_VerifierSerialized_Length) { - if (salt.size() > sizeof(params->saltBuffer)) - { - ChipLogError(NotSpecified, "Salt size exceeds buffer capacity"); - return CHIP_ERROR_BUFFER_TOO_SMALL; - } + ChipLogError(NotSpecified, "Verifier size exceeds buffer capacity"); + return CHIP_ERROR_BUFFER_TOO_SMALL; + } - memcpy(params->saltBuffer, salt.data(), salt.size()); - params->salt = ByteSpan(params->saltBuffer, salt.size()); + if (!salt.empty()) + { + memcpy(mSaltBuffer, salt.data(), salt.size()); + mSalt = ByteSpan(mSaltBuffer, salt.size()); + } + else + { + mSalt = ByteSpan(); } if (!verifier.empty()) { - if (verifier.size() > sizeof(params->verifierBuffer)) - { - ChipLogError(NotSpecified, "Verifier size exceeds buffer capacity"); - return CHIP_ERROR_BUFFER_TOO_SMALL; - } - - memcpy(params->verifierBuffer, verifier.data(), verifier.size()); - params->verifier = ByteSpan(params->verifierBuffer, verifier.size()); + memcpy(mVerifierBuffer, verifier.data(), verifier.size()); + mVerifier = ByteSpan(mVerifierBuffer, verifier.size()); } - - // Schedule work on the Matter thread - return DeviceLayer::PlatformMgr().ScheduleWork(OnOpenCommissioningWindow, reinterpret_cast(params.release())); -} - -void PairingManager::OnOpenCommissioningWindow(intptr_t context) -{ - Platform::UniquePtr params(reinterpret_cast(context)); - PairingManager & self = PairingManager::Instance(); - - if (self.mCommissioner == nullptr) + else { - ChipLogError(NotSpecified, "Commissioner is null, cannot open commissioning window"); - return; + mVerifier = ByteSpan(); } - self.mWindowOpener = Platform::MakeUnique(self.mCommissioner); + return DeviceLayer::SystemLayer().ScheduleLambda([nodeId, endpointId, commissioningTimeoutSec, iterations, discriminator]() { + PairingManager & self = PairingManager::Instance(); - if (!params->verifier.empty()) - { - if (params->salt.empty()) + if (self.mCommissioner == nullptr) { - ChipLogError(NotSpecified, "Salt is required when verifier is set"); - self.mWindowOpener.reset(); + ChipLogError(NotSpecified, "Commissioner is null, cannot open commissioning window"); return; } - CHIP_ERROR err = - self.mWindowOpener->OpenCommissioningWindow(Controller::CommissioningWindowVerifierParams() - .SetNodeId(params->nodeId) - .SetEndpointId(params->endpointId) - .SetTimeout(params->commissioningWindowTimeout) - .SetIteration(params->iteration) - .SetDiscriminator(params->discriminator) - .SetVerifier(params->verifier) - .SetSalt(params->salt) - .SetCallback(&self.mOnOpenCommissioningWindowVerifierCallback)); - if (err != CHIP_NO_ERROR) + self.mWindowOpener = Platform::MakeUnique(self.mCommissioner); + + if (!self.mVerifier.empty()) { - ChipLogError(NotSpecified, "Failed to open commissioning window with verifier: %s", ErrorStr(err)); - self.mWindowOpener.reset(); + if (self.mSalt.empty()) + { + ChipLogError(NotSpecified, "Salt is required when verifier is set"); + self.mWindowOpener.reset(); + return; + } + + // Open the commissioning window with verifier parameters + CHIP_ERROR err = + self.mWindowOpener->OpenCommissioningWindow(Controller::CommissioningWindowVerifierParams() + .SetNodeId(nodeId) + .SetEndpointId(endpointId) + .SetTimeout(commissioningTimeoutSec) + .SetIteration(iterations) + .SetDiscriminator(discriminator) + .SetVerifier(self.mVerifier) + .SetSalt(self.mSalt) + .SetCallback(&self.mOnOpenCommissioningWindowVerifierCallback)); + if (err != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to open commissioning window with verifier: %s", ErrorStr(err)); + self.mWindowOpener.reset(); + } } - } - else - { - SetupPayload ignored; - CHIP_ERROR err = self.mWindowOpener->OpenCommissioningWindow(Controller::CommissioningWindowPasscodeParams() - .SetNodeId(params->nodeId) - .SetEndpointId(params->endpointId) - .SetTimeout(params->commissioningWindowTimeout) - .SetIteration(params->iteration) - .SetDiscriminator(params->discriminator) - .SetSetupPIN(NullOptional) - .SetSalt(NullOptional) - .SetCallback(&self.mOnOpenCommissioningWindowCallback), - ignored); - if (err != CHIP_NO_ERROR) + else { - ChipLogError(NotSpecified, "Failed to open commissioning window with passcode: %s", ErrorStr(err)); - self.mWindowOpener.reset(); + SetupPayload ignored; + // Open the commissioning window with passcode parameters + CHIP_ERROR err = self.mWindowOpener->OpenCommissioningWindow(Controller::CommissioningWindowPasscodeParams() + .SetNodeId(nodeId) + .SetEndpointId(endpointId) + .SetTimeout(commissioningTimeoutSec) + .SetIteration(iterations) + .SetDiscriminator(discriminator) + .SetSetupPIN(NullOptional) + .SetSalt(NullOptional) + .SetCallback(&self.mOnOpenCommissioningWindowCallback), + ignored); + if (err != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to open commissioning window with passcode: %s", ErrorStr(err)); + self.mWindowOpener.reset(); + } } - } + }); } void PairingManager::OnOpenCommissioningWindowResponse(void * context, NodeId remoteId, CHIP_ERROR err, SetupPayload payload) @@ -290,7 +289,7 @@ void PairingManager::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err) if (err == CHIP_NO_ERROR) { // print to console - fprintf(stderr, "New device with Node ID: " ChipLogFormatX64 "has been successfully added.\n", ChipLogValueX64(nodeId)); + 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. @@ -580,34 +579,24 @@ CHIP_ERROR PairingManager::PairDeviceWithCode(NodeId nodeId, const char * payloa return CHIP_ERROR_INVALID_STRING_LENGTH; } - auto params = Platform::MakeUnique(); - VerifyOrReturnError(params != nullptr, CHIP_ERROR_NO_MEMORY); - - params->nodeId = nodeId; - Platform::CopyString(params->payloadBuffer, sizeof(params->payloadBuffer), payload); + Platform::CopyString(mOnboardingPayload, sizeof(mOnboardingPayload), payload); - // Schedule work on the Matter thread - return DeviceLayer::PlatformMgr().ScheduleWork(OnPairDeviceWithCode, reinterpret_cast(params.release())); -} - -void PairingManager::OnPairDeviceWithCode(intptr_t context) -{ - Platform::UniquePtr params(reinterpret_cast(context)); - PairingManager & self = PairingManager::Instance(); + return DeviceLayer::SystemLayer().ScheduleLambda([nodeId]() { + PairingManager & self = PairingManager::Instance(); - self.InitPairingCommand(); + self.InitPairingCommand(); - CommissioningParameters commissioningParams = self.GetCommissioningParameters(); - auto discoveryType = DiscoveryType::kDiscoveryNetworkOnly; + CommissioningParameters commissioningParams = self.GetCommissioningParameters(); + auto discoveryType = DiscoveryType::kDiscoveryNetworkOnly; - self.mNodeId = params->nodeId; - self.mOnboardingPayload = params->payloadBuffer; + self.mNodeId = nodeId; - CHIP_ERROR err = self.mCommissioner->PairDevice(params->nodeId, params->payloadBuffer, commissioningParams, discoveryType); - if (err != CHIP_NO_ERROR) - { - ChipLogError(NotSpecified, "Failed to pair device with code, error: %s", ErrorStr(err)); - } + CHIP_ERROR err = self.mCommissioner->PairDevice(nodeId, self.mOnboardingPayload, commissioningParams, discoveryType); + if (err != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to pair device with code, error: %s", ErrorStr(err)); + } + }); } CHIP_ERROR PairingManager::PairDevice(chip::NodeId nodeId, uint32_t setupPINCode, const char * deviceRemoteIp, @@ -619,72 +608,50 @@ CHIP_ERROR PairingManager::PairDevice(chip::NodeId nodeId, uint32_t setupPINCode return CHIP_ERROR_INVALID_STRING_LENGTH; } - auto params = Platform::MakeUnique(); - VerifyOrReturnError(params != nullptr, CHIP_ERROR_NO_MEMORY); - - params->nodeId = nodeId; - params->setupPINCode = setupPINCode; - params->deviceRemotePort = deviceRemotePort; - - Platform::CopyString(params->ipAddrBuffer, sizeof(params->ipAddrBuffer), deviceRemoteIp); - - // Schedule work on the Matter thread - return DeviceLayer::PlatformMgr().ScheduleWork(OnPairDevice, reinterpret_cast(params.release())); -} + Platform::CopyString(mRemoteIpAddr, sizeof(mRemoteIpAddr), deviceRemoteIp); -void PairingManager::OnPairDevice(intptr_t context) -{ - Platform::UniquePtr params(reinterpret_cast(context)); - PairingManager & self = PairingManager::Instance(); + return DeviceLayer::SystemLayer().ScheduleLambda([nodeId, setupPINCode, deviceRemotePort]() { + PairingManager & self = PairingManager::Instance(); - self.InitPairingCommand(); - self.mSetupPINCode = params->setupPINCode; + self.InitPairingCommand(); + self.mSetupPINCode = setupPINCode; - Inet::IPAddress address; - Inet::InterfaceId interfaceId; + Inet::IPAddress address; + Inet::InterfaceId interfaceId; - if (!ParseAddressWithInterface(params->ipAddrBuffer, address, interfaceId)) - { - ChipLogError(NotSpecified, "Invalid IP address: %s", params->ipAddrBuffer); - return; - } + if (!ParseAddressWithInterface(self.mRemoteIpAddr, address, interfaceId)) + { + ChipLogError(NotSpecified, "Invalid IP address: %s", self.mRemoteIpAddr); + return; + } - CHIP_ERROR err = self.Pair(params->nodeId, Transport::PeerAddress::UDP(address, params->deviceRemotePort, interfaceId)); - if (err != CHIP_NO_ERROR) - { - ChipLogError(NotSpecified, "Failed to pair device, error: %s", ErrorStr(err)); - } + CHIP_ERROR err = self.Pair(nodeId, Transport::PeerAddress::UDP(address, deviceRemotePort, interfaceId)); + if (err != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to pair device, error: %s", ErrorStr(err)); + } + }); } CHIP_ERROR PairingManager::UnpairDevice(NodeId nodeId) { - auto params = Platform::MakeUnique(); - VerifyOrReturnError(params != nullptr, CHIP_ERROR_NO_MEMORY); - - params->nodeId = nodeId; - - // Schedule work on the Matter thread - return DeviceLayer::PlatformMgr().ScheduleWork(OnUnpairDevice, reinterpret_cast(params.release())); -} - -void PairingManager::OnUnpairDevice(intptr_t context) -{ - Platform::UniquePtr params(reinterpret_cast(context)); - PairingManager & self = PairingManager::Instance(); + return DeviceLayer::SystemLayer().ScheduleLambda([nodeId]() { + PairingManager & self = PairingManager::Instance(); - self.InitPairingCommand(); + self.InitPairingCommand(); - self.mCurrentFabricRemover = Platform::MakeUnique(self.mCommissioner); + self.mCurrentFabricRemover = Platform::MakeUnique(self.mCommissioner); - if (!self.mCurrentFabricRemover) - { - ChipLogError(NotSpecified, "Failed to unpair device, mCurrentFabricRemover is null"); - return; - } + 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)); - } + CHIP_ERROR err = self.mCurrentFabricRemover->RemoveCurrentFabric(nodeId, &self.mCurrentFabricRemoveCallback); + if (err != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to unpair device, error: %s", ErrorStr(err)); + } + }); } diff --git a/examples/fabric-admin/device_manager/PairingManager.h b/examples/fabric-admin/device_manager/PairingManager.h index 563d129079d3f1..50e64f9f0ca67f 100644 --- a/examples/fabric-admin/device_manager/PairingManager.h +++ b/examples/fabric-admin/device_manager/PairingManager.h @@ -140,39 +140,6 @@ class PairingManager : public chip::Controller::DevicePairingDelegate, CHIP_ERROR UnpairDevice(chip::NodeId nodeId); private: - struct CommissioningWindowParams - { - chip::NodeId nodeId; - chip::EndpointId endpointId; - uint16_t commissioningWindowTimeout; - uint32_t iteration; - uint16_t discriminator; - chip::Optional setupPIN; - uint8_t verifierBuffer[chip::Crypto::kSpake2p_VerifierSerialized_Length]; - chip::ByteSpan verifier; - uint8_t saltBuffer[chip::Crypto::kSpake2p_Max_PBKDF_Salt_Length]; - chip::ByteSpan salt; - }; - - struct PairDeviceWithCodeParams - { - chip::NodeId nodeId; - char payloadBuffer[kMaxManualCodeLength + 1]; - }; - - struct PairDeviceParams - { - chip::NodeId nodeId; - uint32_t setupPINCode; - uint16_t deviceRemotePort; - char ipAddrBuffer[chip::Inet::IPAddress::kMaxStringLength]; - }; - - struct UnpairDeviceParams - { - chip::NodeId nodeId; - }; - // Constructors PairingManager(); PairingManager(const PairingManager &) = delete; @@ -202,14 +169,10 @@ class PairingManager : public chip::Controller::DevicePairingDelegate, const chip::Credentials::DeviceAttestationVerifier::AttestationDeviceInfo & info, chip::Credentials::AttestationVerificationResult attestationResult) override; - 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); // Private data members chip::Controller::DeviceCommissioner * mCommissioner = nullptr; @@ -219,12 +182,17 @@ class PairingManager : public chip::Controller::DevicePairingDelegate, CommissioningDelegate * mCommissioningDelegate = nullptr; PairingDelegate * mPairingDelegate = nullptr; - chip::NodeId mNodeId = chip::kUndefinedNodeId; - uint16_t mDiscriminator = 0; - uint32_t mSetupPINCode = 0; - const char * mOnboardingPayload = nullptr; - bool mDeviceIsICD = false; + chip::NodeId mNodeId = chip::kUndefinedNodeId; + chip::ByteSpan mVerifier; + chip::ByteSpan mSalt; + uint16_t mDiscriminator = 0; + uint32_t mSetupPINCode = 0; + bool mDeviceIsICD = false; uint8_t mRandomGeneratedICDSymmetricKey[chip::Crypto::kAES_CCM128_Key_Length]; + uint8_t mVerifierBuffer[chip::Crypto::kSpake2p_VerifierSerialized_Length]; + uint8_t mSaltBuffer[chip::Crypto::kSpake2p_Max_PBKDF_Salt_Length]; + char mRemoteIpAddr[chip::Inet::IPAddress::kMaxStringLength]; + char mOnboardingPayload[kMaxManualCodeLength + 1]; chip::Optional mICDRegistration; chip::Optional mICDCheckInNodeId;