From 1556b6edf4afee6245f97a827162de9b4161f743 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Thu, 14 Jun 2018 21:42:00 -0700 Subject: [PATCH] always get service name from rcl to account for remapping (#498) --- rclcpp/include/rclcpp/client.hpp | 9 +++------ rclcpp/include/rclcpp/service.hpp | 21 ++------------------- rclcpp/src/rclcpp/client.cpp | 10 ++++------ rclcpp/src/rclcpp/executor.cpp | 4 ++-- rclcpp/src/rclcpp/service.cpp | 10 ++-------- 5 files changed, 13 insertions(+), 41 deletions(-) diff --git a/rclcpp/include/rclcpp/client.hpp b/rclcpp/include/rclcpp/client.hpp index 3b33d40..da07517 100644 --- a/rclcpp/include/rclcpp/client.hpp +++ b/rclcpp/include/rclcpp/client.hpp @@ -57,14 +57,13 @@ public: RCLCPP_PUBLIC ClientBase( rclcpp::node_interfaces::NodeBaseInterface * node_base, - rclcpp::node_interfaces::NodeGraphInterface::SharedPtr node_graph, - const std::string & service_name); + rclcpp::node_interfaces::NodeGraphInterface::SharedPtr node_graph); RCLCPP_PUBLIC virtual ~ClientBase(); RCLCPP_PUBLIC - const std::string & + const char * get_service_name() const; RCLCPP_PUBLIC @@ -113,8 +112,6 @@ protected: std::shared_ptr node_handle_; std::shared_ptr client_handle_; - - std::string service_name_; }; template @@ -143,7 +140,7 @@ public: rclcpp::node_interfaces::NodeGraphInterface::SharedPtr node_graph, const std::string & service_name, rcl_client_options_t & client_options) - : ClientBase(node_base, node_graph, service_name) + : ClientBase(node_base, node_graph) { using rosidl_typesupport_cpp::get_service_type_support_handle; auto service_type_support_handle = diff --git a/rclcpp/include/rclcpp/service.hpp b/rclcpp/include/rclcpp/service.hpp index efe6fe8..1cf9c55 100644 --- a/rclcpp/include/rclcpp/service.hpp +++ b/rclcpp/include/rclcpp/service.hpp @@ -42,11 +42,6 @@ class ServiceBase public: RCLCPP_SMART_PTR_DEFINITIONS_NOT_COPYABLE(ServiceBase) - RCLCPP_PUBLIC - ServiceBase( - std::shared_ptr node_handle, - const std::string & service_name); - RCLCPP_PUBLIC explicit ServiceBase( std::shared_ptr node_handle); @@ -55,7 +50,7 @@ public: virtual ~ServiceBase(); RCLCPP_PUBLIC - std::string + const char * get_service_name(); RCLCPP_PUBLIC @@ -86,7 +81,6 @@ protected: std::shared_ptr node_handle_; std::shared_ptr service_handle_; - std::string service_name_; bool owns_rcl_handle_ = true; }; @@ -111,7 +105,7 @@ public: const std::string & service_name, AnyServiceCallback any_callback, rcl_service_options_t & service_options) - : ServiceBase(node_handle, service_name), any_callback_(any_callback) + : ServiceBase(node_handle), any_callback_(any_callback) { using rosidl_typesupport_cpp::get_service_type_support_handle; auto service_type_support_handle = get_service_type_support_handle(); @@ -177,12 +171,7 @@ public: // *INDENT-ON* } - const char * service_name = rcl_service_get_service_name(service_handle.get()); - if (!service_name) { - throw std::runtime_error("failed to get service name"); - } service_handle_ = service_handle; - service_name_ = std::string(service_name); } Service( @@ -200,12 +189,6 @@ public: // *INDENT-ON* } - const char * service_name = rcl_service_get_service_name(service_handle); - if (!service_name) { - throw std::runtime_error("failed to get service name"); - } - service_name_ = std::string(service_name); - // In this case, rcl owns the service handle memory service_handle_ = std::shared_ptr(new rcl_service_t); service_handle_->impl = service_handle->impl; diff --git a/rclcpp/src/rclcpp/client.cpp b/rclcpp/src/rclcpp/client.cpp index 4e50c04..f7966a9 100644 --- a/rclcpp/src/rclcpp/client.cpp +++ b/rclcpp/src/rclcpp/client.cpp @@ -35,11 +35,9 @@ using rclcpp::exceptions::throw_from_rcl_error; ClientBase::ClientBase( rclcpp::node_interfaces::NodeBaseInterface * node_base, - rclcpp::node_interfaces::NodeGraphInterface::SharedPtr node_graph, - const std::string & service_name) + rclcpp::node_interfaces::NodeGraphInterface::SharedPtr node_graph) : node_graph_(node_graph), - node_handle_(node_base->get_shared_rcl_node_handle()), - service_name_(service_name) + node_handle_(node_base->get_shared_rcl_node_handle()) { std::weak_ptr weak_node_handle(node_handle_); client_handle_ = std::shared_ptr( @@ -70,10 +68,10 @@ ClientBase::~ClientBase() client_handle_.reset(); } -const std::string & +const char * ClientBase::get_service_name() const { - return this->service_name_; + return rcl_client_get_service_name(this->get_client_handle().get()); } std::shared_ptr diff --git a/rclcpp/src/rclcpp/executor.cpp b/rclcpp/src/rclcpp/executor.cpp index 5cd79e9..13b2bd8 100644 --- a/rclcpp/src/rclcpp/executor.cpp +++ b/rclcpp/src/rclcpp/executor.cpp @@ -345,7 +345,7 @@ Executor::execute_service( RCUTILS_LOG_ERROR_NAMED( "rclcpp", "take request failed for server of service '%s': %s", - service->get_service_name().c_str(), rcl_get_error_string_safe()); + service->get_service_name(), rcl_get_error_string_safe()); rcl_reset_error(); } } @@ -366,7 +366,7 @@ Executor::execute_client( RCUTILS_LOG_ERROR_NAMED( "rclcpp", "take response failed for client of service '%s': %s", - client->get_service_name().c_str(), rcl_get_error_string_safe()); + client->get_service_name(), rcl_get_error_string_safe()); rcl_reset_error(); } } diff --git a/rclcpp/src/rclcpp/service.cpp b/rclcpp/src/rclcpp/service.cpp index 6ea5030..c7f8ff9 100644 --- a/rclcpp/src/rclcpp/service.cpp +++ b/rclcpp/src/rclcpp/service.cpp @@ -27,12 +27,6 @@ using rclcpp::ServiceBase; -ServiceBase::ServiceBase( - std::shared_ptr node_handle, - const std::string & service_name) -: node_handle_(node_handle), service_name_(service_name) -{} - ServiceBase::ServiceBase(std::shared_ptr node_handle) : node_handle_(node_handle) {} @@ -40,10 +34,10 @@ ServiceBase::ServiceBase(std::shared_ptr node_handle) ServiceBase::~ServiceBase() {} -std::string +const char * ServiceBase::get_service_name() { - return this->service_name_; + return rcl_service_get_service_name(this->get_service_handle().get()); } std::shared_ptr