pw_rpc: Generic method invocations and tests
- Restructure the BUILD.gn file to define the pw_rpc server library with
a template. This makes it simple to build different versions of the
pw_rpc server library.
- Define the ServerContext class, which provides context for a
particular RPC invocation.
- Have the server invoke RPCs if it finds a method with matching IDs.
- Implement a test version of the Method class and expand the server
tests.
Change-Id: Id1f46a88f2618a649d72f8c694ee829aef80588c
diff --git a/pw_rpc/BUILD b/pw_rpc/BUILD
index 4d5936e..9c03beb 100644
--- a/pw_rpc/BUILD
+++ b/pw_rpc/BUILD
@@ -24,26 +24,61 @@
pw_cc_library(
name = "pw_rpc",
+ srcs = [
+ "public/pw_rpc/internal/service.h",
+ "public/pw_rpc/internal/service_registry.h",
+ "server.cc",
+ "service.cc",
+ "service_registry.cc",
+ ],
+ hdrs = [
+ "public/pw_rpc/server.h",
+ "public/pw_rpc/server_context.h",
+ # TODO(hepler): Only building the test version of the server for now.
+ "test_public/pw_rpc/internal/method.h",
+ ],
+ includes = [
+ "public",
+ "test_public",
+ ],
+ deps = [
+ ":common",
+ "//pw_assert",
+ "//pw_log",
+ "//pw_span",
+ "//pw_status",
+ ],
+)
+
+pw_cc_library(
+ name = "common",
+ srcs = [
+ "channel.cc",
+ "packet.cc",
+ ],
hdrs = [
"public/pw_rpc/channel.h",
+ "public/pw_rpc/internal/base_method.h",
+ "public/pw_rpc/internal/packet.h",
"public/pw_rpc/server.h",
],
includes = ["public"],
+ visibility = ["//visibility:private"],
deps = [
"//pw_assert",
"//pw_log",
"//pw_span",
"//pw_status",
],
+)
+
+pw_cc_test(
+ name = "packet_test",
srcs = [
- "channel.cc",
- "packet.cc",
- "public/pw_rpc/internal/packet.h",
- "public/pw_rpc/internal/service.h",
- "public/pw_rpc/internal/service_registry.h",
- "server.cc",
- "service.cc",
- "service_registry.cc",
+ "packet_test.cc",
+ ],
+ deps = [
+ ":common",
],
)
@@ -54,15 +89,6 @@
],
deps = [
":pw_rpc",
- ],
-)
-
-pw_cc_test(
- name = "packet_test",
- srcs = [
- "packet_test.cc",
- ],
- deps = [
- ":pw_rpc",
+ "//pw_assert",
],
)
diff --git a/pw_rpc/BUILD.gn b/pw_rpc/BUILD.gn
index 5226243..37aeba1 100644
--- a/pw_rpc/BUILD.gn
+++ b/pw_rpc/BUILD.gn
@@ -18,32 +18,69 @@
config("default_config") {
include_dirs = [ "public" ]
+ visibility = [ ":*" ]
}
-source_set("pw_rpc") {
- deps = [ ":protos_pwpb" ]
+# pw_rpc servers depend on the protobuf library used to encode and decode
+# requests and responses when invoking methods. This template is used to create
+# instances of the pw_rpc server library with different implementations.
+#
+# The implementation parameter must refer to a library that provides the
+# definition of the Method class in pw_rpc/internal/method.h. The Method class
+# provides the Invoke function, which handles the server use to call into the
+# RPC functions themselves.
+template("_pw_rpc_server_library") {
+ assert(defined(invoker.implementation),
+ "_pw_rpc_server_library requires an implementation to be set")
+
+ source_set(target_name) {
+ forward_variables_from(invoker, "*")
+
+ public_configs = [ ":default_config" ]
+ public_deps = [
+ ":common",
+ dir_pw_span,
+ dir_pw_status,
+ implementation,
+ ]
+ deps = [
+ dir_pw_assert,
+ dir_pw_log,
+ ]
+ public = [
+ "public/pw_rpc/server.h",
+ "public/pw_rpc/server_context.h",
+ ]
+ sources = [
+ "public/pw_rpc/internal/service.h",
+ "public/pw_rpc/internal/service_registry.h",
+ "server.cc",
+ "service.cc",
+ "service_registry.cc",
+ ]
+ allow_circular_includes_from = [ implementation ]
+ friend = [ ":*" ]
+ }
+}
+
+# Classes with no dependencies on the protobuf library for method invocations.
+source_set("common") {
public_configs = [ ":default_config" ]
public_deps = [
+ ":protos_pwpb",
dir_pw_assert,
- dir_pw_log,
dir_pw_span,
dir_pw_status,
]
- public = [
- "public/pw_rpc/channel.h",
- "public/pw_rpc/server.h",
- ]
+ public = [ "public/pw_rpc/channel.h" ]
sources = [
"channel.cc",
"packet.cc",
+ "public/pw_rpc/internal/base_method.h",
"public/pw_rpc/internal/packet.h",
- "public/pw_rpc/internal/service.h",
- "public/pw_rpc/internal/service_registry.h",
- "server.cc",
- "service.cc",
- "service_registry.cc",
]
friend = [ ":*" ]
+ visibility = [ ":*" ]
}
pw_proto_library("protos") {
@@ -61,18 +98,37 @@
]
}
-pw_test("server_test") {
- deps = [
- ":protos_pwpb",
- ":pw_rpc",
- ]
- sources = [ "server_test.cc" ]
+# RPC server for tests only. A mock method implementation is used.
+_pw_rpc_server_library("test_server") {
+ implementation = ":test_method"
+ visibility = [ ":*" ]
+}
+
+config("test_config") {
+ include_dirs = [ "test_public" ]
+ visibility = [ ":test_method" ]
+}
+
+source_set("test_method") {
+ public_configs = [ ":test_config" ]
+ public = [ "test_public/pw_rpc/internal/method.h" ]
+ public_deps = [ ":common" ]
+ visibility = [ ":test_server" ]
}
pw_test("packet_test") {
deps = [
- ":pw_rpc",
+ ":common",
dir_pw_protobuf,
]
sources = [ "packet_test.cc" ]
}
+
+pw_test("server_test") {
+ deps = [
+ ":protos_pwpb",
+ ":test_server",
+ dir_pw_assert,
+ ]
+ sources = [ "server_test.cc" ]
+}
diff --git a/pw_rpc/public/pw_rpc/channel.h b/pw_rpc/public/pw_rpc/channel.h
index 68fceca..133aa4f 100644
--- a/pw_rpc/public/pw_rpc/channel.h
+++ b/pw_rpc/public/pw_rpc/channel.h
@@ -58,14 +58,14 @@
constexpr uint32_t id() const { return id_; }
constexpr bool assigned() const { return id_ != kUnassignedChannelId; }
+ private:
+ friend class Server;
+
span<std::byte> AcquireBuffer() const { return output_->AcquireBuffer(); }
void SendAndReleaseBuffer(size_t size) const {
output_->SendAndReleaseBuffer(size);
}
- private:
- friend class Server;
-
constexpr Channel(uint32_t id, ChannelOutput* output)
: id_(id), output_(output) {
PW_CHECK_UINT_NE(id, kUnassignedChannelId);
diff --git a/pw_rpc/public/pw_rpc/internal/base_method.h b/pw_rpc/public/pw_rpc/internal/base_method.h
new file mode 100644
index 0000000..bf6c95f
--- /dev/null
+++ b/pw_rpc/public/pw_rpc/internal/base_method.h
@@ -0,0 +1,40 @@
+// 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 <cstddef>
+#include <cstdint>
+
+namespace pw::rpc::internal {
+
+// RPC server implementations provide a Method class in the
+// pw_rpc/internal/method.h header that is derived from BaseMethod.
+class BaseMethod {
+ public:
+ constexpr uint32_t id() const { return id_; }
+
+ // Implementations must provide the Invoke method, which the Server calls:
+ //
+ // StatusWithSize Invoke(ServerContext& context,
+ // span<const std::byte> request,
+ // span<std::byte> payload_buffer) const;
+
+ protected:
+ constexpr BaseMethod(uint32_t id) : id_(id) {}
+
+ private:
+ uint32_t id_;
+};
+
+} // namespace pw::rpc::internal
diff --git a/pw_rpc/public/pw_rpc/internal/service.h b/pw_rpc/public/pw_rpc/internal/service.h
index 377b6c1..fb42585 100644
--- a/pw_rpc/public/pw_rpc/internal/service.h
+++ b/pw_rpc/public/pw_rpc/internal/service.h
@@ -14,39 +14,29 @@
#pragma once
#include <cstdint>
+#include <utility>
+#include "pw_rpc/internal/method.h"
#include "pw_span/span.h"
-#include "pw_status/status.h"
namespace pw::rpc::internal {
-// Forward-declare Packet instead of including it to avoid exposing the internal
-// dependency on pw_protobuf.
-class Packet;
-
-class ServiceRegistry;
-
-// Base class for all RPC services. This is not meant to be instantiated
-// directly; use a generated subclass instead.
+// Base class for all RPC services. This cannot be instantiated directly; use a
+// generated subclass instead.
class Service {
public:
- struct Method {
- uint32_t id;
- };
-
- Service(uint32_t id, span<const Method> methods)
- : id_(id), methods_(methods), next_(nullptr) {}
-
uint32_t id() const { return id_; }
- // Handles an incoming packet and populates a response. Errors that occur
- // should be set within the response packet.
- void ProcessPacket(const internal::Packet& request,
- internal::Packet& response,
- span<std::byte> payload_buffer);
+ // 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)), next_(nullptr) {}
private:
- friend class internal::ServiceRegistry;
+ friend class ServiceRegistry;
uint32_t id_;
span<const Method> methods_;
diff --git a/pw_rpc/public/pw_rpc/server.h b/pw_rpc/public/pw_rpc/server.h
index 9a63946..dd55c93 100644
--- a/pw_rpc/public/pw_rpc/server.h
+++ b/pw_rpc/public/pw_rpc/server.h
@@ -19,6 +19,12 @@
#include "pw_rpc/internal/service_registry.h"
namespace pw::rpc {
+namespace internal {
+
+class Method;
+class Packet;
+
+} // namespace internal
class Server {
public:
@@ -42,7 +48,12 @@
using Service = internal::Service;
using ServiceRegistry = internal::ServiceRegistry;
- void SendResponse(const Channel& channel,
+ void InvokeMethod(const internal::Packet& request,
+ Channel& channel,
+ internal::Packet& response,
+ span<std::byte> buffer) const;
+
+ void SendResponse(const Channel& output,
const internal::Packet& response,
span<std::byte> response_buffer) const;
diff --git a/pw_rpc/public/pw_rpc/server_context.h b/pw_rpc/public/pw_rpc/server_context.h
new file mode 100644
index 0000000..2dcc3e0
--- /dev/null
+++ b/pw_rpc/public/pw_rpc/server_context.h
@@ -0,0 +1,55 @@
+// 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 "pw_assert/assert.h"
+#include "pw_rpc/channel.h"
+
+namespace pw::rpc {
+namespace internal {
+
+class Method;
+class Service;
+
+} // namespace internal
+
+// The ServerContext collects context for an RPC being invoked on a server.
+class ServerContext {
+ public:
+ constexpr ServerContext()
+ : channel_(nullptr), service_(nullptr), method_(nullptr) {}
+
+ uint32_t channel_id() const {
+ PW_DCHECK_NOTNULL(channel_);
+ return channel_->id();
+ }
+
+ private:
+ friend class Server;
+
+ constexpr ServerContext(Channel& channel,
+ internal::Service& service,
+ const internal::Method& method)
+ : channel_(&channel), service_(&service), method_(&method) {
+ // Temporarily silence unused private field warnings.
+ (void)service_;
+ (void)method_;
+ }
+
+ Channel* channel_;
+ internal::Service* service_;
+ const internal::Method* method_;
+};
+
+} // namespace pw::rpc
diff --git a/pw_rpc/server.cc b/pw_rpc/server.cc
index a797234..879cae4 100644
--- a/pw_rpc/server.cc
+++ b/pw_rpc/server.cc
@@ -15,15 +15,19 @@
#include "pw_rpc/server.h"
#include "pw_log/log.h"
+#include "pw_rpc/internal/method.h"
#include "pw_rpc/internal/packet.h"
+#include "pw_rpc/server_context.h"
namespace pw::rpc {
+using std::byte;
+
+using internal::Method;
using internal::Packet;
using internal::PacketType;
-void Server::ProcessPacket(span<const std::byte> data,
- ChannelOutput& interface) {
+void Server::ProcessPacket(span<const byte> data, ChannelOutput& interface) {
Packet packet = Packet::FromBuffer(data);
if (packet.is_control()) {
// TODO(frolv): Handle control packets.
@@ -55,22 +59,46 @@
}
}
- span<std::byte> response_buffer = channel->AcquireBuffer();
- span<std::byte> payload_buffer = packet.PayloadUsableSpace(response_buffer);
-
response.set_channel_id(channel->id());
+ const span<byte> response_buffer = channel->AcquireBuffer();
- Service* service = services_.Find(packet.service_id());
+ // Invoke the method with matching service and method IDs, if any.
+ InvokeMethod(packet, *channel, response, response_buffer);
+ SendResponse(*channel, response, response_buffer);
+}
+
+void Server::InvokeMethod(const Packet& request,
+ Channel& channel,
+ internal::Packet& response,
+ span<std::byte> buffer) const {
+ Service* service = services_.Find(request.service_id());
if (service == nullptr) {
// Couldn't find the requested service. Reply with a NOT_FOUND response
- // without the server_id field set.
+ // without the service_id field set.
response.set_status(Status::NOT_FOUND);
- SendResponse(*channel, response, response_buffer);
return;
}
- service->ProcessPacket(packet, response, payload_buffer);
- SendResponse(*channel, response, response_buffer);
+ response.set_service_id(service->id());
+
+ const internal::Method* method = service->FindMethod(request.method_id());
+
+ if (method == nullptr) {
+ // Couldn't find the requested method. Reply with a NOT_FOUND response
+ // without the method_id field set.
+ response.set_status(Status::NOT_FOUND);
+ return;
+ }
+
+ response.set_method_id(method->id());
+
+ ServerContext context(channel, *service, *method);
+
+ span<byte> response_buffer = request.PayloadUsableSpace(buffer);
+ StatusWithSize result =
+ method->Invoke(context, request.payload(), response_buffer);
+ response.set_status(result.status());
+ response.set_payload(response_buffer.first(result.size()));
}
Channel* Server::FindChannel(uint32_t id) const {
@@ -94,7 +122,7 @@
void Server::SendResponse(const Channel& channel,
const Packet& response,
- span<std::byte> response_buffer) const {
+ span<byte> response_buffer) const {
StatusWithSize sws = response.Encode(response_buffer);
if (!sws.ok()) {
// TODO(frolv): What should be done here?
@@ -106,4 +134,8 @@
channel.SendAndReleaseBuffer(sws.size());
}
+static_assert(std::is_base_of<internal::BaseMethod, internal::Method>(),
+ "The Method implementation must be derived from "
+ "pw::rpc::internal::BaseMethod");
+
} // namespace pw::rpc
diff --git a/pw_rpc/server_test.cc b/pw_rpc/server_test.cc
index 984f5d1..b242088 100644
--- a/pw_rpc/server_test.cc
+++ b/pw_rpc/server_test.cc
@@ -14,15 +14,22 @@
#include "pw_rpc/server.h"
+#include <array>
+#include <cstdint>
+
#include "gtest/gtest.h"
+#include "pw_assert/assert.h"
#include "pw_rpc/internal/packet.h"
+#include "pw_rpc/internal/service.h"
namespace pw::rpc {
namespace {
+using std::byte;
+
+using internal::Method;
using internal::Packet;
using internal::PacketType;
-using std::byte;
template <size_t buffer_size>
class TestOutput : public ChannelOutput {
@@ -42,106 +49,153 @@
span<const byte> sent_packet_;
};
-Packet MakePacket(uint32_t channel_id,
- uint32_t service_id,
- uint32_t method_id,
- span<const byte> payload) {
- Packet packet(PacketType::RPC);
- packet.set_channel_id(channel_id);
- packet.set_service_id(service_id);
- packet.set_method_id(method_id);
- packet.set_payload(payload);
- return packet;
+class TestService : public internal::Service {
+ public:
+ TestService(uint32_t service_id)
+ : internal::Service(service_id, methods_),
+ methods_{
+ Method(100),
+ Method(200),
+ } {}
+
+ Method& method(uint32_t id) {
+ for (Method& method : methods_) {
+ if (method.id() == id) {
+ return method;
+ }
+ }
+
+ PW_CRASH("Invalid method ID %u", static_cast<unsigned>(id));
+ }
+
+ private:
+ std::array<Method, 2> methods_;
+};
+
+class BasicServer : public ::testing::Test {
+ protected:
+ static constexpr byte kDefaultPayload[] = {
+ byte(0x82), byte(0x02), byte(0xff), byte(0xff)};
+
+ BasicServer()
+ : output_(1),
+ channels_{
+ Channel::Create<1>(&output_),
+ Channel::Create<2>(&output_),
+ Channel(), // available for assignment
+ },
+ server_(channels_),
+ service_(42) {
+ server_.RegisterService(service_);
+ }
+
+ TestOutput<128> output_;
+ std::array<Channel, 3> channels_;
+ Server server_;
+ TestService service_;
+
+ span<const byte> EncodeRequest(PacketType type,
+ uint32_t channel_id,
+ uint32_t service_id,
+ uint32_t method_id,
+ span<const byte> payload = kDefaultPayload) {
+ auto sws = Packet(type, channel_id, service_id, method_id, payload)
+ .Encode(request_buffer_);
+ EXPECT_EQ(Status::OK, sws.status());
+ return span(request_buffer_, sws.size());
+ }
+
+ private:
+ byte request_buffer_[64];
+};
+
+TEST_F(BasicServer, ProcessPacket_ValidMethod_InvokesMethod) {
+ server_.ProcessPacket(EncodeRequest(PacketType::RPC, 1, 42, 100), output_);
+
+ const Method& method = service_.method(100);
+ EXPECT_EQ(1u, method.last_channel_id());
+ EXPECT_EQ(sizeof(kDefaultPayload), method.last_request().size());
+ EXPECT_EQ(std::memcmp(kDefaultPayload,
+ method.last_request().data(),
+ method.last_request().size()),
+ 0);
}
-TEST(Server, ProcessPacket_SendsResponse) {
- TestOutput<128> output(1);
- Channel channels[] = {
- Channel::Create<1>(&output),
- Channel::Create<2>(&output),
- };
- Server server(channels);
- internal::Service service(42, {});
- server.RegisterService(service);
+TEST_F(BasicServer, ProcessPacket_ValidMethod_SendsOkResponse) {
+ server_.ProcessPacket(EncodeRequest(PacketType::RPC, 1, 42, 100), output_);
- byte encoded_packet[64];
- constexpr byte payload[] = {byte(0x82), byte(0x02), byte(0xff), byte(0xff)};
- Packet request = MakePacket(1, 42, 27, payload);
- auto sws = request.Encode(encoded_packet);
-
- server.ProcessPacket(span(encoded_packet, sws.size()), output);
- Packet packet = Packet::FromBuffer(output.sent_packet());
- EXPECT_EQ(packet.status(), Status::OK);
+ Packet packet = Packet::FromBuffer(output_.sent_packet());
EXPECT_EQ(packet.channel_id(), 1u);
EXPECT_EQ(packet.service_id(), 42u);
+ EXPECT_EQ(packet.method_id(), 100u);
+ EXPECT_TRUE(packet.payload().empty());
+ EXPECT_EQ(packet.status(), Status::OK);
}
-TEST(Server, ProcessPacket_SendsNotFoundOnInvalidService) {
- TestOutput<128> output(1);
- Channel channels[] = {
- Channel::Create<1>(&output),
- Channel::Create<2>(&output),
- };
- Server server(channels);
- internal::Service service(42, {});
- server.RegisterService(service);
+TEST_F(BasicServer, ProcessPacket_ValidMethod_SendsErrorResponse) {
+ constexpr byte resp[] = {byte{0xf0}, byte{0x0d}};
+ service_.method(200).set_response(resp);
+ service_.method(200).set_status(Status::FAILED_PRECONDITION);
- byte encoded_packet[64];
- constexpr byte payload[] = {byte(0x82), byte(0x02), byte(0xff), byte(0xff)};
- Packet request = MakePacket(1, 43, 27, payload);
- auto sws = request.Encode(encoded_packet);
+ server_.ProcessPacket(EncodeRequest(PacketType::RPC, 2, 42, 200), output_);
- server.ProcessPacket(span(encoded_packet, sws.size()), output);
- Packet packet = Packet::FromBuffer(output.sent_packet());
+ Packet packet = Packet::FromBuffer(output_.sent_packet());
+ EXPECT_EQ(packet.channel_id(), 2u);
+ EXPECT_EQ(packet.service_id(), 42u);
+ EXPECT_EQ(packet.method_id(), 200u);
+ EXPECT_EQ(packet.status(), Status::FAILED_PRECONDITION);
+ ASSERT_EQ(sizeof(resp), packet.payload().size());
+ EXPECT_EQ(std::memcmp(packet.payload().data(), resp, sizeof(resp)), 0);
+}
+
+TEST_F(BasicServer, ProcessPacket_InvalidMethod_NothingIsInvoked) {
+ server_.ProcessPacket(EncodeRequest(PacketType::RPC, 1, 42, 101), output_);
+
+ EXPECT_EQ(0u, service_.method(100).last_channel_id());
+ EXPECT_EQ(0u, service_.method(200).last_channel_id());
+}
+
+TEST_F(BasicServer, ProcessPacket_InvalidMethod_SendsNotFound) {
+ server_.ProcessPacket(EncodeRequest(PacketType::RPC, 1, 42, 27), output_);
+
+ Packet packet = Packet::FromBuffer(output_.sent_packet());
+ EXPECT_EQ(packet.channel_id(), 1u);
+ EXPECT_EQ(packet.service_id(), 42u);
+ EXPECT_EQ(packet.method_id(), 0u); // No method ID 27
+ EXPECT_EQ(packet.status(), Status::NOT_FOUND);
+}
+
+TEST_F(BasicServer, ProcessPacket_InvalidService_SendsNotFound) {
+ server_.ProcessPacket(EncodeRequest(PacketType::RPC, 1, 43, 27), output_);
+
+ Packet packet = Packet::FromBuffer(output_.sent_packet());
EXPECT_EQ(packet.status(), Status::NOT_FOUND);
EXPECT_EQ(packet.channel_id(), 1u);
EXPECT_EQ(packet.service_id(), 0u);
}
-TEST(Server, ProcessPacket_AssignsAnUnassignedChannel) {
- TestOutput<128> output(1);
- Channel channels[] = {
- Channel::Create<1>(&output),
- Channel::Create<2>(&output),
- Channel(),
- };
- Server server(channels);
- internal::Service service(42, {});
- server.RegisterService(service);
-
- byte encoded_packet[64];
- constexpr byte payload[] = {byte(0x82), byte(0x02), byte(0xff), byte(0xff)};
- Packet request = MakePacket(/*channel_id=*/99, 42, 27, payload);
- auto sws = request.Encode(encoded_packet);
-
+TEST_F(BasicServer, ProcessPacket_UnassignedChannel_AssignsToAvalableSlot) {
TestOutput<128> unassigned_output(2);
- server.ProcessPacket(span(encoded_packet, sws.size()), unassigned_output);
- ASSERT_EQ(channels[2].id(), 99u);
+ server_.ProcessPacket(
+ EncodeRequest(PacketType::RPC, /*channel_id=*/99, 42, 27),
+ unassigned_output);
+ ASSERT_EQ(channels_[2].id(), 99u);
Packet packet = Packet::FromBuffer(unassigned_output.sent_packet());
- EXPECT_EQ(packet.status(), Status::OK);
EXPECT_EQ(packet.channel_id(), 99u);
EXPECT_EQ(packet.service_id(), 42u);
+ EXPECT_EQ(packet.method_id(), 0u); // No method ID 27
+ EXPECT_EQ(packet.status(), Status::NOT_FOUND);
}
-TEST(Server, ProcessPacket_SendsResourceExhaustedWhenChannelCantBeAssigned) {
- TestOutput<128> output(1);
- Channel channels[] = {
- Channel::Create<1>(&output),
- Channel::Create<2>(&output),
- };
- Server server(channels);
- internal::Service service(42, {});
- server.RegisterService(service);
+TEST_F(BasicServer,
+ ProcessPacket_UnassignedChannel_SendsResourceExhaustedIfCannotAssign) {
+ channels_[2] = Channel::Create<3>(&output_); // Occupy only available channel
- byte encoded_packet[64];
- constexpr byte payload[] = {byte(0x82), byte(0x02), byte(0xff), byte(0xff)};
- Packet request = MakePacket(/*channel_id=*/99, 42, 27, payload);
- auto sws = request.Encode(encoded_packet);
+ server_.ProcessPacket(
+ EncodeRequest(PacketType::RPC, /*channel_id=*/99, 42, 27), output_);
- server.ProcessPacket(span(encoded_packet, sws.size()), output);
-
- Packet packet = Packet::FromBuffer(output.sent_packet());
+ Packet packet = Packet::FromBuffer(output_.sent_packet());
EXPECT_EQ(packet.status(), Status::RESOURCE_EXHAUSTED);
EXPECT_EQ(packet.channel_id(), 0u);
EXPECT_EQ(packet.service_id(), 0u);
diff --git a/pw_rpc/service.cc b/pw_rpc/service.cc
index 819e6cd..b0ddd23 100644
--- a/pw_rpc/service.cc
+++ b/pw_rpc/service.cc
@@ -14,23 +14,18 @@
#include "pw_rpc/internal/service.h"
-#include "pw_rpc/internal/packet.h"
+#include <type_traits>
namespace pw::rpc::internal {
-void Service::ProcessPacket(const Packet& request,
- Packet& response,
- span<std::byte> payload_buffer) {
- response.set_service_id(id_);
-
+const Method* Service::FindMethod(uint32_t method_id) const {
for (const Method& method : methods_) {
- if (method.id == request.method_id()) {
- // TODO(frolv): call the method i guess
- response.set_method_id(method.id);
+ if (method.id() == method_id) {
+ return &method;
}
}
- (void)payload_buffer;
+ return nullptr;
}
} // namespace pw::rpc::internal
diff --git a/pw_rpc/test_public/pw_rpc/internal/method.h b/pw_rpc/test_public/pw_rpc/internal/method.h
new file mode 100644
index 0000000..ecd8f02
--- /dev/null
+++ b/pw_rpc/test_public/pw_rpc/internal/method.h
@@ -0,0 +1,64 @@
+// 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 <cstdint>
+#include <cstring>
+
+#include "pw_rpc/internal/base_method.h"
+#include "pw_rpc/server_context.h"
+#include "pw_span/span.h"
+#include "pw_status/status_with_size.h"
+
+namespace pw::rpc::internal {
+
+// This is a fake RPC method implementation for testing only. It stores the
+// channel ID, request, and payload buffer, and optionally provides a response.
+class Method : public BaseMethod {
+ public:
+ constexpr Method(uint32_t id) : BaseMethod(id), last_channel_id_(0) {}
+
+ StatusWithSize Invoke(ServerContext& context,
+ span<const std::byte> request,
+ span<std::byte> payload_buffer) const {
+ last_channel_id_ = context.channel_id();
+ last_request_ = request;
+ last_payload_buffer_ = payload_buffer;
+
+ std::memcpy(payload_buffer.data(),
+ response_.data(),
+ std::min(response_.size(), payload_buffer.size()));
+ return StatusWithSize(response_status_, response_.size());
+ }
+
+ uint32_t last_channel_id() const { return last_channel_id_; }
+ span<const std::byte> last_request() const { return last_request_; }
+ span<std::byte> last_payload_buffer() const { return last_payload_buffer_; }
+
+ void set_response(span<const std::byte> payload) { response_ = payload; }
+ void set_status(Status status) { response_status_ = status; }
+
+ private:
+ // Make these mutable so they can be set in the Invoke method, which is const.
+ // The Method class is used exclusively in tests. Having these members mutable
+ // allows tests to verify that the Method is invoked correctly.
+ mutable uint32_t last_channel_id_;
+ mutable span<const std::byte> last_request_;
+ mutable span<std::byte> last_payload_buffer_;
+
+ span<const std::byte> response_;
+ Status response_status_;
+};
+
+} // namespace pw::rpc::internal
