From 1cd70c14b51c0bde628f856f920a29f41f91686b Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Tue, 16 Jul 2024 12:30:50 -0700 Subject: [PATCH] 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 * Address review comments --------- Co-authored-by: Terence Hampson --- examples/fabric-admin/rpc/RpcClient.cpp | 38 ++++++------------- .../fabric-bridge-app/linux/RpcClient.cpp | 18 ++++----- 2 files changed, 19 insertions(+), 37 deletions(-) diff --git a/examples/fabric-admin/rpc/RpcClient.cpp b/examples/fabric-admin/rpc/RpcClient.cpp index 40d2e8ba09aa2e..00ca1204cc1f19 100644 --- 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 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 index c26d2b8f0aad31..02916b87aaa06a 100644 --- 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 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); }