pw_rpc: Remove deprecated methods and aliases

- Removes Call::open in favor of Call::active. - Removes the channel API from generated nanopb clients. - Removes the `generated` namespace alias for service base classes. These are all done in a single change to simplify migrating downstream. Change-Id: Ib867b91a08f4f5b5f2462c9c514ee4bb33eba18e Requires: pigweed:70882 Requires: pigweed:70900 Requires: pigweed-internal:18423 Requires: pigweed-internal:18462 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/70780 Commit-Queue: Alexei Frolov <[email protected]> Reviewed-by: Wyatt Hepler <[email protected]> Reviewed-by: Ewout van Bekkum <[email protected]>
X-oss-byte · Nov 30, 2021 · 39d8c5c · 39d8c5c
1 parent 4a555a4
commit 39d8c5c
Show file tree

Hide file tree

Showing 27 changed files with 76 additions and 140 deletions.
diff --git a/pw_file/public/pw_file/flat_file_system.h b/pw_file/public/pw_file/flat_file_system.h
@@ -33,7 +33,7 @@ namespace pw::file {
 // was "flat" (i.e. no directories). This means there's no concept of logical
 // directories, despite any "path like" naming that may be employed by a user.
 class FlatFileSystemService
-    : public generated::FileSystem<FlatFileSystemService> {
+    : public pw_rpc::raw::FileSystem::Service<FlatFileSystemService> {
  public:
   class Entry {
    public:

diff --git a/pw_log_rpc/public/pw_log_rpc/log_service.h b/pw_log_rpc/public/pw_log_rpc/log_service.h
@@ -24,7 +24,7 @@ namespace pw::log_rpc {
 // The RPC LogService provides a way to start a log stream on a known RPC
 // channel with a writer provided on a call. Log streams maintenance is flexible
 // and delegated outside the service.
-class LogService final : public log::generated::Logs<LogService> {
+class LogService final : public log::pw_rpc::raw::Logs::Service<LogService> {
  public:
   LogService(RpcLogDrainMap& drains, FilterMap* filters = nullptr)
       : drains_(drains), filters_(filters) {}

diff --git a/pw_log_rpc/rpc_log_drain_test.cc b/pw_log_rpc/rpc_log_drain_test.cc
@@ -54,7 +54,7 @@ TEST(RpcLogDrain, TryFlushDrainWithClosedWriter) {
   EXPECT_EQ(drain.Flush(), Status::Unavailable());
 
   rpc::RawServerWriter writer;
-  ASSERT_FALSE(writer.open());
+  ASSERT_FALSE(writer.active());
   EXPECT_EQ(drain.Open(writer), Status::FailedPrecondition());
   EXPECT_EQ(drain.Flush(), Status::Unavailable());
 }
@@ -122,14 +122,14 @@ TEST(RpcLogDrain, FlushingDrainWithOpenWriter) {
   rpc::RawServerWriter writer =
       rpc::RawServerWriter::Open<log::pw_rpc::raw::Logs::Listen>(
           server, drain_id, log_service);
-  ASSERT_TRUE(writer.open());
+  ASSERT_TRUE(writer.active());
   EXPECT_EQ(drain.Open(writer), OkStatus());
   EXPECT_EQ(drain.Flush(), OkStatus());
   // Can call multliple times until closed on error.
   EXPECT_EQ(drain.Flush(), OkStatus());
   EXPECT_EQ(drain.Close(), OkStatus());
   rpc::RawServerWriter& writer_ref = writer;
-  ASSERT_FALSE(writer_ref.open());
+  ASSERT_FALSE(writer_ref.active());
   EXPECT_EQ(drain.Flush(), Status::Unavailable());
 }
 
@@ -155,14 +155,14 @@ TEST(RpcLogDrain, TryReopenOpenedDrain) {
   rpc::RawServerWriter writer =
       rpc::RawServerWriter::Open<log::pw_rpc::raw::Logs::Listen>(
           server, drain_id, log_service);
-  ASSERT_TRUE(writer.open());
+  ASSERT_TRUE(writer.active());
   RpcLogDrain& drain = drains[0];
   EXPECT_EQ(drain.Open(writer), OkStatus());
   rpc::RawServerWriter second_writer =
       rpc::RawServerWriter::Open<log::pw_rpc::raw::Logs::Listen>(
           server, drain_id, log_service);
-  ASSERT_FALSE(writer.open());
-  ASSERT_TRUE(second_writer.open());
+  ASSERT_FALSE(writer.active());
+  ASSERT_TRUE(second_writer.active());
   EXPECT_EQ(drain.Open(second_writer), OkStatus());
 }
 

diff --git a/pw_metric/public/pw_metric/metric_service_nanopb.h b/pw_metric/public/pw_metric/metric_service_nanopb.h
@@ -31,7 +31,8 @@ namespace pw::metric {
 // method is blocking, and sends all metrics at once (though batched). In the
 // future, we may switch to offering an async version where the Get() method
 // returns immediately, and someone else is responsible for pumping the queue.
-class MetricService final : public generated::MetricService<MetricService> {
+class MetricService final
+    : public pw_rpc::nanopb::MetricService::Service<MetricService> {
  public:
   MetricService(const IntrusiveList<Metric>& metrics,
                 const IntrusiveList<Group>& groups)

diff --git a/pw_rpc/client_integration_test.cc b/pw_rpc/client_integration_test.cc
@@ -81,7 +81,7 @@ constexpr Benchmark::Client kServiceClient(client, kChannelId);
 class StringReceiver {
  public:
   const char* Wait() {
-    PW_CHECK(sem_.try_acquire_for(500ms));
+    PW_CHECK(sem_.try_acquire_for(1500ms));
     return buffer_;
   }
 

diff --git a/pw_rpc/docs.rst b/pw_rpc/docs.rst
@@ -776,12 +776,12 @@ Example
 
   namespace {
   // Generated clients are namespaced with their proto library.
-  using pw::rpc::nanopb::EchoServiceClient;
+  using EchoClient = pw_rpc::nanopb::EchoService::Client;
 
   // RPC channel ID on which to make client calls.
   constexpr uint32_t kDefaultChannelId = 1;
 
-  EchoServiceClient::EchoCall echo_call;
+  EchoClient::EchoCall echo_call;
 
   // Callback invoked when a response is received. This is called synchronously
   // from Client::ProcessPacket.
@@ -799,7 +799,7 @@ Example
 
   void CallEcho(const char* message) {
     // Create a client to call the EchoService.
-    EchoServiceClient echo_client(my_rpc_client, kDefaultChannelId);
+    EchoClient echo_client(my_rpc_client, kDefaultChannelId);
 
     pw_rpc_EchoMessage request = pw_rpc_EchoMessage_init_default;
     pw::string::Copy(message, request.msg);

diff --git a/pw_rpc/nanopb/client_integration_test.cc b/pw_rpc/nanopb/client_integration_test.cc
@@ -41,7 +41,7 @@ constexpr uint32_t kChannelId = 1;
 class PayloadReceiver {
  public:
   const char* Wait() {
-    PW_CHECK(sem_.try_acquire_for(500ms));
+    PW_CHECK(sem_.try_acquire_for(1500ms));
     return reinterpret_cast<const char*>(payload_.payload.bytes);
   }
 

diff --git a/pw_rpc/nanopb/codegen_test.cc b/pw_rpc/nanopb/codegen_test.cc
@@ -22,7 +22,8 @@
 namespace pw::rpc {
 namespace test {
 
-class TestService final : public generated::TestService<TestService> {
+class TestService final
+    : public pw_rpc::nanopb::TestService::Service<TestService> {
  public:
   Status TestUnaryRpc(const pw_rpc_test_TestRequest& request,
                       pw_rpc_test_TestResponse& response) {
@@ -197,7 +198,7 @@ TEST(NanopbCodegen, ClientCall_DefaultConstructor) {
   NanopbClientReader<pw_rpc_test_TestStreamResponse> server_streaming_call;
 }
 
-using TestServiceClient = test::nanopb::TestServiceClient;
+using TestServiceClient = test::pw_rpc::nanopb::TestService::Client;
 
 TEST(NanopbCodegen, Client_InvokesUnaryRpcWithCallback) {
   constexpr uint32_t kServiceId = internal::Hash("pw.rpc.test.TestService");

diff --git a/pw_rpc/nanopb/docs.rst b/pw_rpc/nanopb/docs.rst
@@ -227,17 +227,21 @@ service client and receive the response.
   #include "chat_protos/chat_service.rpc.pb.h"
 
   namespace {
+
+    using ChatClient = pw_rpc::nanopb::Chat::Client;
+
     MyChannelOutput output;
     pw::rpc::Channel channels[] = {pw::rpc::Channel::Create<1>(&output)};
     pw::rpc::Client client(channels);
 
     // Callback function for GetRoomInformation.
     void LogRoomInformation(const RoomInfoResponse& response, Status status);
-  }
+
+  }  // namespace
 
   void InvokeSomeRpcs() {
-    // Instantiate a service client to call ChatService methods on channel 1.
-    ChatServiceClient chat_client(client, 1);
+    // Instantiate a service client to call Chat service methods on channel 1.
+    ChatClient chat_client(client, 1);
 
     // The RPC will remain active as long as `call` is alive.
     auto call = chat_client.GetRoomInformation(

diff --git a/pw_rpc/nanopb/method_lookup_test.cc b/pw_rpc/nanopb/method_lookup_test.cc
@@ -20,7 +20,8 @@
 namespace pw::rpc {
 namespace {
 
-class MixedService1 : public test::generated::TestService<MixedService1> {
+class MixedService1
+    : public test::pw_rpc::nanopb::TestService::Service<MixedService1> {
  public:
   StatusWithSize TestUnaryRpc(ConstByteSpan, ByteSpan) {
     return StatusWithSize(5);
@@ -52,7 +53,8 @@ class MixedService1 : public test::generated::TestService<MixedService1> {
   bool called_bidirectional_streaming_method = false;
 };
 
-class MixedService2 : public test::generated::TestService<MixedService2> {
+class MixedService2
+    : public test::pw_rpc::nanopb::TestService::Service<MixedService2> {
  public:
   Status TestUnaryRpc(const pw_rpc_test_TestRequest&,
                       pw_rpc_test_TestResponse&) {

diff --git a/pw_rpc/nanopb/public/pw_rpc/echo_service_nanopb.h b/pw_rpc/nanopb/public/pw_rpc/echo_service_nanopb.h
@@ -19,7 +19,8 @@
 
 namespace pw::rpc {
 
-class EchoService final : public generated::EchoService<EchoService> {
+class EchoService final
+    : public pw_rpc::nanopb::EchoService::Service<EchoService> {
  public:
   Status Echo(const pw_rpc_EchoMessage& request, pw_rpc_EchoMessage& response) {
     std::strncpy(response.msg, request.msg, sizeof(response.msg));

diff --git a/pw_rpc/nanopb/public/pw_rpc/nanopb/server_reader_writer.h b/pw_rpc/nanopb/public/pw_rpc/nanopb/server_reader_writer.h
@@ -291,7 +291,6 @@ class NanopbServerWriter : private internal::NanopbServerCall {
 
   using internal::Call::active;
   using internal::Call::channel_id;
-  using internal::Call::open;
 
   // Writes a response struct. Returns the following Status codes:
   //

diff --git a/pw_rpc/nanopb/server_reader_writer_test.cc b/pw_rpc/nanopb/server_reader_writer_test.cc
@@ -23,7 +23,7 @@
 namespace pw::rpc {
 
 class TestServiceImpl final
-    : public test::generated::TestService<TestServiceImpl> {
+    : public test::pw_rpc::nanopb::TestService::Service<TestServiceImpl> {
  public:
   Status TestUnaryRpc(const pw_rpc_test_TestRequest&,
                       pw_rpc_test_TestResponse&) {

diff --git a/pw_rpc/public/pw_rpc/benchmark.h b/pw_rpc/public/pw_rpc/benchmark.h
@@ -19,7 +19,8 @@ namespace pw::rpc {
 
 // RPC service with low-level RPCs for transmitting data. Used for benchmarking
 // and testing.
-class BenchmarkService : public generated::Benchmark<BenchmarkService> {
+class BenchmarkService
+    : public pw_rpc::raw::Benchmark::Service<BenchmarkService> {
  public:
   static StatusWithSize UnaryEcho(ConstByteSpan request, ByteSpan response);
 

diff --git a/pw_rpc/public/pw_rpc/internal/call.h b/pw_rpc/public/pw_rpc/internal/call.h
@@ -61,13 +61,6 @@ class Call : public IntrusiveList<Call>::Item {
   // True if the Call is active and ready to send responses.
   [[nodiscard]] bool active() const { return rpc_state_ == kActive; }
 
-  // DEPRECATED: open() was renamed to active() because it is clearer and does
-  //     not conflict with Open() in ReaderWriter classes.
-  // TODO(pwbug/472): Remove the open() method.
-  /* [[deprecated("Renamed to active()")]] */ bool open() const {
-    return active();
-  }
-
   uint32_t id() const { return id_; }
 
   uint32_t channel_id() const {