From 9aecf23a376f0eecac499989ae4cbbc41ccc7bd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20Sch=C3=BCtte?= Date: Wed, 31 Jan 2018 12:26:56 +0100 Subject: [PATCH] Simplify Handler registration. (#865) --- cartographer_grpc/framework/rpc_handler.h | 1 - .../framework/rpc_handler_interface.h | 6 ++ cartographer_grpc/framework/server.cc | 12 ++++ cartographer_grpc/framework/server.h | 25 +++++--- cartographer_grpc/framework/server_test.cc | 30 +++++++--- .../add_fixed_frame_pose_data_handler.h | 3 + .../handlers/add_imu_data_handler.h | 3 + .../handlers/add_landmark_data_handler.h | 3 + .../add_local_slam_result_data_handler.h | 3 + .../handlers/add_odometry_data_handler.h | 3 + .../handlers/add_rangefinder_data_handler.h | 3 + .../handlers/add_trajectory_handler.h | 3 + .../handlers/finish_trajectory_handler.h | 3 + .../handlers/get_all_submap_poses.h | 3 + .../handlers/get_constraints_handler.h | 3 + .../get_local_to_global_transform_handler.h | 4 ++ .../handlers/get_submap_handler.h | 3 + .../get_trajectory_node_poses_handler.h | 3 + cartographer_grpc/handlers/load_map_handler.h | 3 + .../receive_local_slam_results_handler.h | 3 + ...ion.h => run_final_optimization_handler.h} | 3 + cartographer_grpc/map_builder_server.cc | 60 ++++++------------- 22 files changed, 125 insertions(+), 58 deletions(-) rename cartographer_grpc/handlers/{run_final_optimization.h => run_final_optimization_handler.h} (92%) diff --git a/cartographer_grpc/framework/rpc_handler.h b/cartographer_grpc/framework/rpc_handler.h index 4a73cc5..f72de25 100644 --- a/cartographer_grpc/framework/rpc_handler.h +++ b/cartographer_grpc/framework/rpc_handler.h @@ -57,7 +57,6 @@ class RpcHandler : public RpcHandlerInterface { private: const std::weak_ptr rpc_; }; - void SetExecutionContext(ExecutionContext* execution_context) override { execution_context_ = execution_context; } diff --git a/cartographer_grpc/framework/rpc_handler_interface.h b/cartographer_grpc/framework/rpc_handler_interface.h index 8695ef9..368a16e 100644 --- a/cartographer_grpc/framework/rpc_handler_interface.h +++ b/cartographer_grpc/framework/rpc_handler_interface.h @@ -28,6 +28,12 @@ class Rpc; class RpcHandlerInterface { public: virtual ~RpcHandlerInterface() = default; + // Returns the fully qualified name of the gRPC method this handler is + // implementing. The fully qualified name has the structure + // '/<>/<>', where the service name is the + // fully qualified proto package name of the service and method name the name + // of the method as defined in the service definition of the proto. + virtual std::string method_name() const = 0; virtual void SetExecutionContext(ExecutionContext* execution_context) = 0; virtual void SetRpc(Rpc* rpc) = 0; virtual void OnRequestInternal( diff --git a/cartographer_grpc/framework/server.cc b/cartographer_grpc/framework/server.cc index 2c10462..2d07b86 100644 --- a/cartographer_grpc/framework/server.cc +++ b/cartographer_grpc/framework/server.cc @@ -39,6 +39,18 @@ void Server::Builder::SetServerAddress(const std::string& server_address) { options_.server_address = server_address; } +std::tuple Server::Builder::ParseMethodFullName( + const std::string& method_full_name) { + CHECK(method_full_name.at(0) == '/') << "Invalid method name."; + std::stringstream stream(method_full_name.substr(1)); + std::string service_full_name; + std::getline(stream, service_full_name, '/'); + std::string method_name; + std::getline(stream, method_name, '/'); + CHECK(!service_full_name.empty() && !method_name.empty()); + return std::make_tuple(service_full_name, method_name); +} + std::unique_ptr Server::Builder::Build() { std::unique_ptr server(new Server(options_)); for (const auto& service_handlers : rpc_handlers_) { diff --git a/cartographer_grpc/framework/server.h b/cartographer_grpc/framework/server.h index 16fad41..226de42 100644 --- a/cartographer_grpc/framework/server.h +++ b/cartographer_grpc/framework/server.h @@ -55,12 +55,14 @@ class Server { void SetNumEventThreads(std::size_t num_event_threads); void SetServerAddress(const std::string& server_address); - template - void RegisterHandler(const std::string& method_name) { - std::stringstream fully_qualified_name; - fully_qualified_name << "/" << ServiceType::service_full_name() << "/" - << method_name; - rpc_handlers_[ServiceType::service_full_name()].emplace( + template + void RegisterHandler() { + std::string method_full_name = GetMethodFullName(); + std::string service_full_name; + std::string method_name; + std::tie(service_full_name, method_name) = + ParseMethodFullName(method_full_name); + rpc_handlers_[service_full_name].emplace( method_name, RpcHandlerInfo{ RpcHandlerType::RequestType::default_instance().GetDescriptor(), @@ -74,12 +76,21 @@ class Server { }, RpcType::value, - fully_qualified_name.str()}); + method_full_name}); } private: using ServiceInfo = std::map; + template + std::string GetMethodFullName() { + auto handler = cartographer::common::make_unique(); + return handler->method_name(); + } + std::tuple + ParseMethodFullName(const std::string& method_full_name); + Options options_; std::map rpc_handlers_; }; diff --git a/cartographer_grpc/framework/server_test.cc b/cartographer_grpc/framework/server_test.cc index 884ef63..5c5e6bc 100644 --- a/cartographer_grpc/framework/server_test.cc +++ b/cartographer_grpc/framework/server_test.cc @@ -23,6 +23,7 @@ #include "cartographer_grpc/framework/proto/math_service.pb.h" #include "cartographer_grpc/framework/rpc_handler.h" #include "glog/logging.h" +#include "google/protobuf/descriptor.h" #include "grpc++/grpc++.h" #include "gtest/gtest.h" @@ -40,6 +41,9 @@ class MathServerContext : public ExecutionContext { class GetSumHandler : public RpcHandler, proto::GetSumResponse> { public: + std::string method_name() const override { + return "/cartographer_grpc.framework.proto.Math/GetSum"; + } void OnRequest(const proto::GetSumRequest& request) override { sum_ += GetContext()->additional_increment(); sum_ += request.input(); @@ -58,6 +62,9 @@ class GetSumHandler class GetRunningSumHandler : public RpcHandler, Stream> { public: + std::string method_name() const override { + return "/cartographer_grpc.framework.proto.Math/GetRunningSum"; + } void OnRequest(const proto::GetSumRequest& request) override { sum_ += request.input(); @@ -78,6 +85,10 @@ class GetRunningSumHandler : public RpcHandler, class GetSquareHandler : public RpcHandler { + public: + std::string method_name() const override { + return "/cartographer_grpc.framework.proto.Math/GetSquare"; + } void OnRequest(const proto::GetSquareRequest& request) override { auto response = cartographer::common::make_unique(); @@ -88,6 +99,10 @@ class GetSquareHandler class GetEchoHandler : public RpcHandler { + public: + std::string method_name() const override { + return "/cartographer_grpc.framework.proto.Math/GetEcho"; + } void OnRequest(const proto::GetEchoRequest& request) override { int value = request.input(); Writer writer = GetWriter(); @@ -105,6 +120,9 @@ class GetSequenceHandler : public RpcHandler> { public: + std::string method_name() const override { + return "/cartographer_grpc.framework.proto.Math/GetSequence"; + } void OnRequest(const proto::GetSequenceRequest& request) override { for (int i = 0; i < request.input(); ++i) { auto response = @@ -129,13 +147,11 @@ class ServerTest : public ::testing::Test { server_builder.SetServerAddress(kServerAddress); server_builder.SetNumGrpcThreads(kNumThreads); server_builder.SetNumEventThreads(kNumThreads); - server_builder.RegisterHandler("GetSum"); - server_builder.RegisterHandler("GetSquare"); - server_builder.RegisterHandler( - "GetRunningSum"); - server_builder.RegisterHandler("GetEcho"); - server_builder.RegisterHandler( - "GetSequence"); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); server_ = server_builder.Build(); client_channel_ = diff --git a/cartographer_grpc/handlers/add_fixed_frame_pose_data_handler.h b/cartographer_grpc/handlers/add_fixed_frame_pose_data_handler.h index 4973799..5a0f4ad 100644 --- a/cartographer_grpc/handlers/add_fixed_frame_pose_data_handler.h +++ b/cartographer_grpc/handlers/add_fixed_frame_pose_data_handler.h @@ -32,6 +32,9 @@ class AddFixedFramePoseDataHandler framework::Stream, google::protobuf::Empty> { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/AddFixedFramePoseData"; + } void OnRequest(const proto::AddFixedFramePoseDataRequest &request) override { // The 'BlockingQueue' returned by 'sensor_data_queue()' is already // thread-safe. Therefore it suffices to get an unsynchronized reference to diff --git a/cartographer_grpc/handlers/add_imu_data_handler.h b/cartographer_grpc/handlers/add_imu_data_handler.h index fa4a15a..91880ab 100644 --- a/cartographer_grpc/handlers/add_imu_data_handler.h +++ b/cartographer_grpc/handlers/add_imu_data_handler.h @@ -31,6 +31,9 @@ class AddImuDataHandler : public framework::RpcHandler, google::protobuf::Empty> { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/AddImuData"; + } void OnRequest(const proto::AddImuDataRequest &request) override { // The 'BlockingQueue' returned by 'sensor_data_queue()' is already // thread-safe. Therefore it suffices to get an unsynchronized reference to diff --git a/cartographer_grpc/handlers/add_landmark_data_handler.h b/cartographer_grpc/handlers/add_landmark_data_handler.h index a750f84..5b5981e 100644 --- a/cartographer_grpc/handlers/add_landmark_data_handler.h +++ b/cartographer_grpc/handlers/add_landmark_data_handler.h @@ -31,6 +31,9 @@ class AddLandmarkDataHandler framework::Stream, google::protobuf::Empty> { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/AddLandmarkData"; + } void OnRequest(const proto::AddLandmarkDataRequest &request) override { // The 'BlockingQueue' returned by 'sensor_data_queue()' is already // thread-safe. Therefore it suffices to get an unsynchronized reference to diff --git a/cartographer_grpc/handlers/add_local_slam_result_data_handler.h b/cartographer_grpc/handlers/add_local_slam_result_data_handler.h index d3128f3..773f623 100644 --- a/cartographer_grpc/handlers/add_local_slam_result_data_handler.h +++ b/cartographer_grpc/handlers/add_local_slam_result_data_handler.h @@ -32,6 +32,9 @@ class AddLocalSlamResultDataHandler framework::Stream, google::protobuf::Empty> { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/AddLocalSlamResultData"; + } void OnRequest(const proto::AddLocalSlamResultDataRequest& request) override { auto local_slam_result_data = GetContext() diff --git a/cartographer_grpc/handlers/add_odometry_data_handler.h b/cartographer_grpc/handlers/add_odometry_data_handler.h index 0a9f8d9..f2e04be 100644 --- a/cartographer_grpc/handlers/add_odometry_data_handler.h +++ b/cartographer_grpc/handlers/add_odometry_data_handler.h @@ -31,6 +31,9 @@ class AddOdometryDataHandler framework::Stream, google::protobuf::Empty> { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/AddOdometryData"; + } void OnRequest(const proto::AddOdometryDataRequest &request) override { // The 'BlockingQueue' returned by 'sensor_data_queue()' is already // thread-safe. Therefore it suffices to get an unsynchronized reference to diff --git a/cartographer_grpc/handlers/add_rangefinder_data_handler.h b/cartographer_grpc/handlers/add_rangefinder_data_handler.h index 4942d49..d97fc0b 100644 --- a/cartographer_grpc/handlers/add_rangefinder_data_handler.h +++ b/cartographer_grpc/handlers/add_rangefinder_data_handler.h @@ -31,6 +31,9 @@ class AddRangefinderDataHandler framework::Stream, google::protobuf::Empty> { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/AddRangefinderData"; + } void OnRequest(const proto::AddRangefinderDataRequest &request) override { // The 'BlockingQueue' returned by 'sensor_data_queue()' is already // thread-safe. Therefore it suffices to get an unsynchronized reference to diff --git a/cartographer_grpc/handlers/add_trajectory_handler.h b/cartographer_grpc/handlers/add_trajectory_handler.h index e217947..048d8a2 100644 --- a/cartographer_grpc/handlers/add_trajectory_handler.h +++ b/cartographer_grpc/handlers/add_trajectory_handler.h @@ -30,6 +30,9 @@ class AddTrajectoryHandler : public framework::RpcHandler { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/AddTrajectory"; + } void OnRequest(const proto::AddTrajectoryRequest& request) override { auto local_slam_result_callback = GetUnsynchronizedContext() diff --git a/cartographer_grpc/handlers/finish_trajectory_handler.h b/cartographer_grpc/handlers/finish_trajectory_handler.h index 3e40238..e2d7bae 100644 --- a/cartographer_grpc/handlers/finish_trajectory_handler.h +++ b/cartographer_grpc/handlers/finish_trajectory_handler.h @@ -30,6 +30,9 @@ class FinishTrajectoryHandler : public framework::RpcHandler { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/FinishTrajectory"; + } void OnRequest(const proto::FinishTrajectoryRequest& request) override { GetContext() ->map_builder() diff --git a/cartographer_grpc/handlers/get_all_submap_poses.h b/cartographer_grpc/handlers/get_all_submap_poses.h index 81d17b1..aafc8f8 100644 --- a/cartographer_grpc/handlers/get_all_submap_poses.h +++ b/cartographer_grpc/handlers/get_all_submap_poses.h @@ -30,6 +30,9 @@ class GetAllSubmapPosesHandler : public framework::RpcHandler { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/GetAllSubmapPoses"; + } void OnRequest(const google::protobuf::Empty& request) override { auto submap_poses = GetContext() ->map_builder() diff --git a/cartographer_grpc/handlers/get_constraints_handler.h b/cartographer_grpc/handlers/get_constraints_handler.h index 71781dd..4f8e7b4 100644 --- a/cartographer_grpc/handlers/get_constraints_handler.h +++ b/cartographer_grpc/handlers/get_constraints_handler.h @@ -31,6 +31,9 @@ class GetConstraintsHandler : public framework::RpcHandler { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/GetConstraints"; + } void OnRequest(const google::protobuf::Empty& request) override { auto constraints = GetContext() ->map_builder() diff --git a/cartographer_grpc/handlers/get_local_to_global_transform_handler.h b/cartographer_grpc/handlers/get_local_to_global_transform_handler.h index 6c4c2fc..5a37643 100644 --- a/cartographer_grpc/handlers/get_local_to_global_transform_handler.h +++ b/cartographer_grpc/handlers/get_local_to_global_transform_handler.h @@ -24,6 +24,10 @@ class GetLocalToGlobalTransformHandler : public framework::RpcHandler { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/" + "GetLocalToGlobalTransform"; + } void OnRequest( const proto::GetLocalToGlobalTransformRequest& request) override { auto response = cartographer::common::make_unique< diff --git a/cartographer_grpc/handlers/get_submap_handler.h b/cartographer_grpc/handlers/get_submap_handler.h index f836b07..14f2b92 100644 --- a/cartographer_grpc/handlers/get_submap_handler.h +++ b/cartographer_grpc/handlers/get_submap_handler.h @@ -30,6 +30,9 @@ class GetSubmapHandler : public framework::RpcHandler { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/GetSubmap"; + } void OnRequest(const proto::GetSubmapRequest &request) override { auto response = cartographer::common::make_unique(); diff --git a/cartographer_grpc/handlers/get_trajectory_node_poses_handler.h b/cartographer_grpc/handlers/get_trajectory_node_poses_handler.h index fac8e12..cf89db6 100644 --- a/cartographer_grpc/handlers/get_trajectory_node_poses_handler.h +++ b/cartographer_grpc/handlers/get_trajectory_node_poses_handler.h @@ -30,6 +30,9 @@ class GetTrajectoryNodePosesHandler : public framework::RpcHandler { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/GetTrajectoryNodePoses"; + } void OnRequest(const google::protobuf::Empty& request) override { auto node_poses = GetContext() ->map_builder() diff --git a/cartographer_grpc/handlers/load_map_handler.h b/cartographer_grpc/handlers/load_map_handler.h index 36c6b47..ee06245 100644 --- a/cartographer_grpc/handlers/load_map_handler.h +++ b/cartographer_grpc/handlers/load_map_handler.h @@ -31,6 +31,9 @@ class LoadMapHandler : public framework::RpcHandler, google::protobuf::Empty> { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/LoadMap"; + } void OnRequest(const proto::LoadMapRequest& request) override { switch (request.map_chunk_case()) { case proto::LoadMapRequest::kPoseGraph: diff --git a/cartographer_grpc/handlers/receive_local_slam_results_handler.h b/cartographer_grpc/handlers/receive_local_slam_results_handler.h index 0730a32..0c19cba 100644 --- a/cartographer_grpc/handlers/receive_local_slam_results_handler.h +++ b/cartographer_grpc/handlers/receive_local_slam_results_handler.h @@ -30,6 +30,9 @@ class ReceiveLocalSlamResultsHandler proto::ReceiveLocalSlamResultsRequest, framework::Stream> { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/ReceiveLocalSlamResults"; + } void OnRequest( const proto::ReceiveLocalSlamResultsRequest& request) override { auto writer = GetWriter(); diff --git a/cartographer_grpc/handlers/run_final_optimization.h b/cartographer_grpc/handlers/run_final_optimization_handler.h similarity index 92% rename from cartographer_grpc/handlers/run_final_optimization.h rename to cartographer_grpc/handlers/run_final_optimization_handler.h index 8e11124..32a608d 100644 --- a/cartographer_grpc/handlers/run_final_optimization.h +++ b/cartographer_grpc/handlers/run_final_optimization_handler.h @@ -31,6 +31,9 @@ class RunFinalOptimizationHandler : public framework::RpcHandler { public: + std::string method_name() const override { + return "/cartographer_grpc.proto.MapBuilderService/RunFinalOptimization"; + } void OnRequest(const google::protobuf::Empty& request) override { GetContext() ->map_builder() diff --git a/cartographer_grpc/map_builder_server.cc b/cartographer_grpc/map_builder_server.cc index e9db38a..ca62a2a 100644 --- a/cartographer_grpc/map_builder_server.cc +++ b/cartographer_grpc/map_builder_server.cc @@ -31,7 +31,7 @@ #include "cartographer_grpc/handlers/get_trajectory_node_poses_handler.h" #include "cartographer_grpc/handlers/load_map_handler.h" #include "cartographer_grpc/handlers/receive_local_slam_results_handler.h" -#include "cartographer_grpc/handlers/run_final_optimization.h" +#include "cartographer_grpc/handlers/run_final_optimization_handler.h" #include "cartographer_grpc/proto/map_builder_service.grpc.pb.h" #include "cartographer_grpc/sensor/serialization.h" #include "glog/logging.h" @@ -216,48 +216,22 @@ MapBuilderServer::MapBuilderServer( cartographer::common::make_unique( map_builder_server_options.uplink_server_address()); } - server_builder.RegisterHandler("AddTrajectory"); - server_builder.RegisterHandler("AddOdometryData"); - server_builder - .RegisterHandler( - "AddImuData"); - server_builder.RegisterHandler( - "AddRangefinderData"); - server_builder.RegisterHandler( - "AddFixedFramePoseData"); - server_builder.RegisterHandler("AddLandmarkData"); - server_builder.RegisterHandler( - "AddLocalSlamResultData"); - server_builder.RegisterHandler("FinishTrajectory"); - server_builder.RegisterHandler( - "ReceiveLocalSlamResults"); - server_builder - .RegisterHandler( - "GetSubmap"); - server_builder.RegisterHandler( - "GetTrajectoryNodePoses"); - server_builder.RegisterHandler("GetAllSubmapPoses"); - server_builder.RegisterHandler( - "GetLocalToGlobalTransform"); - server_builder.RegisterHandler("GetConstraints"); - server_builder - .RegisterHandler( - "LoadMap"); - server_builder.RegisterHandler( - "RunFinalOptimization"); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); + server_builder.RegisterHandler(); grpc_server_ = server_builder.Build(); grpc_server_->SetExecutionContext( cartographer::common::make_unique(this));