Fix failing to cleanup the synced device after it is removed (project-chip#36412)

Author: yufengwangca

examples/fabric-admin/commands/pairing/PairingCommand.cpp

@@ -23,6 +23,7 @@
 #include <commands/interactive/InteractiveCommands.h>
 #include <controller/ExampleOperationalCredentialsIssuer.h>
 #include <crypto/CHIPCryptoPAL.h>
+#include <device_manager/DeviceManager.h>
 #include <device_manager/DeviceSynchronization.h>
 #include <lib/core/CHIPSafeCasts.h>
 #include <lib/support/logging/CHIPLogging.h>
@@ -33,10 +34,6 @@
 
 #include <string>
 
-#if defined(PW_RPC_ENABLED)
-#include <rpc/RpcClient.h>
-#endif
-
 using namespace ::chip;
 using namespace ::chip::Controller;
 
@@ -534,16 +531,16 @@ void PairingCommand::OnCurrentFabricRemove(void * context, NodeId nodeId, CHIP_E
     PairingCommand * command = reinterpret_cast<PairingCommand *>(context);
     VerifyOrReturn(command != nullptr, ChipLogError(NotSpecified, "OnCurrentFabricRemove: context is null"));
 
+    ChipLogProgress(NotSpecified, "PairingCommand::OnCurrentFabricRemove");
+
     if (err == CHIP_NO_ERROR)
     {
         // print to console
         fprintf(stderr, "Device with Node ID: 0x%lx has been successfully removed.\n", nodeId);
 
-#if defined(PW_RPC_ENABLED)
         app::InteractionModelEngine::GetInstance()->ShutdownSubscriptions(command->CurrentCommissioner().GetFabricIndex(), nodeId);
         ScopedNodeId scopedNodeId(nodeId, command->CurrentCommissioner().GetFabricIndex());
-        admin::RemoveSynchronizedDevice(scopedNodeId);
-#endif
+        admin::DeviceMgr().RemoveSyncedDevice(scopedNodeId);
     }
     else
     {

examples/fabric-admin/device_manager/DeviceManager.cpp

@@ -25,6 +25,10 @@
 #include <cstdio>
 #include <string>
 
+#if defined(PW_RPC_ENABLED)
+#include <rpc/RpcClient.h>
+#endif
+
 using namespace chip;
 
 namespace admin {
@@ -109,8 +113,13 @@ Device * DeviceManager::FindDeviceByNode(NodeId nodeId)
     return nullptr;
 }
 
-void DeviceManager::RemoveSyncedDevice(NodeId nodeId)
+void DeviceManager::RemoveSyncedDevice(chip::ScopedNodeId scopedNodeId)
 {
+#if defined(PW_RPC_ENABLED)
+    RemoveSynchronizedDevice(scopedNodeId);
+#endif
+
+    NodeId nodeId   = scopedNodeId.GetNodeId();
     Device * device = FindDeviceByNode(nodeId);
     if (device == nullptr)
     {
@@ -259,6 +268,10 @@ void DeviceManager::HandleReadSupportedDeviceCategories(TLV::TLVReader & data)
         ChipLogProgress(NotSpecified, "Remote Fabric-Bridge supports Fabric Synchronization, start reverse commissioning.");
         RequestCommissioningApproval();
     }
+    else
+    {
+        ChipLogProgress(NotSpecified, "Remote Fabric-Bridge does not support Fabric Synchronization.");
+    }
 }
 
 void DeviceManager::RequestCommissioningApproval()
@@ -406,6 +419,8 @@ void DeviceManager::SendCommissionNodeRequest(uint64_t requestId, uint16_t respo
 
 void DeviceManager::HandleReverseOpenCommissioningWindow(TLV::TLVReader & data)
 {
+    ChipLogProgress(NotSpecified, "Handle ReverseOpenCommissioningWindow command.");
+
     app::Clusters::CommissionerControl::Commands::ReverseOpenCommissioningWindow::DecodableType value;
     CHIP_ERROR error = app::DataModel::Decode(data, value);
 
@@ -459,17 +474,4 @@ void DeviceManager::HandleCommandResponse(const app::ConcreteCommandPath & path,
     }
 }
 
-void DeviceManager::OnDeviceRemoved(NodeId deviceId, CHIP_ERROR err)
-{
-    if (err != CHIP_NO_ERROR)
-    {
-        ChipLogError(NotSpecified, "Failed to remove synced device:(" ChipLogFormatX64 ") with error: %" CHIP_ERROR_FORMAT,
-                     ChipLogValueX64(deviceId), err.Format());
-        return;
-    }
-
-    RemoveSyncedDevice(deviceId);
-    ChipLogProgress(NotSpecified, "Synced device with NodeId:" ChipLogFormatX64 " has been removed.", ChipLogValueX64(deviceId));
-}
-
 } // namespace admin

examples/fabric-admin/device_manager/DeviceManager.h

@@ -50,7 +50,7 @@ class Device
     chip::EndpointId mEndpointId;
 };
 
-class DeviceManager : public PairingDelegate
+class DeviceManager
 {
 public:
     DeviceManager() = default;
@@ -77,7 +77,7 @@ class DeviceManager : public PairingDelegate
 
     void AddSyncedDevice(const Device & device);
 
-    void RemoveSyncedDevice(chip::NodeId nodeId);
+    void RemoveSyncedDevice(chip::ScopedNodeId scopedNodeId);
 
     /**
      * @brief Determines whether a given nodeId corresponds to the "current bridge device," either local or remote.
@@ -177,8 +177,6 @@ class DeviceManager : public PairingDelegate
 private:
     friend DeviceManager & DeviceMgr();
 
-    void OnDeviceRemoved(chip::NodeId deviceId, CHIP_ERROR err) override;
-
     void RequestCommissioningApproval();
 
     void HandleReadSupportedDeviceCategories(chip::TLV::TLVReader & data);

examples/fabric-admin/device_manager/PairingManager.cpp

@@ -23,15 +23,12 @@
 #include <sys/types.h>
 
 #include <commands/common/CHIPCommand.h>
+#include <device_manager/DeviceManager.h>
 #include <device_manager/DeviceSynchronization.h>
 #include <lib/support/logging/CHIPLogging.h>
 #include <setup_payload/ManualSetupPayloadParser.h>
 #include <setup_payload/QRCodeSetupPayloadParser.h>
 
-#if defined(PW_RPC_ENABLED)
-#include <rpc/RpcClient.h>
-#endif
-
 using namespace ::chip;
 using namespace ::chip::Controller;
 
@@ -547,6 +544,8 @@ void PairingManager::OnCurrentFabricRemove(void * context, NodeId nodeId, CHIP_E
     PairingManager * self = reinterpret_cast<PairingManager *>(context);
     VerifyOrReturn(self != nullptr, ChipLogError(NotSpecified, "OnCurrentFabricRemove: context is null"));
 
+    ChipLogProgress(NotSpecified, "PairingManager::OnCurrentFabricRemove");
+
     if (err == CHIP_NO_ERROR)
     {
         // print to console
@@ -558,12 +557,10 @@ void PairingManager::OnCurrentFabricRemove(void * context, NodeId nodeId, CHIP_E
             self->SetPairingDelegate(nullptr);
         }
 
-#if defined(PW_RPC_ENABLED)
         FabricIndex fabricIndex = self->CurrentCommissioner().GetFabricIndex();
         app::InteractionModelEngine::GetInstance()->ShutdownSubscriptions(fabricIndex, nodeId);
         ScopedNodeId scopedNodeId(nodeId, fabricIndex);
-        RemoveSynchronizedDevice(scopedNodeId);
-#endif
+        DeviceMgr().RemoveSyncedDevice(scopedNodeId);
     }
     else
     {

examples/fabric-sync/admin/DeviceManager.cpp

@@ -143,16 +143,4 @@ CHIP_ERROR DeviceManager::UnpairRemoteDevice(NodeId nodeId)
     return CHIP_NO_ERROR;
 }
 
-void DeviceManager::OnDeviceRemoved(NodeId deviceId, CHIP_ERROR err)
-{
-    if (err != CHIP_NO_ERROR)
-    {
-        ChipLogError(NotSpecified, "Failed to remove synced device:(" ChipLogFormatX64 ") with error: %" CHIP_ERROR_FORMAT,
-                     ChipLogValueX64(deviceId), err.Format());
-        return;
-    }
-
-    ChipLogProgress(NotSpecified, "Synced device with NodeId:" ChipLogFormatX64 " has been removed.", ChipLogValueX64(deviceId));
-}
-
 } // namespace admin

examples/fabric-sync/admin/DeviceManager.h

@@ -25,7 +25,7 @@
 
 namespace admin {
 
-class DeviceManager : public PairingDelegate
+class DeviceManager
 {
 public:
     DeviceManager() = default;
@@ -119,8 +119,6 @@ class DeviceManager : public PairingDelegate
 private:
     friend DeviceManager & DeviceMgr();
 
-    void OnDeviceRemoved(chip::NodeId deviceId, CHIP_ERROR err) override;
-
     static DeviceManager sInstance;
 
     chip::NodeId mLastUsedNodeId = 0;

examples/fabric-sync/admin/PairingManager.cpp

@@ -450,6 +450,8 @@ void PairingManager::OnCurrentFabricRemove(void * context, NodeId nodeId, CHIP_E
     PairingManager * self = reinterpret_cast<PairingManager *>(context);
     VerifyOrReturn(self != nullptr, ChipLogError(NotSpecified, "OnCurrentFabricRemove: context is null"));
 
+    ChipLogProgress(NotSpecified, "PairingManager::OnCurrentFabricRemove");
+
     if (err == CHIP_NO_ERROR)
     {
         // print to console