[Fabric-Sync] Set remote bridge after reverse pair the bridge device (p…

…roject-chip#36509) * Set remote bridge after reverse pair the bridge device * Address review comment
cecille · Nov 18, 2024 · c56d0f0 · c56d0f0
1 parent 83828f5
commit c56d0f0
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 15 deletions.
diff --git a/examples/fabric-sync/admin/DeviceManager.cpp b/examples/fabric-sync/admin/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 SyncedDevice & device)
@@ -131,6 +127,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::OpenLocalBridgeCommissioningWindow(uint32_t iterations, uint16_t commissioningTimeoutSec,
                                                        uint16_t discriminator, const ByteSpan & salt, const ByteSpan & verifier)
 {

diff --git a/examples/fabric-sync/admin/DeviceManager.h b/examples/fabric-sync/admin/DeviceManager.h
@@ -76,6 +76,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 remote bridge device.
      *

diff --git a/examples/fabric-sync/admin/FabricAdmin.cpp b/examples/fabric-sync/admin/FabricAdmin.cpp
@@ -86,13 +86,14 @@ FabricAdmin::CommissionRemoteBridge(Controller::CommissioningWindowPasscodeParam
 
     if (err == CHIP_NO_ERROR)
     {
-        NodeId nodeId = DeviceManager::Instance().GetNextAvailableNodeId();
+        mNodeId = DeviceManager::Instance().GetNextAvailableNodeId();
 
         // 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);
 
-        DeviceManager::Instance().PairRemoteDevice(nodeId, code.c_str());
+        PairingManager::Instance().SetPairingDelegate(this);
+        DeviceManager::Instance().PairRemoteDevice(mNodeId, code.c_str());
     }
     else
     {
@@ -115,6 +116,28 @@ CHIP_ERROR FabricAdmin::KeepActive(ScopedNodeId scopedNodeId, uint32_t stayActiv
     return CHIP_NO_ERROR;
 }
 
+void FabricAdmin::OnCommissioningComplete(NodeId deviceId, CHIP_ERROR err)
+{
+    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;
+}
+
 void FabricAdmin::ScheduleSendingKeepActiveOnCheckIn(ScopedNodeId scopedNodeId, uint32_t stayActiveDurationMs, uint32_t timeoutMs)
 {
     // Accessing mPendingCheckIn should only be done while holding ChipStackLock

diff --git a/examples/fabric-sync/admin/FabricAdmin.h b/examples/fabric-sync/admin/FabricAdmin.h
@@ -37,7 +37,7 @@ struct ScopedNodeIdHasher
     }
 };
 
-class FabricAdmin final : public bridge::FabricAdminDelegate
+class FabricAdmin final : public bridge::FabricAdminDelegate, public PairingDelegate
 {
 public:
     static FabricAdmin & Instance();
@@ -51,6 +51,8 @@ class FabricAdmin final : public bridge::FabricAdminDelegate
 
     CHIP_ERROR KeepActive(chip::ScopedNodeId scopedNodeId, uint32_t stayActiveDurationMs, uint32_t timeoutMs) override;
 
+    void OnCommissioningComplete(chip::NodeId deviceId, CHIP_ERROR err) override;
+
     void ScheduleSendingKeepActiveOnCheckIn(chip::ScopedNodeId scopedNodeId, uint32_t stayActiveDurationMs, uint32_t timeoutMs);
 
 private:
@@ -88,7 +90,8 @@ class FabricAdmin final : public bridge::FabricAdminDelegate
 
     static FabricAdmin sInstance;
 
-    bool mInitialized = false;
+    bool mInitialized    = false;
+    chip::NodeId mNodeId = chip::kUndefinedNodeId;
 
     void Init() { mInitialized = true; }
 };

diff --git a/examples/fabric-sync/admin/PairingManager.cpp b/examples/fabric-sync/admin/PairingManager.cpp
@@ -281,6 +281,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
@@ -294,12 +300,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-sync/shell/AddBridgeCommand.cpp b/examples/fabric-sync/shell/AddBridgeCommand.cpp
@@ -55,6 +55,7 @@ void AddBridgeCommand::OnCommissioningComplete(NodeId deviceId, CHIP_ERROR err)
 
         admin::DeviceManager::Instance().UpdateLastUsedNodeId(mBridgeNodeId);
         admin::DeviceManager::Instance().SubscribeRemoteFabricBridge();
+        admin::DeviceManager::Instance().InitCommissionerControl();
 
         // After successful commissioning of the Commissionee, initiate Reverse Commissioning
         // via the Commissioner Control Cluster. However, we must first verify that the