pw_rpc: Add default and move/assign constructors to ClientCall
This updates the BaseClientCall and NanopbClientCall classes to have
working default and move constructors. This allows client calls to be
declared statically, supporting asynchronous RPC calls.
Change-Id: I6bb153c0ae435708456291fe65d76fd0da26e94c
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/42729
Commit-Queue: Alexei Frolov <frolv@google.com>
Reviewed-by: Keir Mierle <keir@google.com>
diff --git a/pw_protobuf_compiler/proto.gni b/pw_protobuf_compiler/proto.gni
index 33fe5d7..bc5622c 100644
--- a/pw_protobuf_compiler/proto.gni
+++ b/pw_protobuf_compiler/proto.gni
@@ -165,6 +165,7 @@
public_deps = [
":${invoker.base_target}.nanopb",
"$dir_pw_rpc:server",
+ "$dir_pw_rpc/nanopb:client",
"$dir_pw_rpc/nanopb:method_union",
"$dir_pw_third_party/nanopb",
] + invoker.deps
diff --git a/pw_rpc/base_client_call.cc b/pw_rpc/base_client_call.cc
index ec2a3d3..da4ac01 100644
--- a/pw_rpc/base_client_call.cc
+++ b/pw_rpc/base_client_call.cc
@@ -18,6 +18,29 @@
namespace pw::rpc::internal {
+BaseClientCall& BaseClientCall::operator=(BaseClientCall&& other) {
+ // If the current client call is active, it must be unregistered from the
+ // client as it will no longer be alive after assignment.
+ Unregister();
+
+ active_ = other.active_;
+
+ if (other.active()) {
+ // If the call being assigned is active, replace it in the client's list
+ // with a reference to the current object.
+ other.Unregister();
+ other.channel_->client()->RegisterCall(*this);
+ }
+
+ channel_ = other.channel_;
+ service_id_ = other.service_id_;
+ method_id_ = other.method_id_;
+ request_ = std::move(other.request_);
+ handler_ = other.handler_;
+
+ return *this;
+}
+
void BaseClientCall::Cancel() {
if (active()) {
channel_->Send(NewPacket(PacketType::CANCEL_SERVER_STREAM));
diff --git a/pw_rpc/base_client_call_test.cc b/pw_rpc/base_client_call_test.cc
index 7f15146..8ddf5e1 100644
--- a/pw_rpc/base_client_call_test.cc
+++ b/pw_rpc/base_client_call_test.cc
@@ -35,6 +35,26 @@
EXPECT_EQ(context.client().active_calls(), 0u);
}
+TEST(BaseClientCall, Move_UnregistersOriginal) {
+ ClientContextForTest context;
+ EXPECT_EQ(context.client().active_calls(), 0u);
+
+ BaseClientCall moved(&context.channel(),
+ context.service_id(),
+ context.method_id(),
+ [](BaseClientCall&, const Packet&) {});
+ EXPECT_EQ(context.client().active_calls(), 1u);
+
+ BaseClientCall call(std::move(moved));
+ EXPECT_EQ(context.client().active_calls(), 1u);
+
+// Ignore use-after-move.
+#ifndef __clang_analyzer__
+ EXPECT_FALSE(moved.active());
+#endif // __clang_analyzer__
+ EXPECT_TRUE(call.active());
+}
+
class FakeClientCall : public BaseClientCall {
public:
constexpr FakeClientCall(rpc::Channel* channel,
diff --git a/pw_rpc/docs.rst b/pw_rpc/docs.rst
index ae17d5a..9518682 100644
--- a/pw_rpc/docs.rst
+++ b/pw_rpc/docs.rst
@@ -850,6 +850,48 @@
Use ``std::move`` when passing around ``ClientCall`` objects to keep RPCs
alive.
+Example
+^^^^^^^
+.. code-block:: c++
+
+ #include "pw_rpc/echo_service_nanopb.h"
+
+ namespace {
+
+ // RPC response handler for pw.rpc.EchoService/Echo.
+ class EchoResponseHandler
+ : public pw::rpc::UnaryResponseHandler<pw_rpc_EchoMessage> {
+ public:
+ // Callback invoked when a response is received. This is called
+ // synchronously from Client::ProcessPacket.
+ void ReceivedResponse(pw::Status status,
+ const pw_rpc_EchoMessage& response) final {
+ if (status.ok()) {
+ PW_LOG_INFO("Received echo response: %s", response.msg);
+ } else {
+ PW_LOG_ERROR("Echo failed with status %d",
+ static_cast<int>(status.code()));
+ }
+ }
+ };
+
+ pw::rpc::NanopbClientCall<pw::rpc::UnaryResponseHandler<pw_rpc_EchoMessage>>
+ echo_call;
+ EchoResponseHandler response_handler;
+
+ } // namespace
+
+ void CallEcho(const char* message) {
+ pw_rpc_EchoMessage request = pw_rpc_EchoMessage_init_default;
+ pw::string::Copy(message, request.msg);
+
+ // By assigning the returned ClientCall to the global echo_call, 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.
+ echo_call = pw::rpc::nanopb::EchoServiceClient::Echo(
+ my_channel, request, response_handler);
+ }
+
Client implementation details
-----------------------------
diff --git a/pw_rpc/nanopb/public/pw_rpc/nanopb_client_call.h b/pw_rpc/nanopb/public/pw_rpc/nanopb_client_call.h
index 44e8475..368d36f 100644
--- a/pw_rpc/nanopb/public/pw_rpc/nanopb_client_call.h
+++ b/pw_rpc/nanopb/public/pw_rpc/nanopb_client_call.h
@@ -72,6 +72,15 @@
: BaseClientCall(channel, service_id, method_id, handler),
serde_(request_fields, response_fields) {}
+ constexpr BaseNanopbClientCall()
+ : BaseClientCall(), serde_(nullptr, nullptr) {}
+
+ BaseNanopbClientCall(const BaseNanopbClientCall&) = delete;
+ BaseNanopbClientCall& operator=(const BaseNanopbClientCall&) = delete;
+
+ BaseNanopbClientCall(BaseNanopbClientCall&&) = default;
+ BaseNanopbClientCall& operator=(BaseNanopbClientCall&&) = default;
+
constexpr const internal::NanopbMethodSerde& serde() const { return serde_; }
private:
@@ -112,7 +121,15 @@
&ResponseHandler,
request_fields,
response_fields),
- callback_(callback) {}
+ callback_(&callback) {}
+
+ constexpr NanopbClientCall() : BaseNanopbClientCall(), callback_(nullptr) {}
+
+ NanopbClientCall(const NanopbClientCall&) = delete;
+ NanopbClientCall& operator=(const NanopbClientCall&) = delete;
+
+ NanopbClientCall(NanopbClientCall&&) = default;
+ NanopbClientCall& operator=(NanopbClientCall&&) = default;
private:
using Traits = internal::CallbackTraits<Callback>;
@@ -141,18 +158,18 @@
void InvokeUnaryCallback(const internal::Packet& packet) {
if (packet.type() == internal::PacketType::SERVER_ERROR) {
- callback_.RpcError(packet.status());
+ callback_->RpcError(packet.status());
return;
}
ResponseBuffer response_struct{};
if (serde().DecodeResponse(&response_struct, packet.payload())) {
- callback_.ReceivedResponse(
+ callback_->ReceivedResponse(
packet.status(),
*std::launder(reinterpret_cast<Response*>(&response_struct)));
} else {
- callback_.RpcError(Status::DataLoss());
+ callback_->RpcError(Status::DataLoss());
}
Unregister();
@@ -160,26 +177,26 @@
void InvokeServerStreamingCallback(const internal::Packet& packet) {
if (packet.type() == internal::PacketType::SERVER_ERROR) {
- callback_.RpcError(packet.status());
+ callback_->RpcError(packet.status());
return;
}
if (packet.type() == internal::PacketType::SERVER_STREAM_END) {
- callback_.Complete(packet.status());
+ callback_->Complete(packet.status());
return;
}
ResponseBuffer response_struct{};
if (serde().DecodeResponse(&response_struct, packet.payload())) {
- callback_.ReceivedResponse(
+ callback_->ReceivedResponse(
*std::launder(reinterpret_cast<Response*>(&response_struct)));
} else {
- callback_.RpcError(Status::DataLoss());
+ callback_->RpcError(Status::DataLoss());
}
}
- Callback& callback_;
+ Callback* callback_;
};
} // namespace pw::rpc
diff --git a/pw_rpc/public/pw_rpc/internal/base_client_call.h b/pw_rpc/public/pw_rpc/internal/base_client_call.h
index 49cf513..3b4e110 100644
--- a/pw_rpc/public/pw_rpc/internal/base_client_call.h
+++ b/pw_rpc/public/pw_rpc/internal/base_client_call.h
@@ -41,12 +41,21 @@
Register();
}
+ constexpr BaseClientCall()
+ : channel_(nullptr),
+ service_id_(0),
+ method_id_(0),
+ handler_(nullptr),
+ active_(false) {}
+
~BaseClientCall() { Unregister(); }
BaseClientCall(const BaseClientCall&) = delete;
BaseClientCall& operator=(const BaseClientCall&) = delete;
- BaseClientCall(BaseClientCall&& other) { *this = std::move(other); }
+ BaseClientCall(BaseClientCall&& other) : active_(false) {
+ *this = std::move(other);
+ }
BaseClientCall& operator=(BaseClientCall&& other);
constexpr bool active() const { return active_; }
