pw_system: Multi-channel configuration

This change adds a configuration to pw_system that supports separate
channels for primary and logging RPC. The immediate motivation is to
support a Fuchsia pw_rpc integration prototype which will have separate
primary and logging clients.

See the docs.rst change for more info.

Bug: b/297076185

Change-Id: I9c1aff679a77c24a98c95db3f4d9fb54b75c6408
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/167158
Reviewed-by: Armando Montanez <[email protected]>
Commit-Queue: Gary Bressler <[email protected]>

pw_system/BUILD.gn

@@ -39,6 +39,33 @@ config("public_include_path") {
   include_dirs = [ "public" ]
 }
 
+# This config moves RPC logging to a separate RPC channel and HDLC
+# address. This does two things:
+#   * The separate RPC channel allows logging traffic to be treated as
+#     if it is being sent to a different client via a separate RPC
+#     channel. This illustrates the ability for an RPC server to
+#     communicate to multiple clients over multiple physical links.
+#   * The separate HDLC address completely isolates typical RPC traffic
+#     from logging traffic by communicating to a different HDLC endpoint
+#     address. This effectively creates two virtual data pipes over the
+#     same physical link.
+#
+# This is mostly to illustrate pw_rpc's capability to route and multiplex
+# traffic.
+config("multi_endpoint_rpc_overrides") {
+  defines = [
+    "PW_SYSTEM_LOGGING_CHANNEL_ID=10000",
+    "PW_SYSTEM_LOGGING_RPC_HDLC_ADDRESS=10000",
+  ]
+}
+
+# The Pigweed config pattern requires a pw_source_set to provide the
+# configuration defines. This provides the flags in
+# multi_endpoint_rpc_overrides.
+pw_source_set("multi_endpoint_rpc_config") {
+  public_configs = [ ":multi_endpoint_rpc_overrides" ]
+}
+
 pw_source_set("config") {
   sources = [ "public/pw_system/config.h" ]
   public_configs = [ ":public_include_path" ]

pw_system/docs.rst

@@ -169,3 +169,21 @@ See :ref:`module-pw_system-cli` for detailed CLI usage information.
    :maxdepth: 1
 
    cli
+
+-------------------
+Multi-endpoint mode
+-------------------
+
+The default configuration serves all its traffic with the same
+channel ID and RPC address. There is an alternative mode that assigns a separate
+channel ID and address for logging. This can be useful if you want to separate logging and primary RPC to
+``pw_system`` among multiple clients.
+
+To use this mode, add the following to ``gn args out``:
+
+.. code-block::
+
+   pw_system_USE_MULTI_ENDPOINT_CONFIG = true
+
+The settings for the channel ID and address can be found in the target
+``//pw_system:multi_endpoint_rpc_overrides``.

pw_system/hdlc_rpc_server.cc

@@ -16,17 +16,25 @@
 #include <cstddef>
 #include <cstdint>
 #include <cstdio>
+#include <mutex>
 
 #include "pw_assert/check.h"
 #include "pw_hdlc/encoded_size.h"
 #include "pw_hdlc/rpc_channel.h"
 #include "pw_hdlc/rpc_packets.h"
 #include "pw_log/log.h"
+#include "pw_rpc/channel.h"
 #include "pw_sync/mutex.h"
 #include "pw_system/config.h"
 #include "pw_system/io.h"
 #include "pw_system/rpc_server.h"
 
+#if PW_SYSTEM_DEFAULT_CHANNEL_ID != PW_SYSTEM_LOGGING_CHANNEL_ID && \
+    PW_SYSTEM_DEFAULT_RPC_HDLC_ADDRESS == PW_SYSTEM_LOGGING_RPC_HDLC_ADDRESS
+#error \
+    "Default and logging addresses must be different to support multiple channels."
+#endif
+
 namespace pw::system {
 namespace {
 
@@ -35,10 +43,50 @@ constexpr size_t kMaxTransmissionUnit = PW_SYSTEM_MAX_TRANSMISSION_UNIT;
 static_assert(kMaxTransmissionUnit ==
               hdlc::MaxEncodedFrameSize(rpc::cfg::kEncodingBufferSizeBytes));
 
+#if PW_SYSTEM_DEFAULT_CHANNEL_ID == PW_SYSTEM_LOGGING_CHANNEL_ID
 hdlc::FixedMtuChannelOutput<kMaxTransmissionUnit> hdlc_channel_output(
     GetWriter(), PW_SYSTEM_DEFAULT_RPC_HDLC_ADDRESS, "HDLC channel");
 rpc::Channel channels[] = {
     rpc::Channel::Create<kDefaultRpcChannelId>(&hdlc_channel_output)};
+#else
+class SynchronizedChannelOutput : public rpc::ChannelOutput {
+ public:
+  SynchronizedChannelOutput(stream::Writer& writer,
+                            uint64_t address,
+                            const char* channel_name)
+      : rpc::ChannelOutput(channel_name),
+        inner_(writer, address, channel_name) {}
+
+  Status Send(span<const std::byte> buffer) override {
+    std::lock_guard guard(mtx_);
+    auto s = inner_.Send(buffer);
+    return s;
+  }
+
+  size_t MaximumTransmissionUnit() override {
+    std::lock_guard guard(mtx_);
+    auto s = inner_.MaximumTransmissionUnit();
+    return s;
+  }
+
+ private:
+  sync::Mutex mtx_;
+  hdlc::FixedMtuChannelOutput<kMaxTransmissionUnit> inner_ PW_GUARDED_BY(mtx_);
+};
+
+SynchronizedChannelOutput hdlc_channel_output[] = {
+    SynchronizedChannelOutput(GetWriter(),
+                              PW_SYSTEM_DEFAULT_RPC_HDLC_ADDRESS,
+                              "HDLC default channel"),
+    SynchronizedChannelOutput(GetWriter(),
+                              PW_SYSTEM_LOGGING_RPC_HDLC_ADDRESS,
+                              "HDLC logging channel"),
+};
+rpc::Channel channels[] = {
+    rpc::Channel::Create<kDefaultRpcChannelId>(&hdlc_channel_output[0]),
+    rpc::Channel::Create<kLoggingRpcChannelId>(&hdlc_channel_output[1]),
+};
+#endif
 rpc::Server server(channels);
 
 constexpr size_t kDecoderBufferSize =
@@ -69,7 +117,8 @@ class RpcDispatchThread final : public thread::ThreadCore {
         for (std::byte byte : ret_val.value()) {
           if (auto result = decoder.Process(byte); result.ok()) {
             hdlc::Frame& frame = result.value();
-            if (frame.address() == PW_SYSTEM_DEFAULT_RPC_HDLC_ADDRESS) {
+            if (frame.address() == PW_SYSTEM_DEFAULT_RPC_HDLC_ADDRESS ||
+                frame.address() == PW_SYSTEM_LOGGING_RPC_HDLC_ADDRESS) {
               server.ProcessPacket(frame.data());
             }
           }

pw_system/init.cc

@@ -42,7 +42,7 @@ void InitImpl() {
 
   // Setup logging.
   const Status status = GetLogThread().OpenUnrequestedLogStream(
-      kDefaultRpcChannelId, GetRpcServer(), GetLogService());
+      kLoggingRpcChannelId, GetRpcServer(), GetLogService());
   if (!status.ok()) {
     PW_LOG_ERROR("Error opening unrequested log streams %d",
                  static_cast<int>(status.code()));

pw_system/log.cc

@@ -46,7 +46,7 @@ std::array<std::byte, PW_SYSTEM_MAX_LOG_ENTRY_SIZE> log_decode_buffer
     PW_GUARDED_BY(drains_mutex);
 
 std::array<RpcLogDrain, 1> drains{{
-    RpcLogDrain(kDefaultRpcChannelId,
+    RpcLogDrain(kLoggingRpcChannelId,
                 log_decode_buffer,
                 drains_mutex,
                 RpcLogDrain::LogDrainErrorHandling::kIgnoreWriterErrors),

pw_system/public/pw_system/config.h

@@ -44,13 +44,30 @@
 #define PW_SYSTEM_DEFAULT_CHANNEL_ID 1
 #endif  // PW_SYSTEM_DEFAULT_CHANNEL_ID
 
+// PW_SYSTEM_LOGGING_CHANNEL_ID logging RPC channel ID to host. If this is
+// different from PW_SYSTEM_DEFAULT_CHANNEL_ID, then
+// PW_SYSTEM_LOGGING_RPC_HDLC_ADDRESS must also be different from
+// PW_SYSTEM_DEFAULT_RPC_HDLC_ADDRESS.
+//
+// Defaults to PW_SYSTEM_DEFAULT_CHANNEL_ID.
+#ifndef PW_SYSTEM_LOGGING_CHANNEL_ID
+#define PW_SYSTEM_LOGGING_CHANNEL_ID PW_SYSTEM_DEFAULT_CHANNEL_ID
+#endif  // PW_SYSTEM_LOGGING_CHANNEL_ID
+
 // PW_SYSTEM_DEFAULT_RPC_HDLC_ADDRESS RPC HDLC default address.
 //
 // Defaults to 82.
 #ifndef PW_SYSTEM_DEFAULT_RPC_HDLC_ADDRESS
 #define PW_SYSTEM_DEFAULT_RPC_HDLC_ADDRESS 82
 #endif  // PW_SYSTEM_DEFAULT_RPC_HDLC_ADDRESS
 
+// PW_SYSTEM_LOGGING_RPC_HDLC_ADDRESS RPC HDLC logging address.
+//
+// Defaults to PW_SYSTEM_DEFAULT_RPC_HDLC_ADDRESS.
+#ifndef PW_SYSTEM_LOGGING_RPC_HDLC_ADDRESS
+#define PW_SYSTEM_LOGGING_RPC_HDLC_ADDRESS PW_SYSTEM_DEFAULT_RPC_HDLC_ADDRESS
+#endif  // PW_SYSTEM_LOGGING_RPC_HDLC_ADDRESS
+
 // PW_SYSTEM_ENABLE_THREAD_SNAPSHOT_SERVICE specifies if the thread snapshot
 // RPC service is enabled.
 //

pw_system/public/pw_system/rpc_server.h

@@ -23,10 +23,13 @@
 namespace pw::system {
 
 // This is the default channel used by the pw_system RPC server. Some other
-// parts of pw_system (e.g. logging) use this channel ID as the default
-// destination for unrequested data streams.
+// parts of pw_system use this channel ID as the default destination for
+// unrequested data streams.
 inline constexpr uint32_t kDefaultRpcChannelId = PW_SYSTEM_DEFAULT_CHANNEL_ID;
 
+// This is the channel ID used for logging.
+inline constexpr uint32_t kLoggingRpcChannelId = PW_SYSTEM_LOGGING_CHANNEL_ID;
+
 rpc::Server& GetRpcServer();
 
 thread::ThreadCore& GetRpcDispatchThread();

pw_system/system_target.gni

@@ -57,6 +57,18 @@ PW_SYSTEM_SCHEDULER = {
   NATIVE = "native"
 }
 
+declare_args() {
+  # This argument is intended to be user-facing and should NOT be set by a
+  # toolchain. This switches ALL pw_system_target toolchains to use the
+  # multi_endpoint_rpc_config config to illustrate a multi-endpoint mode that
+  # isolates logging and RPC traffic via HDLC multiplexing.
+  #
+  # If you would like to use this in production, it is strongly recommended that
+  # you instead just add the appropriate defines to your target's toolchain
+  # definition.
+  pw_system_USE_MULTI_ENDPOINT_CONFIG = false
+}
+
 # Defines a target toolchain, automatically setting many required build
 # arguments to simplify instantiation of a target.
 #
@@ -113,6 +125,10 @@ template("pw_system_target") {
     # TODO(amontanez): This should be set to a "$dir_pw_unit_test:rpc_main"
     # when RPC is working.
     pw_unit_test_MAIN = "$dir_pw_unit_test:logging_main"
+
+    if (pw_system_USE_MULTI_ENDPOINT_CONFIG) {
+      pw_system_CONFIG = "$dir_pw_system:multi_endpoint_rpc_config"
+    }
   }
 
   # Populate architecture-specific build args.