pw_rpc: Enable SynchronousCall wrappers for pwpb

The SynchronousCall wrappers that were written for nanopb are also compatible with pwpb. This CL migrates the headers to a common directory and adds tests specific to pwpb. Change-Id: I838dc8bf54f8315fdf09911b93db8132972e79da Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/92761 Reviewed-by: Scott James Remnant <[email protected]> Reviewed-by: Wyatt Hepler <[email protected]> Pigweed-Auto-Submit: RJ Ascani <[email protected]> Commit-Queue: Auto-Submit <[email protected]>
google · May 3, 2022 · b4f7df2 · b4f7df2
1 parent 2d52cad
commit b4f7df2
Show file tree

Hide file tree

Showing 15 changed files with 383 additions and 106 deletions.
diff --git a/pw_rpc/BUILD.bazel b/pw_rpc/BUILD.bazel
@@ -118,6 +118,20 @@ pw_cc_library(
     ],
 )
 
+pw_cc_library(
+    name = "synchronous_client_api",
+    hdrs = [
+        "public/pw_rpc/synchronous_call.h",
+        "public/pw_rpc/synchronous_call_result.h",
+    ],
+    includes = ["public"],
+    deps = [
+        ":pw_rpc",
+        "//pw_chrono:system_clock",
+        "//pw_sync:timed_thread_notification",
+    ],
+)
+
 pw_cc_library(
     name = "thread_testing",
     hdrs = ["public/pw_rpc/thread_testing.h"],

diff --git a/pw_rpc/BUILD.gn b/pw_rpc/BUILD.gn
@@ -115,6 +115,20 @@ pw_source_set("client_server") {
   sources = [ "client_server.cc" ]
 }
 
+pw_source_set("synchronous_client_api") {
+  public_configs = [ ":public_include_path" ]
+  public_deps = [
+    ":client",
+    ":common",
+    "$dir_pw_chrono:system_clock",
+    "$dir_pw_sync:timed_thread_notification",
+  ]
+  public = [
+    "public/pw_rpc/synchronous_call.h",
+    "public/pw_rpc/synchronous_call_result.h",
+  ]
+}
+
 # Classes shared by the server and client.
 pw_source_set("common") {
   public_configs = [ ":public_include_path" ]

diff --git a/pw_rpc/CMakeLists.txt b/pw_rpc/CMakeLists.txt
@@ -64,6 +64,17 @@ if(Zephyr_FOUND AND CONFIG_PIGWEED_RPC_CLIENT_SERVER)
   zephyr_link_libraries(pw_rpc.client_server)
 endif()
 
+pw_add_module_library(pw_rpc.synchronous_client_api
+  HEADERS
+    public/pw_rpc/synchronous_call.h
+    public/pw_rpc/synchronous_call_result.h
+  PUBLIC_DEPS
+    pw_chono.system_clock
+    pw_rpc.client
+    pw_rpc.common
+    pw_sync.timed_thread_notification
+)
+
 pw_add_module_library(pw_rpc.common
   SOURCES
     channel.cc

diff --git a/pw_rpc/docs.rst b/pw_rpc/docs.rst
@@ -876,6 +876,81 @@ Example
     }
   }
 
+Client Synchronous Call wrappers
+--------------------------------
+If synchronous behavior is desired when making client calls, users can use one
+of the ``SynchronousCall<RpcMethod>`` wrapper functions to make their RPC call.
+These wrappers effectively wrap the asynchronous Client RPC call with a timed
+thread notification and return once a result is known or a timeout has occurred.
+These return a ``SynchronousCallResult<Response>`` object, which can be queried
+to determine whether any error scenarios occurred and, if not, access the
+response.
+
+``SynchronousCall<RpcMethod>`` will block indefinitely, whereas
+``SynchronousCallFor<RpcMethod>`` and ``SynchronousCallUntil<RpcMethod>`` will
+block for a given timeout or until a deadline, respectively. All wrappers work
+with both the standalone static RPC functions and the generated Client member
+methods.
+
+.. note:: Use of the SynchronousCall wrappers requires a TimedThreadNotification
+   backend.
+.. note:: Only nanopb and pw_protobuf Unary RPC methods are supported.
+
+Example
+^^^^^^^
+.. code-block:: c++
+
+  #include "pw_rpc/synchronous_call.h"
+
+  void InvokeUnaryRpc() {
+    pw::rpc::Client client;
+    pw::rpc::Channel channel;
+
+    RoomInfoRequest request;
+    SynchronousCallResult<RoomInfoResponse> result =
+      SynchronousCall<Chat::GetRoomInformation>(client, channel.id(), request);
+
+    if (result.is_rpc_error()) {
+      ShutdownClient(client);
+    } else if (result.is_server_error()) {
+      HandleServerError(result.status());
+    } else if (result.is_timeout()) {
+      // SynchronousCall will block indefinitely, so we should never get here.
+      PW_UNREACHABLE();
+    }
+    HandleRoomInformation(std::move(result).response());
+  }
+
+  void AnotherExample() {
+    pw_rpc::nanopb::Chat::Client chat_client(client, channel);
+    constexpr auto kTimeout = pw::chrono::SystemClock::for_at_least(500ms);
+
+    RoomInfoRequest request;
+    auto result = SynchronousCallFor<Chat::GetRoomInformation>(
+        chat_client, request, kTimeout);
+
+    if (result.is_timeout()) {
+      RetryRoomRequest();
+    } else {
+    ...
+    }
+  }
+
+The ``SynchronousCallResult<Response>`` is also compatible with the PW_TRY
+family of macros, but users should be aware that their use will lose information
+about the type of error. This should only be used if the caller will handle all
+error scenarios the same.
+
+.. code-block:: c++
+
+  pw::Status SyncRpc() {
+    const RoomInfoRequest request;
+    PW_TRY_ASSIGN(const RoomInfoResponse& response,
+                  SynchronousCall<Chat::GetRoomInformation>(client, request));
+    HandleRoomInformation(response);
+    return pw::OkStatus();
+  }
+
 Client implementation details
 -----------------------------
 

diff --git a/pw_rpc/nanopb/BUILD.bazel b/pw_rpc/nanopb/BUILD.bazel
@@ -51,19 +51,6 @@ pw_cc_library(
     ],
 )
 
-pw_cc_library(
-    name = "synchronous_client_api",
-    hdrs = [
-        "public/pw_rpc/nanopb/synchronous_call.h",
-        "public/pw_rpc/nanopb/synchronous_call_result.h",
-    ],
-    includes = ["public"],
-    deps = [
-        ":client_api",
-        "//pw_sync:timed_thread_notification",
-    ],
-)
-
 pw_cc_library(
     name = "common",
     srcs = ["common.cc"],
@@ -275,12 +262,12 @@ pw_cc_test(
 )
 
 pw_cc_test(
-    name = "synchronos_call_test",
+    name = "synchronous_call_test",
     srcs = ["synchronous_call_test.cc"],
     deps = [
-        ":synchronous_client_api",
         ":test_method_context",
         "//pw_rpc:pw_rpc_test_cc.nanopb_rpc",
+        "//pw_rpc:synchronous_client_api",
         "//pw_work_queue",
         "//pw_work_queue:stl_test_thread",
         "//pw_work_queue:test_thread_header",

diff --git a/pw_rpc/nanopb/BUILD.gn b/pw_rpc/nanopb/BUILD.gn
@@ -60,18 +60,6 @@ pw_source_set("client_api") {
   public = [ "public/pw_rpc/nanopb/client_reader_writer.h" ]
 }
 
-pw_source_set("synchronous_client_api") {
-  public_configs = [ ":public" ]
-  public_deps = [
-    ":client_api",
-    "$dir_pw_sync:timed_thread_notification",
-  ]
-  public = [
-    "public/pw_rpc/nanopb/synchronous_call.h",
-    "public/pw_rpc/nanopb/synchronous_call_result.h",
-  ]
-}
-
 pw_source_set("common") {
   public_deps = [
     "..:common",
@@ -299,11 +287,11 @@ pw_test("stub_generation_test") {
 
 pw_test("synchronous_call_test") {
   deps = [
-    ":synchronous_client_api",
     ":test_method_context",
     "$dir_pw_work_queue:pw_work_queue",
     "$dir_pw_work_queue:stl_test_thread",
     "$dir_pw_work_queue:test_thread",
+    "..:synchronous_client_api",
     "..:test_protos.nanopb_rpc",
   ]
   sources = [ "synchronous_call_test.cc" ]

diff --git a/pw_rpc/nanopb/CMakeLists.txt b/pw_rpc/nanopb/CMakeLists.txt
@@ -76,6 +76,7 @@ pw_auto_add_module_tests(pw_rpc.nanopb
     pw_rpc.client
     pw_rpc.raw
     pw_rpc.server
+    pw_rpc.synchronous_client_api
     pw_rpc.nanopb.common
     pw_rpc.protos.nanopb_rpc
     pw_rpc.test_protos.nanopb_rpc

diff --git a/pw_rpc/nanopb/docs.rst b/pw_rpc/nanopb/docs.rst
@@ -263,81 +263,6 @@ service client and receive the response.
     // Do other stuff now that we have the room information.
   }
 
-Client Synchronous Call wrappers
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-If synchronous behavior is desired when making client calls, users can use one
-of the ``SynchronousCall<RpcMethod>`` wrapper functions to make their RPC call.
-These wrappers effectively wrap the asynchronous Client RPC call with a timed
-thread notification and return once a result is known or a timeout has occurred.
-These return a ``SynchronousCallResult<Response>`` object, which can be queried
-to determine whether any error scenarios occurred and, if not, access the
-response.
-
-``SynchronousCall<RpcMethod>`` will block indefinitely, whereas
-``SynchronousCallFor<RpcMethod>`` and ``SynchronousCallUntil<RpcMethod>`` will
-block for a given timeout or until a deadline, respectively. All wrappers work
-with both the standalone static RPC functions and the generated Client member
-methods.
-
-.. note:: Use of the SynchronousCall wrappers requires a TimedThreadNotification
-   backend.
-.. note:: Only Unary RPC methods are supported.
-
-Example Usage
-~~~~~~~~~~~~~
-.. code-block:: c++
-
-  #include "pw_rpc/nanopb/synchronous_call.h"
-
-  void InvokeUnaryRpc() {
-    pw::rpc::Client client;
-    pw::rpc::Channel channel;
-
-    RoomInfoRequest request;
-    SynchronousCallResult<RoomInfoResponse> result =
-      SynchronousCall<Chat::GetRoomInformation>(client, channel.id(), request);
-
-    if (result.is_rpc_error()) {
-      ShutdownClient(client);
-    } else if (result.is_server_error()) {
-      HandleServerError(result.status());
-    } else if (result.is_timeout()) {
-      // SynchronousCall will block indefinitely, so we should never get here.
-      PW_UNREACHABLE();
-    }
-    HandleRoomInformation(std::move(result).response());
-  }
-
-  void AnotherExample() {
-    pw_rpc::nanopb::Chat::Client chat_client(client, channel);
-    constexpr auto kTimeout = pw::chrono::SystemClock::for_at_least(500ms);
-
-    RoomInfoRequest request;
-    auto result = SynchronousCallFor<Chat::GetRoomInformation>(
-        chat_client, request, kTimeout);
-
-    if (result.is_timeout()) {
-      RetryRoomRequest();
-    } else {
-    ...
-    }
-  }
-
-The ``SynchronousCallResult<Response>`` is also compatible with the PW_TRY
-family of macros, but users should be aware that their use will lose information
-about the type of error. This should only be used if the caller will handle all
-error scenarios the same.
-
-.. code-block:: c++
-
-  pw::Status SyncRpc() {
-    const RoomInfoRequest request;
-    PW_TRY_ASSIGN(const RoomInfoResponse& response,
-                  SynchronousCall<Chat::GetRoomInformation>(client, request));
-    HandleRoomInformation(response);
-    return pw::OkStatus();
-  }
-
 Zephyr
 ======
 To enable ``pw_rpc.nanopb.*`` for Zephyr add ``CONFIG_PIGWEED_RPC_NANOPB=y`` to

diff --git a/pw_rpc/nanopb/synchronous_call_test.cc b/pw_rpc/nanopb/synchronous_call_test.cc
@@ -12,7 +12,7 @@
 // License for the specific language governing permissions and limitations under
 // the License.
 
-#include "pw_rpc/nanopb/synchronous_call.h"
+#include "pw_rpc/synchronous_call.h"
 
 #include <chrono>
 

diff --git a/...b/public/pw_rpc/nanopb/synchronous_call.h → pw_rpc/public/pw_rpc/synchronous_call.h b/...b/public/pw_rpc/nanopb/synchronous_call.h → pw_rpc/public/pw_rpc/synchronous_call.h
@@ -16,7 +16,7 @@
 #include "pw_chrono/system_clock.h"
 #include "pw_rpc/client.h"
 #include "pw_rpc/internal/method_info.h"
-#include "pw_rpc/nanopb/synchronous_call_result.h"
+#include "pw_rpc/synchronous_call_result.h"
 #include "pw_sync/timed_thread_notification.h"
 
 // Synchronous Call wrappers

diff --git a/...c/pw_rpc/nanopb/synchronous_call_result.h → ...c/public/pw_rpc/synchronous_call_result.h b/...c/pw_rpc/nanopb/synchronous_call_result.h → ...c/public/pw_rpc/synchronous_call_result.h
diff --git a/pw_rpc/pwpb/BUILD.bazel b/pw_rpc/pwpb/BUILD.bazel
@@ -256,3 +256,16 @@ pw_cc_test(
         "//pw_rpc:pw_rpc_test_cc.pwpb_rpc",
     ],
 )
+
+pw_cc_test(
+    name = "synchronous_call_test",
+    srcs = ["synchronous_call_test.cc"],
+    deps = [
+        ":test_method_context",
+        "//pw_rpc:pw_rpc_test_cc.pwpb_rpc",
+        "//pw_rpc:synchronous_client_api",
+        "//pw_work_queue",
+        "//pw_work_queue:stl_test_thread",
+        "//pw_work_queue:test_thread_header",
+    ],
+)
diff --git a/pw_rpc/pwpb/BUILD.gn b/pw_rpc/pwpb/BUILD.gn
@@ -16,6 +16,7 @@ import("//build_overrides/pigweed.gni")
 
 import("$dir_pw_build/target_types.gni")
 import("$dir_pw_docgen/docs.gni")
+import("$dir_pw_sync/backend.gni")
 import("$dir_pw_unit_test/test.gni")
 
 config("public") {
@@ -137,9 +138,9 @@ pw_test_group("tests") {
     ":method_union_test",
     ":server_callback_test",
     ":server_reader_writer_test",
-
     ":serde_test",
     ":stub_generation_test",
+    ":synchronous_call_test",
   ]
 }
 
@@ -263,3 +264,16 @@ pw_test("stub_generation_test") {
   deps = [ "..:test_protos.pwpb_rpc" ]
   sources = [ "stub_generation_test.cc" ]
 }
+
+pw_test("synchronous_call_test") {
+  deps = [
+    ":test_method_context",
+    "$dir_pw_work_queue:pw_work_queue",
+    "$dir_pw_work_queue:stl_test_thread",
+    "$dir_pw_work_queue:test_thread",
+    "..:synchronous_client_api",
+    "..:test_protos.pwpb_rpc",
+  ]
+  sources = [ "synchronous_call_test.cc" ]
+  enable_if = pw_sync_TIMED_THREAD_NOTIFICATION_BACKEND != ""
+}
diff --git a/pw_rpc/pwpb/CMakeLists.txt b/pw_rpc/pwpb/CMakeLists.txt
@@ -58,6 +58,7 @@ pw_auto_add_module_tests(pw_rpc.pwpb
     pw_rpc.client
     pw_rpc.raw
     pw_rpc.server
+    pw_rpc.synchronous_client_api
     pw_rpc.pwpb.common
     pw_rpc.protos.pwpb_rpc
     pw_rpc.test_protos.pwpb_rpc