pw_rpc: Rename BaseServerWriter to Responder The pw::rpc::internal::Responder class will be used to respond to all types of RPCs, not just server streaming RPCs, so the name "BaseServerWriter" is not a good fit. Change-Id: I72d1475f26896c76f0b0dd968b0aca65d0c618a4 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/49200 Pigweed-Auto-Submit: Wyatt Hepler <hepler@google.com> Reviewed-by: Alexei Frolov <frolv@google.com> Commit-Queue: Auto-Submit <auto-submit@pigweed.google.com.iam.gserviceaccount.com>
diff --git a/pw_rpc/base_server_writer.cc b/pw_rpc/base_server_writer.cc index 64d73bc..d1a51b7 100644 --- a/pw_rpc/base_server_writer.cc +++ b/pw_rpc/base_server_writer.cc
@@ -21,21 +21,20 @@ namespace pw::rpc::internal { -BaseServerWriter::BaseServerWriter(ServerCall& call) - : call_(call), state_(kOpen) { - call_.server().RegisterWriter(*this); +Responder::Responder(ServerCall& call) : call_(call), state_(kOpen) { + call_.server().RegisterResponder(*this); } -BaseServerWriter& BaseServerWriter::operator=(BaseServerWriter&& other) { +Responder& Responder::operator=(Responder&& other) { Finish(); state_ = other.state_; if (other.open()) { - other.call_.server().RemoveWriter(other); + other.call_.server().RemoveResponder(other); other.state_ = kClosed; - other.call_.server().RegisterWriter(*this); + other.call_.server().RegisterResponder(*this); } call_ = std::move(other.call_); @@ -44,14 +43,14 @@ return *this; } -uint32_t BaseServerWriter::method_id() const { return call_.method().id(); } +uint32_t Responder::method_id() const { return call_.method().id(); } -Status BaseServerWriter::Finish(Status status) { +Status Responder::Finish(Status status) { if (!open()) { return Status::FailedPrecondition(); } - // If the ServerWriter implementer or user forgets to release an acquired + // If the Responder implementer or user forgets to release an acquired // buffer before finishing, release it here. if (!response_.empty()) { ReleasePayloadBuffer(); @@ -68,7 +67,7 @@ status)); } -std::span<std::byte> BaseServerWriter::AcquirePayloadBuffer() { +std::span<std::byte> Responder::AcquirePayloadBuffer() { PW_DCHECK(open()); // Only allow having one active buffer at a time. @@ -79,29 +78,27 @@ return response_.payload(ResponsePacket()); } -Status BaseServerWriter::ReleasePayloadBuffer( - std::span<const std::byte> payload) { +Status Responder::ReleasePayloadBuffer(std::span<const std::byte> payload) { PW_DCHECK(open()); return call_.channel().Send(response_, ResponsePacket(payload)); } -Status BaseServerWriter::ReleasePayloadBuffer() { +Status Responder::ReleasePayloadBuffer() { PW_DCHECK(open()); call_.channel().Release(response_); return OkStatus(); } -void BaseServerWriter::Close() { +void Responder::Close() { if (!open()) { return; } - call_.server().RemoveWriter(*this); + call_.server().RemoveResponder(*this); state_ = kClosed; } -Packet BaseServerWriter::ResponsePacket( - std::span<const std::byte> payload) const { +Packet Responder::ResponsePacket(std::span<const std::byte> payload) const { return Packet(PacketType::RESPONSE, call_.channel().id(), call_.service().id(),
diff --git a/pw_rpc/base_server_writer_test.cc b/pw_rpc/base_server_writer_test.cc index f2d9d27..d9812ba 100644 --- a/pw_rpc/base_server_writer_test.cc +++ b/pw_rpc/base_server_writer_test.cc
@@ -39,19 +39,19 @@ using std::byte; -TEST(BaseServerWriter, ConstructWithContext_StartsOpen) { +TEST(Responder, ConstructWithContext_StartsOpen) { ServerContextForTest<TestService> context(TestService::method.method()); - BaseServerWriter writer(context.get()); + Responder writer(context.get()); EXPECT_TRUE(writer.open()); } -TEST(BaseServerWriter, Move_ClosesOriginal) { +TEST(Responder, Move_ClosesOriginal) { ServerContextForTest<TestService> context(TestService::method.method()); - BaseServerWriter moved(context.get()); - BaseServerWriter writer(std::move(moved)); + Responder moved(context.get()); + Responder writer(std::move(moved)); #ifndef __clang_analyzer__ EXPECT_FALSE(moved.open()); @@ -59,9 +59,9 @@ EXPECT_TRUE(writer.open()); } -class FakeServerWriter : public BaseServerWriter { +class FakeServerWriter : public Responder { public: - FakeServerWriter(ServerCall& context) : BaseServerWriter(context) {} + FakeServerWriter(ServerCall& context) : Responder(context) {} constexpr FakeServerWriter() = default;
diff --git a/pw_rpc/nanopb/nanopb_method.cc b/pw_rpc/nanopb/nanopb_method.cc index c86837d..e5937cc 100644 --- a/pw_rpc/nanopb/nanopb_method.cc +++ b/pw_rpc/nanopb/nanopb_method.cc
@@ -42,7 +42,7 @@ return; } - internal::BaseServerWriter server_writer(call); + internal::Responder server_writer(call); function_.server_streaming(call, request_struct, server_writer); }
diff --git a/pw_rpc/nanopb/public/pw_rpc/internal/nanopb_method.h b/pw_rpc/nanopb/public/pw_rpc/internal/nanopb_method.h index 6944e4a..80b5864 100644 --- a/pw_rpc/nanopb/public/pw_rpc/internal/nanopb_method.h +++ b/pw_rpc/nanopb/public/pw_rpc/internal/nanopb_method.h
@@ -32,7 +32,7 @@ // Define the Nanopb version of the the ServerWriter class. template <typename T> -class ServerWriter : public internal::BaseServerWriter { +class ServerWriter : public internal::Responder { public: // Allow default construction so that users can declare a variable into which // to move ServerWriters from RPC calls. @@ -159,11 +159,11 @@ NanopbMessageDescriptor request, NanopbMessageDescriptor response) { // Define a wrapper around the user-defined function that takes the request - // struct as void* and a BaseServerWriter instead of the templated - // ServerWriter class. This wrapper is stored generically in the Function - // union, defined below. + // struct as void* and a Responder instead of the templated ServerWriter + // class. This wrapper is stored generically in the Function union, defined + // below. constexpr ServerStreamingFunction wrapper = - [](ServerCall& call, const void* req, BaseServerWriter& writer) { + [](ServerCall& call, const void* req, Responder& writer) { return CallMethodImplFunction<method>( call, *static_cast<const Request<method>*>(req), @@ -210,7 +210,7 @@ // using ServerStreamingFunction = void (*)(ServerCall&, const void* request, - BaseServerWriter& writer); + Responder& writer); // The Function union stores a pointer to a generic version of the // user-defined RPC function. Using a union instead of void* avoids
diff --git a/pw_rpc/nanopb/public/pw_rpc/nanopb_test_method_context.h b/pw_rpc/nanopb/public/pw_rpc/nanopb_test_method_context.h index 4efd0c9..b65ea22 100644 --- a/pw_rpc/nanopb/public/pw_rpc/nanopb_test_method_context.h +++ b/pw_rpc/nanopb/public/pw_rpc/nanopb_test_method_context.h
@@ -210,7 +210,7 @@ // Invokes the RPC with the provided request. void call(const Request& request) { ctx_.output.clear(); - internal::BaseServerWriter server_writer(ctx_.call); + internal::Responder server_writer(ctx_.call); return CallMethodImplFunction<method>( ctx_.call, request, @@ -221,7 +221,7 @@ // This should not be called alongside call(); use one or the other. ServerWriter<Response> writer() { ctx_.output.clear(); - internal::BaseServerWriter server_writer(ctx_.call); + internal::Responder server_writer(ctx_.call); return std::move(static_cast<ServerWriter<Response>&>(server_writer)); }
diff --git a/pw_rpc/public/pw_rpc/internal/base_server_writer.h b/pw_rpc/public/pw_rpc/internal/base_server_writer.h index 895c1b8..26269d9 100644 --- a/pw_rpc/public/pw_rpc/internal/base_server_writer.h +++ b/pw_rpc/public/pw_rpc/internal/base_server_writer.h
@@ -32,37 +32,35 @@ class Packet; -// Internal ServerWriter base class. ServerWriters are used to stream responses. -// Implementations must provide a derived class that provides the interface for -// sending responses. -class BaseServerWriter : public IntrusiveList<BaseServerWriter>::Item { +// Internal RPC Responder class. The Responder is used to respond to any type of +// RPC. Public classes like ServerWriters inherit from it and provide a public +// API for their use case. +class Responder : public IntrusiveList<Responder>::Item { public: - BaseServerWriter(ServerCall& call); + Responder(ServerCall& call); - BaseServerWriter(const BaseServerWriter&) = delete; + Responder(const Responder&) = delete; - BaseServerWriter(BaseServerWriter&& other) : state_(kClosed) { - *this = std::move(other); - } + Responder(Responder&& other) : state_(kClosed) { *this = std::move(other); } - ~BaseServerWriter() { Finish(); } + ~Responder() { Finish(); } - BaseServerWriter& operator=(const BaseServerWriter&) = delete; + Responder& operator=(const Responder&) = delete; - BaseServerWriter& operator=(BaseServerWriter&& other); + Responder& operator=(Responder&& other); - // True if the ServerWriter is active and ready to send responses. + // True if the Responder is active and ready to send responses. bool open() const { return state_ == kOpen; } uint32_t channel_id() const { return call_.channel().id(); } uint32_t service_id() const { return call_.service().id(); } uint32_t method_id() const; - // Closes the ServerWriter, if it is open. + // Closes the Responder, if it is open. Status Finish(Status status = OkStatus()); protected: - constexpr BaseServerWriter() : state_{kClosed} {} + constexpr Responder() : state_{kClosed} {} const Method& method() const { return call_.method(); } @@ -70,12 +68,12 @@ constexpr const Channel::OutputBuffer& buffer() const { return response_; } - // Acquires a buffer into which to write a payload. The BaseServerWriter MUST - // be open when this is called! + // Acquires a buffer into which to write a payload. The Responder MUST be open + // when this is called! std::span<std::byte> AcquirePayloadBuffer(); // Releases the buffer, sending a packet with the specified payload. The - // BaseServerWriter MUST be open when this is called! + // Responder MUST be open when this is called! Status ReleasePayloadBuffer(std::span<const std::byte> payload); // Releases the buffer without sending a packet.
diff --git a/pw_rpc/public/pw_rpc/internal/server.h b/pw_rpc/public/pw_rpc/internal/server.h index e260636..14c285d 100644 --- a/pw_rpc/public/pw_rpc/internal/server.h +++ b/pw_rpc/public/pw_rpc/internal/server.h
@@ -21,13 +21,9 @@ public: Server() = delete; - void RegisterWriter(BaseServerWriter& writer) { - writers().push_front(writer); - } + void RegisterResponder(Responder& writer) { writers().push_front(writer); } - void RemoveWriter(const BaseServerWriter& writer) { - writers().remove(writer); - } + void RemoveResponder(const Responder& writer) { writers().remove(writer); } }; } // namespace pw::rpc::internal
diff --git a/pw_rpc/public/pw_rpc/server.h b/pw_rpc/public/pw_rpc/server.h index 51e3f36..b57e1cb 100644 --- a/pw_rpc/public/pw_rpc/server.h +++ b/pw_rpc/public/pw_rpc/server.h
@@ -53,7 +53,7 @@ constexpr size_t channel_count() const { return channels_.size(); } protected: - IntrusiveList<internal::BaseServerWriter>& writers() { return writers_; } + IntrusiveList<internal::Responder>& writers() { return writers_; } private: std::tuple<Service*, const internal::Method*> FindMethod( @@ -68,7 +68,7 @@ std::span<internal::Channel> channels_; IntrusiveList<Service> services_; - IntrusiveList<internal::BaseServerWriter> writers_; + IntrusiveList<internal::Responder> writers_; }; } // namespace pw::rpc
diff --git a/pw_rpc/raw/public/pw_rpc/internal/raw_method.h b/pw_rpc/raw/public/pw_rpc/internal/raw_method.h index 6d2e898..768238d 100644 --- a/pw_rpc/raw/public/pw_rpc/internal/raw_method.h +++ b/pw_rpc/raw/public/pw_rpc/internal/raw_method.h
@@ -21,7 +21,7 @@ namespace pw::rpc { -class RawServerWriter : public internal::BaseServerWriter { +class RawServerWriter : public internal::Responder { public: RawServerWriter() = default; RawServerWriter(RawServerWriter&&) = default; @@ -65,7 +65,7 @@ template <auto method> static constexpr RawMethod ServerStreaming(uint32_t id) { constexpr ServerStreamingFunction wrapper = - [](ServerCall& call, ConstByteSpan request, BaseServerWriter& writer) { + [](ServerCall& call, ConstByteSpan request, Responder& writer) { CallMethodImplFunction<method>( call, request, static_cast<RawServerWriter&>(writer)); }; @@ -83,7 +83,7 @@ using ServerStreamingFunction = void (*)(ServerCall&, ConstByteSpan, - BaseServerWriter&); + Responder&); union Function { UnaryFunction unary; ServerStreamingFunction server_streaming;
diff --git a/pw_rpc/raw/public/pw_rpc/raw_test_method_context.h b/pw_rpc/raw/public/pw_rpc/raw_test_method_context.h index 0c005fd..b9fc660 100644 --- a/pw_rpc/raw/public/pw_rpc/raw_test_method_context.h +++ b/pw_rpc/raw/public/pw_rpc/raw_test_method_context.h
@@ -216,7 +216,7 @@ // Invokes the RPC with the provided request. void call(ConstByteSpan request) { ctx_.output.clear(); - BaseServerWriter server_writer(ctx_.call); + Responder server_writer(ctx_.call); return CallMethodImplFunction<method>( ctx_.call, request, static_cast<RawServerWriter&>(server_writer)); } @@ -225,7 +225,7 @@ // This should not be called alongside call(); use one or the other. RawServerWriter writer() { ctx_.output.clear(); - BaseServerWriter server_writer(ctx_.call); + Responder server_writer(ctx_.call); return std::move(static_cast<RawServerWriter&>(server_writer)); }
diff --git a/pw_rpc/raw/raw_method.cc b/pw_rpc/raw/raw_method.cc index 7fe7f80..761b3a3 100644 --- a/pw_rpc/raw/raw_method.cc +++ b/pw_rpc/raw/raw_method.cc
@@ -69,7 +69,7 @@ void RawMethod::CallServerStreaming(ServerCall& call, const Packet& request) const { - internal::BaseServerWriter server_writer(call); + internal::Responder server_writer(call); function_.server_streaming(call, request.payload(), server_writer); }
diff --git a/pw_rpc/server_test.cc b/pw_rpc/server_test.cc index 541000b..8894b5a 100644 --- a/pw_rpc/server_test.cc +++ b/pw_rpc/server_test.cc
@@ -236,7 +236,7 @@ } internal::ServerCall call_; - internal::BaseServerWriter writer_; + internal::Responder writer_; }; TEST_F(MethodPending, ProcessPacket_Cancel_ClosesServerWriter) {