From 67af91bc7c76fd1d6fc7c28bc2b6dcbf66f6b8c5 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Sun, 10 Nov 2024 19:49:08 -0800 Subject: [PATCH] [Fabric-Admin] Set remote bridge after reverse pair the bridge devide --- .../fabric-sync/FabricSyncCommand.cpp | 1 + .../device_manager/DeviceManager.cpp | 18 +++++++---- .../device_manager/DeviceManager.h | 7 +++-- .../device_manager/DeviceSynchronization.cpp | 4 ++- .../device_manager/PairingManager.cpp | 12 ++++---- examples/fabric-admin/rpc/RpcServer.cpp | 30 +++++++++++++++++-- 6 files changed, 56 insertions(+), 16 deletions(-) diff --git a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp index 315bf2f3711f35..8409825587d962 100644 --- a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp +++ b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp @@ -57,6 +57,7 @@ void FabricSyncAddBridgeCommand::OnCommissioningComplete(NodeId deviceId, CHIP_E DeviceManager::Instance().UpdateLastUsedNodeId(mBridgeNodeId); DeviceManager::Instance().SubscribeRemoteFabricBridge(); + DeviceManager::Instance().InitCommissionerControl(); if (DeviceManager::Instance().IsLocalBridgeReady()) { diff --git a/examples/fabric-admin/device_manager/DeviceManager.cpp b/examples/fabric-admin/device_manager/DeviceManager.cpp index 9f40f92a87c411..988d8813dd918e 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.cpp +++ b/examples/fabric-admin/device_manager/DeviceManager.cpp @@ -72,11 +72,7 @@ void DeviceManager::UpdateLastUsedNodeId(NodeId nodeId) void DeviceManager::SetRemoteBridgeNodeId(chip::NodeId nodeId) { mRemoteBridgeNodeId = nodeId; - - if (mRemoteBridgeNodeId != kUndefinedNodeId) - { - mCommissionerControl.Init(PairingManager::Instance().CurrentCommissioner(), mRemoteBridgeNodeId, kAggregatorEndpointId); - } + ChipLogProgress(NotSpecified, "Set remote bridge NodeId:" ChipLogFormatX64, ChipLogValueX64(mRemoteBridgeNodeId)); } void DeviceManager::AddSyncedDevice(const Device & device) @@ -129,6 +125,18 @@ void DeviceManager::RemoveSyncedDevice(chip::ScopedNodeId scopedNodeId) ChipLogValueX64(device->GetNodeId()), device->GetEndpointId()); } +void DeviceManager::InitCommissionerControl() +{ + if (mRemoteBridgeNodeId != kUndefinedNodeId) + { + mCommissionerControl.Init(PairingManager::Instance().CurrentCommissioner(), mRemoteBridgeNodeId, kAggregatorEndpointId); + } + else + { + ChipLogError(NotSpecified, "Failed to initialize the Commissioner Control delegate"); + } +} + void DeviceManager::OpenDeviceCommissioningWindow(ScopedNodeId scopedNodeId, uint32_t iterations, uint16_t commissioningTimeoutSec, uint16_t discriminator, const ByteSpan & salt, const ByteSpan & verifier) { diff --git a/examples/fabric-admin/device_manager/DeviceManager.h b/examples/fabric-admin/device_manager/DeviceManager.h index 594fcfc5745703..f59ad286bd7a65 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.h +++ b/examples/fabric-admin/device_manager/DeviceManager.h @@ -85,6 +85,11 @@ class DeviceManager void RemoveSyncedDevice(chip::ScopedNodeId scopedNodeId); + /** + * @brief Initializes the CommissionerControl for fabric sync setup process. + */ + void InitCommissionerControl(); + /** * @brief Determines whether a given nodeId corresponds to the "current bridge device," either local or remote. * @@ -193,8 +198,6 @@ class DeviceManager void HandleReverseOpenCommissioningWindow(chip::TLV::TLVReader & data); - static DeviceManager sInstance; - chip::NodeId mLastUsedNodeId = 0; // The Node ID of the remote bridge used for Fabric-Sync diff --git a/examples/fabric-admin/device_manager/DeviceSynchronization.cpp b/examples/fabric-admin/device_manager/DeviceSynchronization.cpp index 092c2505d19d73..600129d65ab07c 100644 --- a/examples/fabric-admin/device_manager/DeviceSynchronization.cpp +++ b/examples/fabric-admin/device_manager/DeviceSynchronization.cpp @@ -148,7 +148,8 @@ void DeviceSynchronizer::OnDone(app::ReadClient * apReadClient) GetUniqueId(); if (mState == State::GettingUid) { - // GetUniqueId was successful and we rely on callback to call SynchronizationCompleteAddDevice. + ChipLogProgress(NotSpecified, + "GetUniqueId was successful and we rely on callback to call SynchronizationCompleteAddDevice."); return; } SynchronizationCompleteAddDevice(); @@ -272,6 +273,7 @@ void DeviceSynchronizer::GetUniqueId() void DeviceSynchronizer::SynchronizationCompleteAddDevice() { VerifyOrDie(mState == State::ReceivedResponse || mState == State::GettingUid); + ChipLogProgress(NotSpecified, "Synchronization complete and add device"); #if defined(PW_RPC_ENABLED) AddSynchronizedDevice(mCurrentDeviceData); diff --git a/examples/fabric-admin/device_manager/PairingManager.cpp b/examples/fabric-admin/device_manager/PairingManager.cpp index 8474d78f023343..8d985ac553f010 100644 --- a/examples/fabric-admin/device_manager/PairingManager.cpp +++ b/examples/fabric-admin/device_manager/PairingManager.cpp @@ -285,6 +285,12 @@ void PairingManager::OnPairingDeleted(CHIP_ERROR err) void PairingManager::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err) { + if (mPairingDelegate) + { + mPairingDelegate->OnCommissioningComplete(nodeId, err); + SetPairingDelegate(nullptr); + } + if (err == CHIP_NO_ERROR) { // print to console @@ -308,12 +314,6 @@ void PairingManager::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err) } ChipLogProgress(NotSpecified, "Device commissioning Failure: %s", ErrorStr(err)); } - - if (mPairingDelegate) - { - mPairingDelegate->OnCommissioningComplete(nodeId, err); - SetPairingDelegate(nullptr); - } } void PairingManager::OnReadCommissioningInfo(const Controller::ReadCommissioningInfo & info) diff --git a/examples/fabric-admin/rpc/RpcServer.cpp b/examples/fabric-admin/rpc/RpcServer.cpp index 05fb9492248131..f2ff1cf64b84c1 100644 --- a/examples/fabric-admin/rpc/RpcServer.cpp +++ b/examples/fabric-admin/rpc/RpcServer.cpp @@ -55,7 +55,7 @@ struct ScopedNodeIdHasher } }; -class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate +class FabricAdmin final : public rpc::FabricAdmin, public admin::PairingDelegate, public IcdManager::Delegate { public: void OnCheckInCompleted(const app::ICDClientInfo & clientInfo) override @@ -96,6 +96,29 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate } } + void OnCommissioningComplete(NodeId deviceId, CHIP_ERROR err) override + { + if (mNodeId != deviceId) + { + ChipLogError(NotSpecified, + "Tried to pair a non-bridge device (0x:" ChipLogFormatX64 ") with result: %" CHIP_ERROR_FORMAT, + ChipLogValueX64(deviceId), err.Format()); + return; + } + + if (err == CHIP_NO_ERROR) + { + DeviceManager::Instance().SetRemoteBridgeNodeId(deviceId); + } + else + { + ChipLogError(NotSpecified, "Failed to pair bridge device (0x:" ChipLogFormatX64 ") with error: %" CHIP_ERROR_FORMAT, + ChipLogValueX64(deviceId), err.Format()); + } + + mNodeId = kUndefinedNodeId; + } + pw::Status OpenCommissioningWindow(const chip_rpc_DeviceCommissioningWindowInfo & request, chip_rpc_OperationStatus & response) override { @@ -147,11 +170,12 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate if (error == CHIP_NO_ERROR) { NodeId nodeId = DeviceManager::Instance().GetNextAvailableNodeId(); + mNodeId = nodeId; // After responding with RequestCommissioningApproval to the node where the client initiated the // RequestCommissioningApproval, you need to wait for it to open a commissioning window on its bridge. usleep(kCommissionPrepareTimeMs * 1000); - + PairingManager::Instance().SetPairingDelegate(this); DeviceManager::Instance().PairRemoteDevice(nodeId, code.c_str()); } else @@ -224,6 +248,8 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate Platform::Delete(data); } + NodeId mNodeId = chip::kUndefinedNodeId; + // Modifications to mPendingCheckIn should be done on the MatterEventLoop thread // otherwise we would need a mutex protecting this data to prevent race as this // data is accessible by both RPC thread and Matter eventloop.