[Fabric-Sync] Add RPC method RemoveSynchronizedDevice (#34121)

* Add RPC method RemoveSynchronizedDevice * Update examples/fabric-admin/rpc/RpcClient.cpp Co-authored-by: Andrei Litvin <[email protected]> * Update examples/fabric-bridge-app/linux/RpcServer.cpp Co-authored-by: Andrei Litvin <[email protected]> * Update comments for RemoveSynchronizedDevice --------- Co-authored-by: Andrei Litvin <[email protected]>
project-chip · Nov 29, 2024 · ca5a148 · ca5a148
1 parent c9a5802
commit ca5a148
Show file tree

Hide file tree

Showing 10 changed files with 98 additions and 9 deletions.
diff --git a/.github/workflows/examples-linux-standalone.yaml b/.github/workflows/examples-linux-standalone.yaml
@@ -202,21 +202,21 @@ jobs:
               run: |
                   ./scripts/run_in_build_env.sh \
                      "./scripts/build/build_examples.py \
-                        --target linux-x64-fabric-admin \
+                        --target linux-x64-fabric-admin-rpc \
                         build"
                   .environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
                     linux debug fabric-admin \
-                    out/linux-x64-fabric-admin/fabric-admin \
+                    out/linux-x64-fabric-admin-rpc/fabric-admin \
                     /tmp/bloat_reports/
             - name: Build example Fabric Bridge App
               run: |
                   ./scripts/run_in_build_env.sh \
                      "./scripts/build/build_examples.py \
-                        --target linux-x64-fabric-bridge \
+                        --target linux-x64-fabric-bridge-no-ble-rpc \
                         build"
                   .environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
                     linux debug fabric-bridge-app \
-                    out/linux-x64-fabric-bridge/fabric-bridge-app \
+                    out/linux-x64-fabric-bridge-no-ble-rpc/fabric-bridge-app \
                     /tmp/bloat_reports/
             - name: Uploading Size Reports
               uses: ./.github/actions/upload-size-reports

diff --git a/examples/common/pigweed/protos/fabric_bridge_service.proto b/examples/common/pigweed/protos/fabric_bridge_service.proto
@@ -11,5 +11,6 @@ message SynchronizedDevice {
 
 service FabricBridge {
   rpc AddSynchronizedDevice(SynchronizedDevice) returns (pw.protobuf.Empty){}
+  rpc RemoveSynchronizedDevice(SynchronizedDevice) returns (pw.protobuf.Empty){}  
 }
 
diff --git a/examples/common/pigweed/rpc_services/FabricBridge.h b/examples/common/pigweed/rpc_services/FabricBridge.h
@@ -38,6 +38,11 @@ class FabricBridge : public pw_rpc::nanopb::FabricBridge::Service<FabricBridge>
     {
         return pw::Status::Unimplemented();
     }
+
+    virtual pw::Status RemoveSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response)
+    {
+        return pw::Status::Unimplemented();
+    }
 };
 
 } // namespace rpc

diff --git a/examples/fabric-admin/BUILD.gn b/examples/fabric-admin/BUILD.gn
@@ -46,8 +46,6 @@ config("config") {
     defines += [ "CONFIG_USE_LOCAL_STORAGE" ]
   }
 
-  cflags = [ "-Wconversion" ]
-
   if (chip_enable_pw_rpc) {
     defines += [ "PW_RPC_ENABLED" ]
   }
@@ -144,6 +142,10 @@ static_library("fabric-admin-utils") {
     ]
 
     deps += pw_build_LINK_DEPS
+  } else {
+    # The system_rpc_server.cc file is in pigweed and doesn't compile with
+    # -Wconversion, remove check for RPC build only.
+    cflags = [ "-Wconversion" ]
   }
 
   if (chip_enable_transport_trace) {

diff --git a/examples/fabric-admin/args.gni b/examples/fabric-admin/args.gni
@@ -32,3 +32,4 @@ matter_log_json_payload_decode_full = true
 # make fabric-admin very strict by default
 chip_tlv_validate_char_string_on_read = true
 chip_tlv_validate_char_string_on_write = true
+chip_enable_ble = true
diff --git a/examples/fabric-admin/rpc/RpcClient.cpp b/examples/fabric-admin/rpc/RpcClient.cpp
@@ -41,6 +41,7 @@ constexpr uint32_t kDefaultChannelId = 1;
 // Fabric Bridge Client
 rpc::pw_rpc::nanopb::FabricBridge::Client fabricBridgeClient(rpc::client::GetDefaultRpcClient(), kDefaultChannelId);
 pw::rpc::NanopbUnaryReceiver<::pw_protobuf_Empty> addSynchronizedDeviceCall;
+pw::rpc::NanopbUnaryReceiver<::pw_protobuf_Empty> removeSynchronizedDeviceCall;
 
 // Callback function to be called when the RPC response is received
 void OnAddDeviceResponseCompleted(const pw_protobuf_Empty & response, pw::Status status)
@@ -55,6 +56,19 @@ void OnAddDeviceResponseCompleted(const pw_protobuf_Empty & response, pw::Status
     }
 }
 
+// Callback function to be called when the RPC response is received
+void OnRemoveDeviceResponseCompleted(const pw_protobuf_Empty & response, pw::Status status)
+{
+    if (status.ok())
+    {
+        ChipLogProgress(NotSpecified, "RemoveSynchronizedDevice RPC call succeeded!");
+    }
+    else
+    {
+        ChipLogProgress(NotSpecified, "RemoveSynchronizedDevice RPC call failed with status: %d", status.code());
+    }
+}
+
 } // namespace
 
 CHIP_ERROR InitRpcClient(uint16_t rpcServerPort)
@@ -76,11 +90,41 @@ CHIP_ERROR AddSynchronizedDevice(chip::NodeId nodeId)
     chip_rpc_SynchronizedDevice device;
     device.node_id = nodeId;
 
-    // The RPC will remain active as long as `addSynchronizedDeviceCall` is alive.
+    // By assigning the returned call to the global 'addSynchronizedDeviceCall', the RPC
+    // call is kept alive until it completes. When a response is received, it
+    // will be logged by the handler function and the call will complete.
     addSynchronizedDeviceCall = fabricBridgeClient.AddSynchronizedDevice(device, OnAddDeviceResponseCompleted);
 
     if (!addSynchronizedDeviceCall.active())
     {
+        // The RPC call was not sent. This could occur due to, for example, an invalid channel ID. Handle if necessary.
+        return CHIP_ERROR_INTERNAL;
+    }
+
+    return CHIP_NO_ERROR;
+}
+
+CHIP_ERROR RemoveSynchronizedDevice(chip::NodeId nodeId)
+{
+    ChipLogProgress(NotSpecified, "RemoveSynchronizedDevice");
+
+    if (removeSynchronizedDeviceCall.active())
+    {
+        ChipLogError(NotSpecified, "Remove Synchronized Device operation is in progress\n");
+        return CHIP_ERROR_BUSY;
+    }
+
+    chip_rpc_SynchronizedDevice device;
+    device.node_id = nodeId;
+
+    // By assigning the returned call to the global 'removeSynchronizedDeviceCall', the RPC
+    // call is kept alive until it completes. When a response is received, it
+    // will be logged by the handler function and the call will complete.
+    removeSynchronizedDeviceCall = fabricBridgeClient.RemoveSynchronizedDevice(device, OnRemoveDeviceResponseCompleted);
+
+    if (!removeSynchronizedDeviceCall.active())
+    {
+        // The RPC call was not sent. This could occur due to, for example, an invalid channel ID. Handle if necessary.
         return CHIP_ERROR_INTERNAL;
     }
 

diff --git a/examples/fabric-admin/rpc/RpcClient.h b/examples/fabric-admin/rpc/RpcClient.h
@@ -46,3 +46,18 @@ CHIP_ERROR InitRpcClient(uint16_t rpcServerPort);
  * - CHIP_ERROR_INTERNAL: An internal error occurred while activating the RPC call.
  */
 CHIP_ERROR AddSynchronizedDevice(chip::NodeId nodeId);
+
+/**
+ * @brief Removes a synchronized device from the RPC client.
+ *
+ * This function attempts to remove a device identified by its `nodeId` from the synchronized device list.
+ * It logs the progress and checks if a `RemoveSynchronizedDevice` operation is already in progress.
+ * If an operation is in progress, it returns `CHIP_ERROR_BUSY`.
+ *
+ * @param nodeId The Node ID of the device to be removed.
+ * @return CHIP_ERROR An error code indicating the success or failure of the operation.
+ * - CHIP_NO_ERROR: The RPC command was successfully sent.
+ * - CHIP_ERROR_BUSY: Another operation is currently in progress.
+ * - CHIP_ERROR_INTERNAL: An internal error occurred while activating the RPC call.
+ */
+CHIP_ERROR RemoveSynchronizedDevice(chip::NodeId nodeId);
diff --git a/examples/fabric-bridge-app/linux/BUILD.gn b/examples/fabric-bridge-app/linux/BUILD.gn
@@ -46,8 +46,6 @@ executable("fabric-bridge-app") {
     "${chip_root}/src/lib",
   ]
 
-  cflags = [ "-Wconversion" ]
-
   include_dirs = [ "include" ]
 
   if (bridge_enable_pw_rpc) {
@@ -87,6 +85,10 @@ executable("fabric-bridge-app") {
       "${chip_root}/examples/common",
       "${chip_root}/examples/platform/linux",
     ]
+  } else {
+    # The system_rpc_server.cc file is in pigweed and doesn't compile with
+    # -Wconversion, remove check for RPC build only.
+    cflags = [ "-Wconversion" ]
   }
 
   output_dir = root_out_dir

diff --git a/examples/fabric-bridge-app/linux/RpcServer.cpp b/examples/fabric-bridge-app/linux/RpcServer.cpp
@@ -43,6 +43,7 @@ class FabricBridge final : public chip::rpc::FabricBridge
 {
 public:
     pw::Status AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) override;
+    pw::Status RemoveSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) override;
 };
 
 pw::Status FabricBridge::AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response)
@@ -64,6 +65,21 @@ pw::Status FabricBridge::AddSynchronizedDevice(const chip_rpc_SynchronizedDevice
     return pw::OkStatus();
 }
 
+pw::Status FabricBridge::RemoveSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response)
+{
+    NodeId nodeId = request.node_id;
+    ChipLogProgress(NotSpecified, "Received RemoveSynchronizedDevice: " ChipLogFormatX64, ChipLogValueX64(nodeId));
+
+    int removed_idx = DeviceMgr().RemoveDeviceByNodeId(nodeId);
+    if (removed_idx < 0)
+    {
+        ChipLogError(NotSpecified, "Failed to remove device with nodeId=0x" ChipLogFormatX64, ChipLogValueX64(nodeId));
+        return pw::Status::NotFound();
+    }
+
+    return pw::OkStatus();
+}
+
 FabricBridge fabric_bridge_service;
 #endif // defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE
 

diff --git a/examples/fabric-bridge-app/linux/main.cpp b/examples/fabric-bridge-app/linux/main.cpp
@@ -47,7 +47,10 @@ using namespace chip::app::Clusters::AdministratorCommissioning;
 namespace {
 
 constexpr uint16_t kPollIntervalMs = 100;
+
+#if defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE
 constexpr uint16_t kRetryIntervalS = 3;
+#endif
 
 bool KeyboardHit()
 {