pw_rpc: Add synchronized channel output
This adds an channel output wrapper which synchronizes a ChannelOutput's
operations with a sync::Mutex, and updates the host target's system RPC server
to use it.
Change-Id: I3d2407a8c81a8f0370fd714ded850293d7030209
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/28540
Commit-Queue: Alexei Frolov <frolv@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>
Reviewed-by: Keir Mierle <keir@google.com>
Reviewed-by: Ewout van Bekkum <ewout@google.com>
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 9a84842..ad9e3be 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -24,6 +24,8 @@
add_subdirectory(pw_build EXCLUDE_FROM_ALL)
add_subdirectory(pw_bytes EXCLUDE_FROM_ALL)
add_subdirectory(pw_checksum EXCLUDE_FROM_ALL)
+add_subdirectory(pw_chrono EXCLUDE_FROM_ALL)
+add_subdirectory(pw_chrono_stl EXCLUDE_FROM_ALL)
add_subdirectory(pw_containers EXCLUDE_FROM_ALL)
add_subdirectory(pw_cpu_exception EXCLUDE_FROM_ALL)
add_subdirectory(pw_cpu_exception_armv7m EXCLUDE_FROM_ALL)
@@ -43,6 +45,8 @@
add_subdirectory(pw_status EXCLUDE_FROM_ALL)
add_subdirectory(pw_stream EXCLUDE_FROM_ALL)
add_subdirectory(pw_string EXCLUDE_FROM_ALL)
+add_subdirectory(pw_sync EXCLUDE_FROM_ALL)
+add_subdirectory(pw_sync_stl EXCLUDE_FROM_ALL)
add_subdirectory(pw_sys_io EXCLUDE_FROM_ALL)
add_subdirectory(pw_sys_io_stdio EXCLUDE_FROM_ALL)
add_subdirectory(pw_tokenizer EXCLUDE_FROM_ALL)
diff --git a/pw_chrono/CMakeLists.txt b/pw_chrono/CMakeLists.txt
new file mode 100644
index 0000000..d6cc918
--- /dev/null
+++ b/pw_chrono/CMakeLists.txt
@@ -0,0 +1,22 @@
+# Copyright 2020 The Pigweed Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in compliance with the License. You may obtain a copy of
+# the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations under
+# the License.
+
+include($ENV{PW_ROOT}/pw_build/pigweed.cmake)
+
+pw_add_facade(pw_chrono.system_clock
+ SOURCES
+ system_clock.cc
+ PUBLIC_DEPS
+ pw_preprocessor
+)
diff --git a/pw_chrono_stl/CMakeLists.txt b/pw_chrono_stl/CMakeLists.txt
new file mode 100644
index 0000000..f0b6197
--- /dev/null
+++ b/pw_chrono_stl/CMakeLists.txt
@@ -0,0 +1,20 @@
+# Copyright 2020 The Pigweed Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in compliance with the License. You may obtain a copy of
+# the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations under
+# the License.
+
+include($ENV{PW_ROOT}/pw_build/pigweed.cmake)
+
+pw_add_module_library(pw_chrono_stl.system_clock
+ IMPLEMENTS_FACADES
+ pw_chrono.system_clock
+)
diff --git a/pw_hdlc_lite/BUILD b/pw_hdlc_lite/BUILD
index ef4e127..f80de90 100644
--- a/pw_hdlc_lite/BUILD
+++ b/pw_hdlc_lite/BUILD
@@ -46,12 +46,19 @@
)
pw_cc_library(
+ name = "rpc_channel_output",
+ hdrs = ["public/pw_hdlc_lite/rpc_channel.h"],
+ includes = ["public"],
+ deps = [
+ ":pw_hdlc_lite",
+ "//pw_rpc:server",
+ ],
+)
+
+pw_cc_library(
name = "pw_rpc",
srcs = ["rpc_packets.cc"],
- hdrs = [
- "public/pw_hdlc_lite/rpc_channel.h",
- "public/pw_hdlc_lite/rpc_packets.h",
- ],
+ hdrs = ["public/pw_hdlc_lite/rpc_packets.h"],
includes = ["public"],
deps = [
":pw_hdlc_lite",
diff --git a/pw_hdlc_lite/public/pw_hdlc_lite/rpc_channel.h b/pw_hdlc_lite/public/pw_hdlc_lite/rpc_channel.h
index 03ecd44..049b1f7 100644
--- a/pw_hdlc_lite/public/pw_hdlc_lite/rpc_channel.h
+++ b/pw_hdlc_lite/public/pw_hdlc_lite/rpc_channel.h
@@ -26,8 +26,8 @@
// Custom HDLC ChannelOutput class to write and read data through serial using
// the HDLC-Lite protocol.
//
-// WARNING: This ChannelOutput is not thread-safe.
-// TODO(frolv): Update this to use OS locking primitives.
+// WARNING: This ChannelOutput is not thread-safe. If thread-safety is required,
+// wrap this in a pw::rpc::SynchronizedChannelOutput.
class RpcChannelOutput : public rpc::ChannelOutput {
public:
// The RpcChannelOutput class does not own the buffer it uses to store the
@@ -60,8 +60,8 @@
// RpcChannelOutput with its own buffer.
//
-// WARNING: This ChannelOutput is not thread-safe.
-// TODO(frolv): Update this to use OS locking primitives.
+// WARNING: This ChannelOutput is not thread-safe. If thread-safety is required,
+// wrap this in a pw::rpc::SynchronizedChannelOutput.
template <size_t buffer_size>
class RpcChannelOutputBuffer : public rpc::ChannelOutput {
public:
diff --git a/pw_rpc/BUILD b/pw_rpc/BUILD
index bccb767..aef8d1c 100644
--- a/pw_rpc/BUILD
+++ b/pw_rpc/BUILD
@@ -85,6 +85,16 @@
)
pw_cc_library(
+ name = "synchronized_channel_output",
+ hdrs = ["public/pw_rpc/synchronized_channel_output.h"],
+ includes = ["public"],
+ deps = [
+ ":common",
+ "//pw_sync:mutex",
+ ],
+)
+
+pw_cc_library(
name = "internal_test_utils",
hdrs = [
"public/pw_rpc/internal/test_method.h",
diff --git a/pw_rpc/BUILD.gn b/pw_rpc/BUILD.gn
index 939f074..80db934 100644
--- a/pw_rpc/BUILD.gn
+++ b/pw_rpc/BUILD.gn
@@ -103,6 +103,15 @@
friend = [ "./*" ]
}
+pw_source_set("synchronized_channel_output") {
+ public_configs = [ ":public_include_path" ]
+ public_deps = [
+ ":common",
+ "$dir_pw_sync:mutex",
+ ]
+ public = [ "public/pw_rpc/synchronized_channel_output.h" ]
+}
+
pw_source_set("test_utils") {
public = [
"public/pw_rpc/internal/test_method.h",
diff --git a/pw_rpc/CMakeLists.txt b/pw_rpc/CMakeLists.txt
index 50d9ab0..5a892d1 100644
--- a/pw_rpc/CMakeLists.txt
+++ b/pw_rpc/CMakeLists.txt
@@ -58,6 +58,12 @@
pw_log
)
+pw_add_module_library(pw_rpc.synchronized_channel_output
+ PUBLIC_DEPS
+ pw_rpc.common
+ pw_sync.mutex
+)
+
add_library(pw_rpc.test_utils INTERFACE)
target_include_directories(pw_rpc.test_utils INTERFACE .)
diff --git a/pw_rpc/public/pw_rpc/synchronized_channel_output.h b/pw_rpc/public/pw_rpc/synchronized_channel_output.h
new file mode 100644
index 0000000..348fde9
--- /dev/null
+++ b/pw_rpc/public/pw_rpc/synchronized_channel_output.h
@@ -0,0 +1,49 @@
+// Copyright 2020 The Pigweed Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+#pragma once
+
+#include <algorithm>
+
+#include "pw_rpc/channel.h"
+#include "pw_sync/mutex.h"
+
+namespace pw::rpc {
+
+// Wraps an RPC ChannelOutput implementation with a mutex to synchronize its
+// acquire and release buffer operations. This can be used to allow a simple
+// ChannelOutput implementation to run in multi-threaded contexts. More complex
+// implementations may want to roll their own synchronization.
+template <typename BaseChannelOutput>
+class SynchronizedChannelOutput final : public BaseChannelOutput {
+ public:
+ template <typename... Args>
+ constexpr SynchronizedChannelOutput(sync::Mutex& mutex, Args&&... args)
+ : BaseChannelOutput(std::forward<Args>(args)...), mutex_(mutex) {}
+
+ std::span<std::byte> AcquireBuffer() final {
+ mutex_.lock();
+ return BaseChannelOutput::AcquireBuffer();
+ }
+
+ Status SendAndReleaseBuffer(std::span<const std::byte> buffer) final {
+ Status status = BaseChannelOutput::SendAndReleaseBuffer(buffer);
+ mutex_.unlock();
+ return status;
+ }
+
+ private:
+ sync::Mutex& mutex_;
+};
+
+} // namespace pw::rpc
diff --git a/pw_sync/CMakeLists.txt b/pw_sync/CMakeLists.txt
new file mode 100644
index 0000000..69c553e
--- /dev/null
+++ b/pw_sync/CMakeLists.txt
@@ -0,0 +1,23 @@
+# Copyright 2020 The Pigweed Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in compliance with the License. You may obtain a copy of
+# the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations under
+# the License.
+
+include($ENV{PW_ROOT}/pw_build/pigweed.cmake)
+
+pw_add_facade(pw_sync.mutex
+ SOURCES
+ mutex.cc
+ PUBLIC_DEPS
+ pw_chrono.system_clock
+ pw_preprocessor
+)
diff --git a/pw_sync_stl/CMakeLists.txt b/pw_sync_stl/CMakeLists.txt
new file mode 100644
index 0000000..bf8d343
--- /dev/null
+++ b/pw_sync_stl/CMakeLists.txt
@@ -0,0 +1,20 @@
+# Copyright 2020 The Pigweed Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in compliance with the License. You may obtain a copy of
+# the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations under
+# the License.
+
+include($ENV{PW_ROOT}/pw_build/pigweed.cmake)
+
+pw_add_module_library(pw_sync_stl.mutex_backend
+ IMPLEMENTS_FACADES
+ pw_sync.mutex
+)
diff --git a/pw_toolchain/host_clang/toolchain.cmake b/pw_toolchain/host_clang/toolchain.cmake
index 8c0f866..9f4dd6a 100644
--- a/pw_toolchain/host_clang/toolchain.cmake
+++ b/pw_toolchain/host_clang/toolchain.cmake
@@ -14,9 +14,11 @@
include($ENV{PW_ROOT}/pw_build/pigweed.cmake)
-pw_set_backend(pw_log pw_log_basic)
pw_set_backend(pw_assert pw_assert_log)
+pw_set_backend(pw_chrono.system_clock pw_chrono_stl.system_clock)
+pw_set_backend(pw_log pw_log_basic)
pw_set_backend(pw_rpc.system_server targets.host.system_rpc_server)
+pw_set_backend(pw_sync.mutex pw_sync_stl.mutex_backend)
pw_set_backend(pw_sys_io pw_sys_io_stdio)
set(CMAKE_C_COMPILER clang)
diff --git a/pw_toolchain/host_gcc/toolchain.cmake b/pw_toolchain/host_gcc/toolchain.cmake
index e2ce85f..cf87958 100644
--- a/pw_toolchain/host_gcc/toolchain.cmake
+++ b/pw_toolchain/host_gcc/toolchain.cmake
@@ -14,9 +14,11 @@
include($ENV{PW_ROOT}/pw_build/pigweed.cmake)
-pw_set_backend(pw_log pw_log_basic)
pw_set_backend(pw_assert pw_assert_log)
+pw_set_backend(pw_chrono.system_clock pw_chrono_stl.system_clock)
+pw_set_backend(pw_log pw_log_basic)
pw_set_backend(pw_rpc.system_server targets.host.system_rpc_server)
+pw_set_backend(pw_sync.mutex pw_sync_stl.mutex_backend)
pw_set_backend(pw_sys_io pw_sys_io_stdio)
set(CMAKE_C_COMPILER gcc)
diff --git a/targets/host/BUILD.gn b/targets/host/BUILD.gn
index cbb53ca..4f35eef 100644
--- a/targets/host/BUILD.gn
+++ b/targets/host/BUILD.gn
@@ -31,6 +31,7 @@
deps = [
"$dir_pw_hdlc_lite:pw_rpc",
"$dir_pw_hdlc_lite:rpc_channel_output",
+ "$dir_pw_rpc:synchronized_channel_output",
"$dir_pw_rpc/system_server:facade",
"$dir_pw_stream:socket_stream",
dir_pw_log,
diff --git a/targets/host/CMakeLists.txt b/targets/host/CMakeLists.txt
index 345ae92..a05e237 100644
--- a/targets/host/CMakeLists.txt
+++ b/targets/host/CMakeLists.txt
@@ -15,13 +15,13 @@
include("$ENV{PW_ROOT}/pw_build/pigweed.cmake")
pw_add_module_library(targets.host.system_rpc_server
- IMPLEMENTS_FACADE
+ IMPLEMENTS_FACADES
pw_rpc.system_server
SOURCES
system_rpc_server.cc
PRIVATE_DEPS
- pw_rpc.system_server.facade
pw_hdlc_lite
pw_rpc.server
+ pw_rpc.synchronized_channel_output
pw_stream.socket_stream
)
diff --git a/targets/host/system_rpc_server.cc b/targets/host/system_rpc_server.cc
index bc8ec37..203d2a9 100644
--- a/targets/host/system_rpc_server.cc
+++ b/targets/host/system_rpc_server.cc
@@ -18,6 +18,7 @@
#include "pw_hdlc_lite/rpc_channel.h"
#include "pw_hdlc_lite/rpc_packets.h"
#include "pw_log/log.h"
+#include "pw_rpc/synchronized_channel_output.h"
#include "pw_rpc_system_server/rpc_server.h"
#include "pw_stream/socket_stream.h"
@@ -28,8 +29,13 @@
constexpr uint16_t kSocketPort = 33000;
stream::SocketStream socket_stream;
-hdlc_lite::RpcChannelOutputBuffer<kMaxTransmissionUnit> hdlc_channel_output(
- socket_stream, hdlc_lite::kDefaultRpcAddress, "HDLC channel");
+sync::Mutex channel_output_mutex;
+rpc::SynchronizedChannelOutput<
+ hdlc_lite::RpcChannelOutputBuffer<kMaxTransmissionUnit>>
+ hdlc_channel_output(channel_output_mutex,
+ socket_stream,
+ hdlc_lite::kDefaultRpcAddress,
+ "HDLC channel");
Channel channels[] = {rpc::Channel::Create<1>(&hdlc_channel_output)};
rpc::Server server(channels);
