Remove global RPC call handler for each RPC call (#34338)

* Remove global RPC call handler for each RPC call * Update examples/fabric-admin/rpc/RpcClient.cpp Co-authored-by: Terence Hampson <[email protected]> * Address review comments --------- Co-authored-by: Terence Hampson <[email protected]>
project-chip · Jul 16, 2024 · 1cd70c1 · 1cd70c1
1 parent af976f1
commit 1cd70c1
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 37 deletions.
diff --git a/examples/fabric-admin/rpc/RpcClient.cpp b/examples/fabric-admin/rpc/RpcClient.cpp
@@ -43,14 +43,14 @@ 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;
 
 std::mutex responseMutex;
 std::condition_variable responseCv;
 bool responseReceived    = false;
 CHIP_ERROR responseError = CHIP_NO_ERROR;
 
+// By passing the `call` parameter into WaitForResponse we are explicitly trying to insure the caller takes into consideration that
+// the lifetime of the `call` object when calling WaitForResponse
 template <typename CallType>
 CHIP_ERROR WaitForResponse(CallType & call)
 {
@@ -117,52 +117,38 @@ CHIP_ERROR AddSynchronizedDevice(chip::NodeId nodeId)
 {
     ChipLogProgress(NotSpecified, "AddSynchronizedDevice");
 
-    if (addSynchronizedDeviceCall.active())
-    {
-        ChipLogError(NotSpecified, "Add 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 '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);
+    // 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.
+    auto call = fabricBridgeClient.AddSynchronizedDevice(device, OnAddDeviceResponseCompleted);
 
-    if (!addSynchronizedDeviceCall.active())
+    if (!call.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 WaitForResponse(addSynchronizedDeviceCall);
+    return WaitForResponse(call);
 }
 
 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);
+    // 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.
+    auto call = fabricBridgeClient.RemoveSynchronizedDevice(device, OnRemoveDeviceResponseCompleted);
 
-    if (!removeSynchronizedDeviceCall.active())
+    if (!call.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 WaitForResponse(removeSynchronizedDeviceCall);
+    return WaitForResponse(call);
 }
diff --git a/examples/fabric-bridge-app/linux/RpcClient.cpp b/examples/fabric-bridge-app/linux/RpcClient.cpp
@@ -43,13 +43,14 @@ constexpr uint32_t kDefaultChannelId = 1;
 
 // Fabric Admin Client
 rpc::pw_rpc::nanopb::FabricAdmin::Client fabricAdminClient(rpc::client::GetDefaultRpcClient(), kDefaultChannelId);
-pw::rpc::NanopbUnaryReceiver<::chip_rpc_OperationStatus> openCommissioningWindowCall;
 
 std::mutex responseMutex;
 std::condition_variable responseCv;
 bool responseReceived    = false;
 CHIP_ERROR responseError = CHIP_NO_ERROR;
 
+// By passing the `call` parameter into WaitForResponse we are explicitly trying to insure the caller takes into consideration that
+// the lifetime of the `call` object when calling WaitForResponse
 template <typename CallType>
 CHIP_ERROR WaitForResponse(CallType & call)
 {
@@ -98,23 +99,18 @@ CHIP_ERROR OpenCommissioningWindow(NodeId nodeId)
 {
     ChipLogProgress(NotSpecified, "OpenCommissioningWindow with Node Id 0x:" ChipLogFormatX64, ChipLogValueX64(nodeId));
 
-    if (openCommissioningWindowCall.active())
-    {
-        ChipLogError(NotSpecified, "OpenCommissioningWindow is in progress\n");
-        return CHIP_ERROR_BUSY;
-    }
-
     chip_rpc_DeviceInfo device;
     device.node_id = nodeId;
 
-    // The RPC will remain active as long as `openCommissioningWindowCall` is alive.
-    openCommissioningWindowCall = fabricAdminClient.OpenCommissioningWindow(device, OnOpenCommissioningWindowCompleted);
+    // 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.
+    auto call = fabricAdminClient.OpenCommissioningWindow(device, OnOpenCommissioningWindowCompleted);
 
-    if (!openCommissioningWindowCall.active())
+    if (!call.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 WaitForResponse(openCommissioningWindowCall);
+    return WaitForResponse(call);
 }