From b48b103b3e3e43ab4e38bd0016e9deb214d18ddc Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Sun, 13 Oct 2024 11:29:14 -0700 Subject: [PATCH 1/8] Implement CommissionerControl command APIs --- examples/fabric-admin/BUILD.gn | 2 + .../commands/clusters/ClusterCommand.h | 1 - .../device_manager/CommissionerControl.cpp | 142 ++++++++++++++++++ .../device_manager/CommissionerControl.h | 122 +++++++++++++++ .../device_manager/DeviceManager.cpp | 50 +++--- .../device_manager/DeviceManager.h | 4 +- 6 files changed, 299 insertions(+), 22 deletions(-) create mode 100644 examples/fabric-admin/device_manager/CommissionerControl.cpp create mode 100644 examples/fabric-admin/device_manager/CommissionerControl.h diff --git a/examples/fabric-admin/BUILD.gn b/examples/fabric-admin/BUILD.gn index ab584459582657..c7ef24f29a3dff 100644 --- a/examples/fabric-admin/BUILD.gn +++ b/examples/fabric-admin/BUILD.gn @@ -80,6 +80,8 @@ static_library("fabric-admin-utils") { "commands/pairing/OpenCommissioningWindowCommand.h", "commands/pairing/PairingCommand.cpp", "commands/pairing/ToTLVCert.cpp", + "device_manager/CommissionerControl.cpp", + "device_manager/CommissionerControl.h", "device_manager/DeviceManager.cpp", "device_manager/DeviceManager.h", "device_manager/DeviceSubscription.cpp", diff --git a/examples/fabric-admin/commands/clusters/ClusterCommand.h b/examples/fabric-admin/commands/clusters/ClusterCommand.h index edf2302219fa1c..7c66bebcecfbea 100644 --- a/examples/fabric-admin/commands/clusters/ClusterCommand.h +++ b/examples/fabric-admin/commands/clusters/ClusterCommand.h @@ -84,7 +84,6 @@ class ClusterCommand : public InteractionModelCommands, public ModelCommand, pub if (data != nullptr) { LogErrorOnFailure(RemoteDataModelLogger::LogCommandAsJSON(path, data)); - DeviceMgr().HandleCommandResponse(path, *data); } } diff --git a/examples/fabric-admin/device_manager/CommissionerControl.cpp b/examples/fabric-admin/device_manager/CommissionerControl.cpp new file mode 100644 index 00000000000000..89d56b3d468545 --- /dev/null +++ b/examples/fabric-admin/device_manager/CommissionerControl.cpp @@ -0,0 +1,142 @@ +#include "CommissionerControl.h" +#include + +using namespace ::chip; + +void CommissionerControl::Init(Controller::DeviceCommissioner & commissioner, NodeId nodeId, EndpointId endpointId) +{ + ChipLogProgress(NotSpecified, "Initilize CommissionerControl"); + mCommissioner = &commissioner; + mDestinationId = nodeId; + mEndpointId = endpointId; +} + +CHIP_ERROR CommissionerControl::RequestCommissioningApproval(uint64_t requestId, uint16_t vendorId, uint16_t productId, + Optional label) +{ + VerifyOrReturnError(mCommissioner != nullptr, CHIP_ERROR_INCORRECT_STATE); + + ChipLogProgress(NotSpecified, "Sending RequestCommissioningApproval to node " ChipLogFormatX64, + ChipLogValueX64(mDestinationId)); + + mRequestCommissioningApproval.requestID = requestId; + mRequestCommissioningApproval.vendorID = static_cast(vendorId); + mRequestCommissioningApproval.productID = productId; + mRequestCommissioningApproval.label = label; + + CommissioneeDeviceProxy * commissioneeDeviceProxy = nullptr; + if (CHIP_NO_ERROR == mCommissioner->GetDeviceBeingCommissioned(mDestinationId, &commissioneeDeviceProxy)) + { + return SendCommandForType(CommandType::kRequestCommissioningApproval, commissioneeDeviceProxy); + } + + mCommandType = CommandType::kRequestCommissioningApproval; + return mCommissioner->GetConnectedDevice(mDestinationId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback); +} + +CHIP_ERROR CommissionerControl::CommissionNode(uint64_t requestId, uint16_t responseTimeoutSeconds) +{ + VerifyOrReturnError(mCommissioner != nullptr, CHIP_ERROR_INCORRECT_STATE); + + ChipLogProgress(NotSpecified, "Sending CommissionNode to node " ChipLogFormatX64, ChipLogValueX64(mDestinationId)); + + mCommissionNode.requestID = requestId; + mCommissionNode.responseTimeoutSeconds = responseTimeoutSeconds; + + CommissioneeDeviceProxy * commissioneeDeviceProxy = nullptr; + if (CHIP_NO_ERROR == mCommissioner->GetDeviceBeingCommissioned(mDestinationId, &commissioneeDeviceProxy)) + { + return SendCommandForType(CommandType::kCommissionNode, commissioneeDeviceProxy); + } + + mCommandType = CommandType::kCommissionNode; + return mCommissioner->GetConnectedDevice(mDestinationId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback); +} + +void CommissionerControl::OnResponse(app::CommandSender * client, const app::ConcreteCommandPath & path, + const app::StatusIB & status, TLV::TLVReader * data) +{ + ChipLogProgress(NotSpecified, "CommissionerControl: OnResponse."); + + CHIP_ERROR error = status.ToChipError(); + if (CHIP_NO_ERROR != error) + { + ChipLogError(NotSpecified, "Response Failure: %s", ErrorStr(error)); + return; + } + + if (data != nullptr) + { + DeviceMgr().HandleCommandResponse(path, *data); + } +} + +void CommissionerControl::OnError(const app::CommandSender * client, CHIP_ERROR error) +{ + // Handle the error, then reset mCommandSender + ChipLogProgress(NotSpecified, "CommissionerControl: OnError: Error: %s", ErrorStr(error)); + mCommandSender.reset(); +} + +void CommissionerControl::OnDone(app::CommandSender * client) +{ + ChipLogProgress(NotSpecified, "CommissionerControl: OnDone."); + + switch (mCommandType) + { + case CommandType::kRequestCommissioningApproval: + ChipLogProgress(NotSpecified, "CommissionerControl: Command RequestCommissioningApproval has been successfully processed."); + break; + + case CommandType::kCommissionNode: + ChipLogProgress(NotSpecified, "CommissionerControl: Command CommissionNode has been successfully processed."); + break; + + default: + ChipLogError(NotSpecified, "CommissionerControl: Unknown or unhandled command type in OnDone."); + break; + } + + // Reset command type to undefined after processing is done + mCommandType = CommandType::kUndefined; + + // Ensure that mCommandSender is cleaned up after it is done + mCommandSender.reset(); +} + +CHIP_ERROR CommissionerControl::SendCommandForType(CommandType commandType, DeviceProxy * device) +{ + switch (commandType) + { + case CommandType::kRequestCommissioningApproval: + return SendCommand(device, mEndpointId, app::Clusters::CommissionerControl::Id, + app::Clusters::CommissionerControl::Commands::RequestCommissioningApproval::Id, + mRequestCommissioningApproval); + case CommandType::kCommissionNode: + return SendCommand(device, mEndpointId, app::Clusters::CommissionerControl::Id, + app::Clusters::CommissionerControl::Commands::CommissionNode::Id, mCommissionNode); + default: + return CHIP_ERROR_INVALID_ARGUMENT; + } +} + +void CommissionerControl::OnDeviceConnectedFn(void * context, Messaging::ExchangeManager & exchangeMgr, + const SessionHandle & sessionHandle) +{ + CommissionerControl * self = reinterpret_cast(context); + VerifyOrReturn(self != nullptr, ChipLogError(NotSpecified, "OnDeviceConnectedFn: context is null")); + + OperationalDeviceProxy device(&exchangeMgr, sessionHandle); + + CHIP_ERROR err = self->SendCommandForType(self->mCommandType, &device); + ; + LogErrorOnFailure(err); +} + +void CommissionerControl::OnDeviceConnectionFailureFn(void * context, const ScopedNodeId & peerId, CHIP_ERROR err) +{ + LogErrorOnFailure(err); + + CommissionerControl * self = reinterpret_cast(context); + VerifyOrReturn(self != nullptr, ChipLogError(NotSpecified, "OnDeviceConnectionFailureFn: context is null")); +} diff --git a/examples/fabric-admin/device_manager/CommissionerControl.h b/examples/fabric-admin/device_manager/CommissionerControl.h new file mode 100644 index 00000000000000..685b7186488f6c --- /dev/null +++ b/examples/fabric-admin/device_manager/CommissionerControl.h @@ -0,0 +1,122 @@ +/* + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#pragma once + +#include +#include + +/** + * @class CommissionerControl + * @brief This class handles commissioning control operations using CHIP commands, including sending + * commissioning approval requests and commissioning nodes. + * + * The class implements the `chip::app::CommandSender::Callback` interface to handle responses, errors, + * and completion events for the commands it sends. It allows sending commands related to commissioning + * and tracks the status of the operations. + */ +class CommissionerControl : public chip::app::CommandSender::Callback +{ +public: + CommissionerControl() : + mOnDeviceConnectedCallback(OnDeviceConnectedFn, this), mOnDeviceConnectionFailureCallback(OnDeviceConnectionFailureFn, this) + {} + + /** + * @brief Initializes the CommissionerControl with a DeviceCommissioner, NodeId, and EndpointId. + * + * @param commissioner The DeviceCommissioner to use for the commissioning process. + * @param nodeId The node ID of the remote fabric bridge. + * @param endpointId The endpoint on which to send CommissionerControl commands. + */ + void Init(chip::Controller::DeviceCommissioner & commissioner, chip::NodeId nodeId, chip::EndpointId endpointId); + + /** + * @brief Sends a RequestCommissioningApproval command to the device. + * + * @param requestId The unique request ID. + * @param vendorId The vendor ID of the device. + * @param productId The product ID of the device. + * @param label Optional label for the device. + * @return CHIP_ERROR CHIP_NO_ERROR on success, or an appropriate error code on failure. + */ + CHIP_ERROR RequestCommissioningApproval(uint64_t requestId, uint16_t vendorId, uint16_t productId, + chip::Optional label); + /** + * @brief Sends a CommissionNode command to the device. + * + * @param requestId The unique request ID. + * @param responseTimeoutSeconds Timeout for the response in seconds. + * @return CHIP_ERROR CHIP_NO_ERROR on success, or an appropriate error code on failure. + */ + CHIP_ERROR CommissionNode(uint64_t requestId, uint16_t responseTimeoutSeconds); + + /////////// CommandSender Callback Interface ///////// + virtual void OnResponse(chip::app::CommandSender * client, const chip::app::ConcreteCommandPath & path, + const chip::app::StatusIB & status, chip::TLV::TLVReader * data) override; + + virtual void OnError(const chip::app::CommandSender * client, CHIP_ERROR error) override; + + virtual void OnDone(chip::app::CommandSender * client) override; + +private: + enum class CommandType : uint8_t + { + kUndefined = 0, + kRequestCommissioningApproval = 1, + kCommissionNode = 2, + }; + + template + CHIP_ERROR SendCommand(chip::DeviceProxy * device, chip::EndpointId endpointId, chip::ClusterId clusterId, + chip::CommandId commandId, const T & value) + { + chip::app::CommandPathParams commandPath = { endpointId, clusterId, commandId, + (chip::app::CommandPathFlags::kEndpointIdValid) }; + mCommandSender = std::make_unique(this, device->GetExchangeManager(), false, false, + device->GetSecureSession().Value()->AllowsLargePayload()); + + VerifyOrReturnError(mCommandSender != nullptr, CHIP_ERROR_NO_MEMORY); + + chip::app::CommandSender::AddRequestDataParameters addRequestDataParams(chip::NullOptional); + // Using TestOnly AddRequestData to allow for an intentionally malformed request for server validation testing. + ReturnErrorOnFailure(mCommandSender->TestOnlyAddRequestDataNoTimedCheck(commandPath, value, addRequestDataParams)); + ReturnErrorOnFailure(mCommandSender->SendCommandRequest(device->GetSecureSession().Value())); + + return CHIP_NO_ERROR; + } + + CHIP_ERROR SendCommandForType(CommandType commandType, chip::DeviceProxy * device); + + static void OnDeviceConnectedFn(void * context, chip::Messaging::ExchangeManager & exchangeMgr, + const chip::SessionHandle & sessionHandle); + static void OnDeviceConnectionFailureFn(void * context, const chip::ScopedNodeId & peerId, CHIP_ERROR error); + + // Private data members + chip::Controller::DeviceCommissioner * mCommissioner = nullptr; + std::unique_ptr mCommandSender; + chip::NodeId mDestinationId = chip::kUndefinedNodeId; + chip::EndpointId mEndpointId = chip::kRootEndpointId; + CommandType mCommandType = CommandType::kUndefined; + + chip::Callback::Callback mOnDeviceConnectedCallback; + chip::Callback::Callback mOnDeviceConnectionFailureCallback; + + chip::app::Clusters::CommissionerControl::Commands::RequestCommissioningApproval::Type mRequestCommissioningApproval; + chip::app::Clusters::CommissionerControl::Commands::CommissionNode::Type mCommissionNode; +}; diff --git a/examples/fabric-admin/device_manager/DeviceManager.cpp b/examples/fabric-admin/device_manager/DeviceManager.cpp index 3a27c68184c8b0..feb93013cbf5d6 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.cpp +++ b/examples/fabric-admin/device_manager/DeviceManager.cpp @@ -26,7 +26,6 @@ #include using namespace chip; -using namespace chip::app::Clusters; namespace { @@ -234,7 +233,7 @@ void DeviceManager::HandleReadSupportedDeviceCategories(TLV::TLVReader & data) { ChipLogProgress(NotSpecified, "Attribute SupportedDeviceCategories detected."); - BitMask value; + BitMask value; CHIP_ERROR error = app::DataModel::Decode(data, value); if (error != CHIP_NO_ERROR) { @@ -242,9 +241,10 @@ void DeviceManager::HandleReadSupportedDeviceCategories(TLV::TLVReader & data) return; } - if (value.Has(CommissionerControl::SupportedDeviceCategoryBitmap::kFabricSynchronization)) + if (value.Has(app::Clusters::CommissionerControl::SupportedDeviceCategoryBitmap::kFabricSynchronization)) { ChipLogProgress(NotSpecified, "Remote Fabric-Bridge supports Fabric Synchronization, start reverse commissioning."); + mCommissionerControl.Init(PairingManager::Instance().CurrentCommissioner(), mRemoteBridgeNodeId, kAggregatorEndpointId); RequestCommissioningApproval(); } } @@ -258,19 +258,24 @@ void DeviceManager::RequestCommissioningApproval() uint16_t vendorId = static_cast(CHIP_DEVICE_CONFIG_DEVICE_VENDOR_ID); uint16_t productId = static_cast(CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID); - StringBuilder commandBuilder; - commandBuilder.Add("commissionercontrol request-commissioning-approval "); - commandBuilder.AddFormat("%lu %u %u %lu %d", requestId, vendorId, productId, mRemoteBridgeNodeId, kAggregatorEndpointId); + CHIP_ERROR error = mCommissionerControl.RequestCommissioningApproval(requestId, vendorId, productId, NullOptional); + + if (error != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, + "Failed to request commissioning-approval to the remote bridge (NodeId: %lu). Error: %" CHIP_ERROR_FORMAT, + mRemoteBridgeNodeId, error.Format()); + return; + } mRequestId = requestId; - PushCommand(commandBuilder.c_str()); } void DeviceManager::HandleCommissioningRequestResult(TLV::TLVReader & data) { ChipLogProgress(NotSpecified, "CommissioningRequestResult event received."); - CommissionerControl::Events::CommissioningRequestResult::DecodableType value; + app::Clusters::CommissionerControl::Events::CommissioningRequestResult::DecodableType value; CHIP_ERROR error = app::DataModel::Decode(data, value); if (error != CHIP_NO_ERROR) { @@ -397,16 +402,20 @@ void DeviceManager::SendCommissionNodeRequest(uint64_t requestId, uint16_t respo { ChipLogProgress(NotSpecified, "Request the Commissioner Control Server to begin commissioning a previously approved request."); - StringBuilder commandBuilder; - commandBuilder.Add("commissionercontrol commission-node "); - commandBuilder.AddFormat("%lu %u %lu %d", requestId, responseTimeoutSeconds, mRemoteBridgeNodeId, kAggregatorEndpointId); + CHIP_ERROR error = mCommissionerControl.CommissionNode(requestId, responseTimeoutSeconds); - PushCommand(commandBuilder.c_str()); + if (error != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, + "Failed to send CommissionNode command to the remote bridge (NodeId: %lu). Error: %" CHIP_ERROR_FORMAT, + mRemoteBridgeNodeId, error.Format()); + return; + } } void DeviceManager::HandleReverseOpenCommissioningWindow(TLV::TLVReader & data) { - CommissionerControl::Commands::ReverseOpenCommissioningWindow::DecodableType value; + app::Clusters::CommissionerControl::Commands::ReverseOpenCommissioningWindow::DecodableType value; CHIP_ERROR error = app::DataModel::Decode(data, value); if (error != CHIP_NO_ERROR) @@ -430,14 +439,15 @@ void DeviceManager::HandleReverseOpenCommissioningWindow(TLV::TLVReader & data) void DeviceManager::HandleAttributeData(const app::ConcreteDataAttributePath & path, TLV::TLVReader & data) { - if (path.mClusterId == CommissionerControl::Id && - path.mAttributeId == CommissionerControl::Attributes::SupportedDeviceCategories::Id) + if (path.mClusterId == app::Clusters::CommissionerControl::Id && + path.mAttributeId == app::Clusters::CommissionerControl::Attributes::SupportedDeviceCategories::Id) { HandleReadSupportedDeviceCategories(data); return; } - if (path.mClusterId == Descriptor::Id && path.mAttributeId == Descriptor::Attributes::PartsList::Id) + if (path.mClusterId == app::Clusters::Descriptor::Id && + path.mAttributeId == app::Clusters::Descriptor::Attributes::PartsList::Id) { HandleAttributePartsListUpdate(data); return; @@ -446,8 +456,8 @@ void DeviceManager::HandleAttributeData(const app::ConcreteDataAttributePath & p void DeviceManager::HandleEventData(const app::EventHeader & header, TLV::TLVReader & data) { - if (header.mPath.mClusterId == CommissionerControl::Id && - header.mPath.mEventId == CommissionerControl::Events::CommissioningRequestResult::Id) + if (header.mPath.mClusterId == app::Clusters::CommissionerControl::Id && + header.mPath.mEventId == app::Clusters::CommissionerControl::Events::CommissioningRequestResult::Id) { HandleCommissioningRequestResult(data); } @@ -457,8 +467,8 @@ void DeviceManager::HandleCommandResponse(const app::ConcreteCommandPath & path, { ChipLogProgress(NotSpecified, "Command Response received."); - if (path.mClusterId == CommissionerControl::Id && - path.mCommandId == CommissionerControl::Commands::ReverseOpenCommissioningWindow::Id) + if (path.mClusterId == app::Clusters::CommissionerControl::Id && + path.mCommandId == app::Clusters::CommissionerControl::Commands::ReverseOpenCommissioningWindow::Id) { HandleReverseOpenCommissioningWindow(data); } diff --git a/examples/fabric-admin/device_manager/DeviceManager.h b/examples/fabric-admin/device_manager/DeviceManager.h index 1514c417be4b4d..0b69326d5f5b11 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.h +++ b/examples/fabric-admin/device_manager/DeviceManager.h @@ -19,9 +19,9 @@ #pragma once #include +#include #include #include - #include constexpr uint32_t kDefaultSetupPinCode = 20202021; @@ -209,6 +209,8 @@ class DeviceManager : public PairingDelegate bool mAutoSyncEnabled = false; bool mInitialized = false; uint64_t mRequestId = 0; + + CommissionerControl mCommissionerControl; }; /** From c57bd7ba3549f6889c57309ca4b9b7078122a73d Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Wed, 16 Oct 2024 08:10:36 -0700 Subject: [PATCH 2/8] Address review comment --- .../device_manager/CommissionerControl.cpp | 5 +++-- .../device_manager/CommissionerControl.h | 15 +++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/examples/fabric-admin/device_manager/CommissionerControl.cpp b/examples/fabric-admin/device_manager/CommissionerControl.cpp index 89d56b3d468545..8c88557235c853 100644 --- a/examples/fabric-admin/device_manager/CommissionerControl.cpp +++ b/examples/fabric-admin/device_manager/CommissionerControl.cpp @@ -5,6 +5,9 @@ using namespace ::chip; void CommissionerControl::Init(Controller::DeviceCommissioner & commissioner, NodeId nodeId, EndpointId endpointId) { + // Ensure that mCommissioner is not already initialized + VerifyOrDie(mCommissioner == nullptr); + ChipLogProgress(NotSpecified, "Initilize CommissionerControl"); mCommissioner = &commissioner; mDestinationId = nodeId; @@ -75,7 +78,6 @@ void CommissionerControl::OnError(const app::CommandSender * client, CHIP_ERROR { // Handle the error, then reset mCommandSender ChipLogProgress(NotSpecified, "CommissionerControl: OnError: Error: %s", ErrorStr(error)); - mCommandSender.reset(); } void CommissionerControl::OnDone(app::CommandSender * client) @@ -129,7 +131,6 @@ void CommissionerControl::OnDeviceConnectedFn(void * context, Messaging::Exchang OperationalDeviceProxy device(&exchangeMgr, sessionHandle); CHIP_ERROR err = self->SendCommandForType(self->mCommandType, &device); - ; LogErrorOnFailure(err); } diff --git a/examples/fabric-admin/device_manager/CommissionerControl.h b/examples/fabric-admin/device_manager/CommissionerControl.h index 685b7186488f6c..1fad323e125c2a 100644 --- a/examples/fabric-admin/device_manager/CommissionerControl.h +++ b/examples/fabric-admin/device_manager/CommissionerControl.h @@ -23,12 +23,13 @@ /** * @class CommissionerControl - * @brief This class handles commissioning control operations using CHIP commands, including sending + * @brief This class handles sending CHIP commands related to commissioning, including sending * commissioning approval requests and commissioning nodes. * - * The class implements the `chip::app::CommandSender::Callback` interface to handle responses, errors, - * and completion events for the commands it sends. It allows sending commands related to commissioning - * and tracks the status of the operations. + * The class acts as a command sender and implements the `chip::app::CommandSender::Callback` interface + * to handle responses, errors, and completion events for the commands it sends. It relies on external + * CCTRL delegate and server mechanisms to manage the overall protocol and state transitions, including + * processing the CommissioningRequestResult and invoking CommissionNode. */ class CommissionerControl : public chip::app::CommandSender::Callback { @@ -75,6 +76,8 @@ class CommissionerControl : public chip::app::CommandSender::Callback virtual void OnDone(chip::app::CommandSender * client) override; private: + static constexpr uint16_t kMaxDeviceLabelLength = 64; + enum class CommandType : uint8_t { kUndefined = 0, @@ -94,8 +97,7 @@ class CommissionerControl : public chip::app::CommandSender::Callback VerifyOrReturnError(mCommandSender != nullptr, CHIP_ERROR_NO_MEMORY); chip::app::CommandSender::AddRequestDataParameters addRequestDataParams(chip::NullOptional); - // Using TestOnly AddRequestData to allow for an intentionally malformed request for server validation testing. - ReturnErrorOnFailure(mCommandSender->TestOnlyAddRequestDataNoTimedCheck(commandPath, value, addRequestDataParams)); + ReturnErrorOnFailure(mCommandSender->AddRequestData(commandPath, value, addRequestDataParams)); ReturnErrorOnFailure(mCommandSender->SendCommandRequest(device->GetSecureSession().Value())); return CHIP_NO_ERROR; @@ -113,6 +115,7 @@ class CommissionerControl : public chip::app::CommandSender::Callback chip::NodeId mDestinationId = chip::kUndefinedNodeId; chip::EndpointId mEndpointId = chip::kRootEndpointId; CommandType mCommandType = CommandType::kUndefined; + char mLabelBuffer[kMaxDeviceLabelLength]; chip::Callback::Callback mOnDeviceConnectedCallback; chip::Callback::Callback mOnDeviceConnectionFailureCallback; From 1c94baba2b2463c0275b9a96e295d438e68901cc Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Wed, 16 Oct 2024 10:32:30 -0700 Subject: [PATCH 3/8] Add missing change --- .../fabric-admin/device_manager/CommissionerControl.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/examples/fabric-admin/device_manager/CommissionerControl.cpp b/examples/fabric-admin/device_manager/CommissionerControl.cpp index 8c88557235c853..b3180b432d30cb 100644 --- a/examples/fabric-admin/device_manager/CommissionerControl.cpp +++ b/examples/fabric-admin/device_manager/CommissionerControl.cpp @@ -25,7 +25,12 @@ CHIP_ERROR CommissionerControl::RequestCommissioningApproval(uint64_t requestId, mRequestCommissioningApproval.requestID = requestId; mRequestCommissioningApproval.vendorID = static_cast(vendorId); mRequestCommissioningApproval.productID = productId; - mRequestCommissioningApproval.label = label; + + if (label.HasValue()) + { + memcpy(mLabelBuffer, label.Value().data(), label.Value().size()); + mRequestCommissioningApproval.label = Optional>(CharSpan(mLabelBuffer, label.Value().size())); + } CommissioneeDeviceProxy * commissioneeDeviceProxy = nullptr; if (CHIP_NO_ERROR == mCommissioner->GetDeviceBeingCommissioned(mDestinationId, &commissioneeDeviceProxy)) From 37c34fd3bc11ee8de9c65f89c1a827e4346e5c5a Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Wed, 16 Oct 2024 10:41:22 -0700 Subject: [PATCH 4/8] Init mCommissionerControl in SetRemoteBridgeNodeId --- examples/fabric-admin/device_manager/DeviceManager.cpp | 7 ++++++- examples/fabric-admin/device_manager/DeviceManager.h | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/examples/fabric-admin/device_manager/DeviceManager.cpp b/examples/fabric-admin/device_manager/DeviceManager.cpp index feb93013cbf5d6..7e2aa3c5829dfd 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.cpp +++ b/examples/fabric-admin/device_manager/DeviceManager.cpp @@ -69,6 +69,12 @@ void DeviceManager::UpdateLastUsedNodeId(NodeId nodeId) } } +void DeviceManager::SetRemoteBridgeNodeId(chip::NodeId nodeId) +{ + mRemoteBridgeNodeId = nodeId; + mCommissionerControl.Init(PairingManager::Instance().CurrentCommissioner(), mRemoteBridgeNodeId, kAggregatorEndpointId); +} + void DeviceManager::AddSyncedDevice(const Device & device) { mSyncedDevices.insert(device); @@ -244,7 +250,6 @@ void DeviceManager::HandleReadSupportedDeviceCategories(TLV::TLVReader & data) if (value.Has(app::Clusters::CommissionerControl::SupportedDeviceCategoryBitmap::kFabricSynchronization)) { ChipLogProgress(NotSpecified, "Remote Fabric-Bridge supports Fabric Synchronization, start reverse commissioning."); - mCommissionerControl.Init(PairingManager::Instance().CurrentCommissioner(), mRemoteBridgeNodeId, kAggregatorEndpointId); RequestCommissioningApproval(); } } diff --git a/examples/fabric-admin/device_manager/DeviceManager.h b/examples/fabric-admin/device_manager/DeviceManager.h index 0b69326d5f5b11..e2c1471aa5892c 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.h +++ b/examples/fabric-admin/device_manager/DeviceManager.h @@ -61,7 +61,7 @@ class DeviceManager : public PairingDelegate void UpdateLastUsedNodeId(chip::NodeId nodeId); - void SetRemoteBridgeNodeId(chip::NodeId nodeId) { mRemoteBridgeNodeId = nodeId; } + void SetRemoteBridgeNodeId(chip::NodeId nodeId); void SetLocalBridgePort(uint16_t port) { mLocalBridgePort = port; } void SetLocalBridgeSetupPinCode(uint32_t pinCode) { mLocalBridgeSetupPinCode = pinCode; } From de828fef0c2822145970094aba05d3e183432b7f Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Wed, 16 Oct 2024 13:15:22 -0700 Subject: [PATCH 5/8] Remove redundant optimization --- .../device_manager/CommissionerControl.cpp | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/examples/fabric-admin/device_manager/CommissionerControl.cpp b/examples/fabric-admin/device_manager/CommissionerControl.cpp index b3180b432d30cb..626bd9e4b904cb 100644 --- a/examples/fabric-admin/device_manager/CommissionerControl.cpp +++ b/examples/fabric-admin/device_manager/CommissionerControl.cpp @@ -28,16 +28,11 @@ CHIP_ERROR CommissionerControl::RequestCommissioningApproval(uint64_t requestId, if (label.HasValue()) { + VerifyOrReturnError(label.Value().size() <= kMaxDeviceLabelLength, CHIP_ERROR_BUFFER_TOO_SMALL); memcpy(mLabelBuffer, label.Value().data(), label.Value().size()); mRequestCommissioningApproval.label = Optional>(CharSpan(mLabelBuffer, label.Value().size())); } - CommissioneeDeviceProxy * commissioneeDeviceProxy = nullptr; - if (CHIP_NO_ERROR == mCommissioner->GetDeviceBeingCommissioned(mDestinationId, &commissioneeDeviceProxy)) - { - return SendCommandForType(CommandType::kRequestCommissioningApproval, commissioneeDeviceProxy); - } - mCommandType = CommandType::kRequestCommissioningApproval; return mCommissioner->GetConnectedDevice(mDestinationId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback); } @@ -51,12 +46,6 @@ CHIP_ERROR CommissionerControl::CommissionNode(uint64_t requestId, uint16_t resp mCommissionNode.requestID = requestId; mCommissionNode.responseTimeoutSeconds = responseTimeoutSeconds; - CommissioneeDeviceProxy * commissioneeDeviceProxy = nullptr; - if (CHIP_NO_ERROR == mCommissioner->GetDeviceBeingCommissioned(mDestinationId, &commissioneeDeviceProxy)) - { - return SendCommandForType(CommandType::kCommissionNode, commissioneeDeviceProxy); - } - mCommandType = CommandType::kCommissionNode; return mCommissioner->GetConnectedDevice(mDestinationId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback); } From 3168f76b1655e8f96436ae7ff6c82e88633e031e Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Wed, 16 Oct 2024 15:05:06 -0700 Subject: [PATCH 6/8] Fix merge conflict --- examples/fabric-admin/device_manager/DeviceManager.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/examples/fabric-admin/device_manager/DeviceManager.h b/examples/fabric-admin/device_manager/DeviceManager.h index 896562f46ec6a3..acccc44d380858 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.h +++ b/examples/fabric-admin/device_manager/DeviceManager.h @@ -211,11 +211,8 @@ class DeviceManager : public PairingDelegate bool mInitialized = false; uint64_t mRequestId = 0; -<<<<<<< HEAD - CommissionerControl mCommissionerControl; -======= BridgeSubscription mBridgeSubscriber; ->>>>>>> upstream/master + CommissionerControl mCommissionerControl; }; /** From 3b9ab97cb7a98dc24dffeb535aa4078eba056781 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Wed, 16 Oct 2024 15:29:25 -0700 Subject: [PATCH 7/8] Call OnDone on failure --- .../device_manager/CommissionerControl.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/examples/fabric-admin/device_manager/CommissionerControl.cpp b/examples/fabric-admin/device_manager/CommissionerControl.cpp index 626bd9e4b904cb..b919ceb0683192 100644 --- a/examples/fabric-admin/device_manager/CommissionerControl.cpp +++ b/examples/fabric-admin/device_manager/CommissionerControl.cpp @@ -125,13 +125,17 @@ void CommissionerControl::OnDeviceConnectedFn(void * context, Messaging::Exchang OperationalDeviceProxy device(&exchangeMgr, sessionHandle); CHIP_ERROR err = self->SendCommandForType(self->mCommandType, &device); - LogErrorOnFailure(err); + if (err != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to send CommissionerControl command."); + self->OnDone(nullptr); + } } void CommissionerControl::OnDeviceConnectionFailureFn(void * context, const ScopedNodeId & peerId, CHIP_ERROR err) { LogErrorOnFailure(err); - CommissionerControl * self = reinterpret_cast(context); - VerifyOrReturn(self != nullptr, ChipLogError(NotSpecified, "OnDeviceConnectionFailureFn: context is null")); + VerifyOrReturn(self != nullptr, ChipLogError(NotSpecified, "OnDeviceConnectedFn: context is null")); + self->OnDone(nullptr); } From 3b330affb548d51944200e5b6d1cb08715c1622a Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Thu, 17 Oct 2024 13:52:54 -0700 Subject: [PATCH 8/8] Fix build error after merge --- examples/fabric-admin/device_manager/DeviceManager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/fabric-admin/device_manager/DeviceManager.cpp b/examples/fabric-admin/device_manager/DeviceManager.cpp index fcd6935fd89a3d..0d7d656a4e685e 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.cpp +++ b/examples/fabric-admin/device_manager/DeviceManager.cpp @@ -419,7 +419,8 @@ void DeviceManager::HandleReverseOpenCommissioningWindow(TLV::TLVReader & data) void DeviceManager::HandleAttributeData(const app::ConcreteDataAttributePath & path, TLV::TLVReader & data) { - if (path.mClusterId == Descriptor::Id && path.mAttributeId == Descriptor::Attributes::PartsList::Id) + if (path.mClusterId == app::Clusters::Descriptor::Id && + path.mAttributeId == app::Clusters::Descriptor::Attributes::PartsList::Id) { HandleAttributePartsListUpdate(data); return;