pw_rpc: Expose Service class

This change moves the base Service class to pw_rpc's public API and adds
a source_set that provides service.h separately to the rest of RPC. This
is done to allow creating APIs to globally register services without
having to pull in the entirety of pw_rpc (and potentially creating
dependency cycles).

Change-Id: I80eb34ed35b74caffb71444a59ee05eff5bc8fba
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/15360
Commit-Queue: Alexei Frolov <frolv@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>

13 files changed, 105 insertions, 54 deletions

diff --git a/pw_rpc/BUILD b/pw_rpc/BUILD
index b3670665f..1d5a7dbc1 100644
--- a/pw_rpc/BUILD
+++ b/pw_rpc/BUILD
@@ -27,13 +27,13 @@ pw_cc_library(
     srcs = [
         "base_server_writer.cc",
         "public/pw_rpc/internal/base_server_writer.h",
-        "public/pw_rpc/internal/service.h",
         "server.cc",
         "service.cc",
     ],
     hdrs = [
         "public/pw_rpc/server.h",
         "public/pw_rpc/server_context.h",
+        "public/pw_rpc/service.h",
         "public/pw_rpc/internal/channel.h",
         "public/pw_rpc/internal/server.h",
         "public/pw_rpc/internal/hash.h",
diff --git a/pw_rpc/BUILD.gn b/pw_rpc/BUILD.gn
index 0067ecc0e..233464297 100644
--- a/pw_rpc/BUILD.gn
+++ b/pw_rpc/BUILD.gn
@@ -56,7 +56,6 @@ template("_pw_rpc_server_library") {
       "base_server_writer.cc",
       "public/pw_rpc/internal/base_server_writer.h",
       "public/pw_rpc/internal/server.h",
-      "public/pw_rpc/internal/service.h",
       "server.cc",
       "service.cc",
     ]
@@ -77,13 +76,22 @@ template("_pw_rpc_server_library") {
   }
 }
 
+# Provides the public RPC service definition (but not implementation). Can be
+# used to expose global service registration without depending on the complete
+# RPC library.
+pw_source_set("service") {
+  public_configs = [ ":default_config" ]
+  public_deps = [ "$dir_pw_containers:intrusive_list" ]
+  public = [ "public/pw_rpc/service.h" ]
+}
+
 # Put these dependencies into a group since they need to be shared by the server
 # library and its implementation library.
 group("server_library_deps") {
   public_configs = [ ":default_config" ]
   public_deps = [
     ":common",
-    "$dir_pw_containers:intrusive_list",
+    ":service",
     dir_pw_span,
     dir_pw_status,
   ]
diff --git a/pw_rpc/base_server_writer_test.cc b/pw_rpc/base_server_writer_test.cc
index 2100d5a90..e8999dc52 100644
--- a/pw_rpc/base_server_writer_test.cc
+++ b/pw_rpc/base_server_writer_test.cc
@@ -19,13 +19,13 @@
 #include <cstring>
 
 #include "gtest/gtest.h"
-#include "pw_rpc/internal/service.h"
 #include "pw_rpc/server_context.h"
+#include "pw_rpc/service.h"
 #include "pw_rpc_private/internal_test_utils.h"
 
 namespace pw::rpc {
 
-class TestService : public internal::Service {
+class TestService : public Service {
  public:
   constexpr TestService(uint32_t id)
       : Service(id, std::span(&method, 1)), method(8) {}
diff --git a/pw_rpc/docs.rst b/pw_rpc/docs.rst
index 62835ba1a..09351b70e 100644
--- a/pw_rpc/docs.rst
+++ b/pw_rpc/docs.rst
@@ -437,7 +437,7 @@ Requests
       label = "pw_rpc library";
 
       server [label = "Server"];
-      service [label = "internal::Service"];
+      service [label = "Service"];
       method [label = "internal::Method"];
     }
 
diff --git a/pw_rpc/nanopb/method_test.cc b/pw_rpc/nanopb/method_test.cc
index bbdf7538a..a408b673e 100644
--- a/pw_rpc/nanopb/method_test.cc
+++ b/pw_rpc/nanopb/method_test.cc
@@ -18,8 +18,8 @@
 
 #include "gtest/gtest.h"
 #include "pb_encode.h"
-#include "pw_rpc/internal/service.h"
 #include "pw_rpc/server_context.h"
+#include "pw_rpc/service.h"
 #include "pw_rpc_private/internal_test_utils.h"
 #include "pw_rpc_test_protos/test.pb.h"
 
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 2b4b0faa2..9d2254baa 100644
--- a/pw_rpc/public/pw_rpc/internal/base_server_writer.h
+++ b/pw_rpc/public/pw_rpc/internal/base_server_writer.h
@@ -20,7 +20,7 @@
 #include "pw_containers/intrusive_list.h"
 #include "pw_rpc/internal/call.h"
 #include "pw_rpc/internal/channel.h"
-#include "pw_rpc/internal/service.h"
+#include "pw_rpc/service.h"
 #include "pw_status/status.h"
 
 namespace pw::rpc::internal {
diff --git a/pw_rpc/public/pw_rpc/internal/call.h b/pw_rpc/public/pw_rpc/internal/call.h
index d8813aed2..bdfa56ca1 100644
--- a/pw_rpc/public/pw_rpc/internal/call.h
+++ b/pw_rpc/public/pw_rpc/internal/call.h
@@ -22,12 +22,12 @@
 namespace pw::rpc {
 
 class ServerContext;
+class Service;
 
 namespace internal {
 
 class Method;
 class Server;
-class Service;
 
 // Collects information for an ongoing RPC being processed by the server.
 // The Server creates a ServerCall object to represent a method invocation. The
diff --git a/pw_rpc/public/pw_rpc/server.h b/pw_rpc/public/pw_rpc/server.h
index 5fa5f6a3d..c0ca50048 100644
--- a/pw_rpc/public/pw_rpc/server.h
+++ b/pw_rpc/public/pw_rpc/server.h
@@ -20,7 +20,7 @@
 #include "pw_rpc/channel.h"
 #include "pw_rpc/internal/base_server_writer.h"
 #include "pw_rpc/internal/channel.h"
-#include "pw_rpc/internal/service.h"
+#include "pw_rpc/service.h"
 
 namespace pw::rpc {
 
@@ -33,11 +33,8 @@ class Server {
   ~Server();
 
   // Registers a service with the server. This should not be called directly
-  // with an internal::Service; instead, use a generated class which inherits
-  // from it.
-  void RegisterService(internal::Service& service) {
-    services_.push_front(service);
-  }
+  // with a Service; instead, use a generated class which inherits from it.
+  void RegisterService(Service& service) { services_.push_front(service); }
 
   void ProcessPacket(std::span<const std::byte> packet,
                      ChannelOutput& interface);
@@ -55,7 +52,7 @@ class Server {
   internal::Channel* AssignChannel(uint32_t id, ChannelOutput& interface);
 
   std::span<internal::Channel> channels_;
-  IntrusiveList<internal::Service> services_;
+  IntrusiveList<Service> services_;
   IntrusiveList<internal::BaseServerWriter> writers_;
 };
 
diff --git a/pw_rpc/public/pw_rpc/internal/service.h b/pw_rpc/public/pw_rpc/service.h
index 957de33a9..2076896b7 100644
--- a/pw_rpc/public/pw_rpc/internal/service.h
+++ b/pw_rpc/public/pw_rpc/service.h
@@ -19,29 +19,32 @@
 
 #include "pw_containers/intrusive_list.h"
 
-namespace pw::rpc::internal {
+namespace pw::rpc {
+namespace internal {
 
 class Method;
 
+}  // namespace internal
+
 // Base class for all RPC services. This cannot be instantiated directly; use a
 // generated subclass instead.
 class Service : public IntrusiveList<Service>::Item {
  public:
   uint32_t id() const { return id_; }
 
-  // Finds the method with the provided method_id. Returns nullptr if no match.
-  const Method* FindMethod(uint32_t method_id) const;
-
  protected:
   template <typename T>
   constexpr Service(uint32_t id, T&& methods)
       : id_(id), methods_(std::forward<T>(methods)) {}
 
  private:
-  friend class ServiceRegistry;
+  friend class Server;
+
+  // Finds the method with the provided method_id. Returns nullptr if no match.
+  const internal::Method* FindMethod(uint32_t method_id) const;
 
   uint32_t id_;
-  std::span<const Method> methods_;
+  std::span<const internal::Method> methods_;
 };
 
-}  // namespace pw::rpc::internal
+}  // namespace pw::rpc
diff --git a/pw_rpc/py/codegen_test.py b/pw_rpc/py/codegen_test.py
index d72b6e5da..ee96a8c72 100644
--- a/pw_rpc/py/codegen_test.py
+++ b/pw_rpc/py/codegen_test.py
@@ -48,52 +48,95 @@ service TestService {
 EXPECTED_NANOPB_CODE = """\
 #pragma once
 
+#include <array>
 #include <cstddef>
 #include <cstdint>
+#include <type_traits>
 
-#include "pw_rpc/internal/service.h"
+#include "pw_rpc/internal/method.h"
+#include "pw_rpc/server_context.h"
+#include "pw_rpc/service.h"
 #include "test.pb.h"
 
+namespace pw::rpc::internal {
+
+template <auto>
+class ServiceMethodTraits;
+
+}  // namespace pw::rpc::internal
 
 namespace pw::rpc::test {
+namespace generated {
 
-class TestService : public ::pw::rpc::internal::Service {
+template <typename Implementation>
+class TestService : public ::pw::rpc::Service {
  public:
   using ServerContext = ::pw::rpc::ServerContext;
-  template <typename T> using ServerWriter = ::pw::rpc::ServerWriter<T>;
+  template <typename T>
+  using ServerWriter = ::pw::rpc::ServerWriter<T>;
 
-  constexpr TestService() : ::pw::rpc::internal::Service(kServiceId, kMethods) {}
+  constexpr TestService()
+      : ::pw::rpc::Service(kServiceId, kMethods) {}
 
   TestService(const TestService&) = delete;
   TestService& operator=(const TestService&) = delete;
 
-  static ::pw::Status TestRpc(
-      ServerContext& ctx,
-      const pw_rpc_test_TestRequest& request,
-      pw_rpc_test_TestResponse& response);
+  static constexpr const char* name() { return "TestService"; }
 
-  static ::pw::Status TestStreamRpc(
-      ServerContext& ctx,
-      const pw_rpc_test_Empty& request,
-      ServerWriter<pw_rpc_test_TestStreamResponse>& writer);
+  // Used by ServiceMethodTraits to identify a base service.
+  constexpr void _PwRpcInternalGeneratedBase() const {}
 
  private:
-  // 65599 hash of "TestService".
-  static constexpr uint32_t kServiceId = 0x105b6ac8;
-  static constexpr char* kServiceName = "TestService";
+  // Hash of "pw.rpc.test.TestService".
+  static constexpr uint32_t kServiceId = 0xcc0f6de0;
+
+  static ::pw::Status Invoke_TestRpc(
+      ::pw::rpc::internal::ServerCall& call,
+      const pw_rpc_test_TestRequest& request,
+      pw_rpc_test_TestResponse& response) {
+    return static_cast<Implementation&>(call.service())
+        .TestRpc(call.context(), request, response);
+  }
+
+  static void Invoke_TestStreamRpc(
+      ::pw::rpc::internal::ServerCall& call,
+      const pw_rpc_test_TestRequest& request,
+      ServerWriter<pw_rpc_test_TestStreamResponse>& writer) {
+    static_cast<Implementation&>(call.service())
+        .TestStreamRpc(call.context(), request, writer);
+  }
 
   static constexpr std::array<::pw::rpc::internal::Method, 2> kMethods = {
-      ::pw::rpc::internal::Method::Unary<TestRpc>(
-          0xbc924054,  // 65599 hash of "TestRpc"
+      ::pw::rpc::internal::Method::Unary<Invoke_TestRpc>(
+          0xbc924054,  // Hash of "TestRpc"
           pw_rpc_test_TestRequest_fields,
           pw_rpc_test_TestResponse_fields),
-      ::pw::rpc::internal::Method::ServerStreaming<TestStreamRpc>(
-          0xd97a28fa,  // 65599 hash of "TestStreamRpc"
-          pw_rpc_test_Empty_fields,
+      ::pw::rpc::internal::Method::ServerStreaming<Invoke_TestStreamRpc>(
+          0xd97a28fa,  // Hash of "TestStreamRpc"
+          pw_rpc_test_TestRequest_fields,
           pw_rpc_test_TestStreamResponse_fields),
   };
+
+  template <auto impl_method>
+  static constexpr const ::pw::rpc::internal::Method* MethodFor() {
+    if constexpr (std::is_same_v<decltype(impl_method), decltype(&Implementation::TestRpc)>) {
+      if constexpr (impl_method == &Implementation::TestRpc) {
+        return &std::get<0>(kMethods);
+      }
+    }
+    if constexpr (std::is_same_v<decltype(impl_method), decltype(&Implementation::TestStreamRpc)>) {
+      if constexpr (impl_method == &Implementation::TestStreamRpc) {
+        return &std::get<1>(kMethods);
+      }
+    }
+    return nullptr;
+  }
+
+  template <auto>
+  friend class ::pw::rpc::internal::ServiceMethodTraits;
 };
 
+}  // namespace generated
 }  // namespace pw::rpc::test
 """
 
@@ -128,7 +171,7 @@ class TestNanopbCodegen(unittest.TestCase):
 
         generated_files = os.listdir(self._output_dir.name)
         self.assertEqual(len(generated_files), 1)
-        self.assertEqual(generated_files[0], 'test_rpc.pb.h')
+        self.assertEqual(generated_files[0], 'test.rpc.pb.h')
 
         # Read the generated file, ignoring its preamble.
         generated_code = Path(self._output_dir.name,
diff --git a/pw_rpc/py/pw_rpc/codegen_nanopb.py b/pw_rpc/py/pw_rpc/codegen_nanopb.py
index 9432fa921..5aa21b86e 100644
--- a/pw_rpc/py/pw_rpc/codegen_nanopb.py
+++ b/pw_rpc/py/pw_rpc/codegen_nanopb.py
@@ -145,7 +145,7 @@ def _generate_code_for_service(service: ProtoNode, root: ProtoNode,
                                output: OutputFile) -> None:
     """Generates a C++ derived class for a nanopb RPC service."""
 
-    base_class = f'{RPC_NAMESPACE}::internal::Service'
+    base_class = f'{RPC_NAMESPACE}::Service'
     output.write_line('\ntemplate <typename Implementation>')
     output.write_line(
         f'class {service.cpp_namespace(root)} : public {base_class} {{')
@@ -229,8 +229,8 @@ def generate_code_for_package(file_descriptor_proto, package: ProtoNode,
     output.write_line('#include <cstdint>')
     output.write_line('#include <type_traits>\n')
     output.write_line('#include "pw_rpc/internal/method.h"')
-    output.write_line('#include "pw_rpc/internal/service.h"')
     output.write_line('#include "pw_rpc/server_context.h"')
+    output.write_line('#include "pw_rpc/service.h"')
 
     # Include the corresponding nanopb header file for this proto file, in which
     # the file's messages and enums are generated. All other files imported from
diff --git a/pw_rpc/server_test.cc b/pw_rpc/server_test.cc
index 7f559d756..e2042ac75 100644
--- a/pw_rpc/server_test.cc
+++ b/pw_rpc/server_test.cc
@@ -20,7 +20,7 @@
 #include "gtest/gtest.h"
 #include "pw_assert/assert.h"
 #include "pw_rpc/internal/packet.h"
-#include "pw_rpc/internal/service.h"
+#include "pw_rpc/service.h"
 #include "pw_rpc_private/internal_test_utils.h"
 
 namespace pw::rpc {
@@ -32,10 +32,10 @@ using internal::Method;
 using internal::Packet;
 using internal::PacketType;
 
-class TestService : public internal::Service {
+class TestService : public Service {
  public:
   TestService(uint32_t service_id)
-      : internal::Service(service_id, methods_),
+      : Service(service_id, methods_),
         methods_{
             Method(100),
             Method(200),
diff --git a/pw_rpc/service.cc b/pw_rpc/service.cc
index 3107c926a..50e0447bf 100644
--- a/pw_rpc/service.cc
+++ b/pw_rpc/service.cc
@@ -12,16 +12,16 @@
 // License for the specific language governing permissions and limitations under
 // the License.
 
-#include "pw_rpc/internal/service.h"
+#include "pw_rpc/service.h"
 
 #include <type_traits>
 
 #include "pw_rpc/internal/method.h"
 
-namespace pw::rpc::internal {
+namespace pw::rpc {
 
-const Method* Service::FindMethod(uint32_t method_id) const {
-  for (const Method& method : methods_) {
+const internal::Method* Service::FindMethod(uint32_t method_id) const {
+  for (const internal::Method& method : methods_) {
     if (method.id() == method_id) {
       return &method;
     }
@@ -30,4 +30,4 @@ const Method* Service::FindMethod(uint32_t method_id) const {
   return nullptr;
 }
 
-}  // namespace pw::rpc::internal
+}  // namespace pw::rpc