From 4d1054964550a53bbcdae67a22d456869b2e7343 Mon Sep 17 00:00:00 2001 From: Emerson Knapp Date: Wed, 3 Apr 2019 15:10:14 -0700 Subject: [PATCH] Options-based create_publisher and create_subscription interfaces Introduce new Options structs for creating publishers and subscribers. Deprecate existing interfaces for checking in CI how often they are used. Signed-off-by: Emerson Knapp --- rclcpp/CMakeLists.txt | 9 ++ .../include/rclcpp/intra_process_setting.hpp | 34 +++++ rclcpp/include/rclcpp/node.hpp | 63 +++++++-- rclcpp/include/rclcpp/node_impl.hpp | 124 ++++++++++++------ rclcpp/include/rclcpp/publisher_options.hpp | 39 ++++++ .../include/rclcpp/subscription_options.hpp | 41 ++++++ rclcpp/test/test_pub_sub_option_interface.cpp | 57 ++++++++ 7 files changed, 316 insertions(+), 51 deletions(-) create mode 100644 rclcpp/include/rclcpp/intra_process_setting.hpp create mode 100644 rclcpp/include/rclcpp/publisher_options.hpp create mode 100644 rclcpp/include/rclcpp/subscription_options.hpp create mode 100644 rclcpp/test/test_pub_sub_option_interface.cpp diff --git a/rclcpp/CMakeLists.txt b/rclcpp/CMakeLists.txt index 683792d384..798d8e60fd 100644 --- a/rclcpp/CMakeLists.txt +++ b/rclcpp/CMakeLists.txt @@ -248,6 +248,15 @@ if(BUILD_TESTING) ) target_link_libraries(test_publisher ${PROJECT_NAME}) endif() + ament_add_gtest(test_pub_sub_option_interface test/test_pub_sub_option_interface.cpp) + if(TARGET test_pub_sub_option_interface) + ament_target_dependencies(test_pub_sub_option_interface + test_msgs + ) + target_link_libraries(test_pub_sub_option_interface + ${PROJECT_NAME} + ) + endif() ament_add_gtest(test_publisher_subscription_count_api test/test_publisher_subscription_count_api.cpp) if(TARGET test_publisher_subscription_count_api) ament_target_dependencies(test_publisher_subscription_count_api diff --git a/rclcpp/include/rclcpp/intra_process_setting.hpp b/rclcpp/include/rclcpp/intra_process_setting.hpp new file mode 100644 index 0000000000..8e4b44eb64 --- /dev/null +++ b/rclcpp/include/rclcpp/intra_process_setting.hpp @@ -0,0 +1,34 @@ +// Copyright 2019 Open Source Robotics Foundation, Inc. +// +// 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 +// +// http://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. + +#ifndef RCLCPP__INTRA_PROCESS_SETTING_HPP_ +#define RCLCPP__INTRA_PROCESS_SETTING_HPP_ + +namespace rclcpp +{ + +/// Used as argument in create_publisher and create_subscriber. +enum class IntraProcessSetting +{ + /// Explicitly enable intraprocess comm at publisher/subscription level. + Enable, + /// Explicitly disable intraprocess comm at publisher/subscription level. + Disable, + /// Take intraprocess configuration from the node. + NodeDefault +}; + +} // namespace rclcpp + +#endif // RCLCPP__INTRA_PROCESS_SETTING_HPP_ diff --git a/rclcpp/include/rclcpp/node.hpp b/rclcpp/include/rclcpp/node.hpp index 2e34104e78..34e6fbf14d 100644 --- a/rclcpp/include/rclcpp/node.hpp +++ b/rclcpp/include/rclcpp/node.hpp @@ -54,8 +54,10 @@ #include "rclcpp/node_interfaces/node_waitables_interface.hpp" #include "rclcpp/parameter.hpp" #include "rclcpp/publisher.hpp" +#include "rclcpp/publisher_options.hpp" #include "rclcpp/service.hpp" #include "rclcpp/subscription.hpp" +#include "rclcpp/subscription_options.hpp" #include "rclcpp/subscription_traits.hpp" #include "rclcpp/time.hpp" #include "rclcpp/timer.hpp" @@ -64,17 +66,6 @@ namespace rclcpp { -/// Used as argument in create_publisher and create_subscriber. -enum class IntraProcessSetting -{ - /// Explicitly enable intraprocess comm at publisher/subscription level. - Enable, - /// Explicitly disable intraprocess comm at publisher/subscription level. - Disable, - /// Take intraprocess configuration from the node. - NodeDefault -}; - /// Node is the single point of entry for creating publishers and subscribers. class Node : public std::enable_shared_from_this { @@ -150,6 +141,23 @@ class Node : public std::enable_shared_from_this const std::vector & get_callback_groups() const; + /// Create and return a Publisher. + /** + * \param[in] topic_name The topic for this publisher to publish on. + * \param[in] qos_history_depth The depth of the publisher message queue. + * \param[in] options Additional options for the created Publisher. + * \return Shared pointer to the created publisher. + */ + template< + typename MessageT, + typename Alloc = std::allocator, + typename PublisherT = ::rclcpp::Publisher> + std::shared_ptr + create_publisher( + const std::string & topic_name, + size_t qos_history_depth, + const PublisherOptions & options = PublisherOptions()); + /// Create and return a Publisher. /** * \param[in] topic_name The topic for this publisher to publish on. @@ -160,6 +168,7 @@ class Node : public std::enable_shared_from_this template< typename MessageT, typename Alloc = std::allocator, typename PublisherT = ::rclcpp::Publisher> + [[deprecated]] std::shared_ptr create_publisher( const std::string & topic_name, size_t qos_history_depth, @@ -176,6 +185,7 @@ class Node : public std::enable_shared_from_this template< typename MessageT, typename Alloc = std::allocator, typename PublisherT = ::rclcpp::Publisher> + [[deprecated]] std::shared_ptr create_publisher( const std::string & topic_name, @@ -183,6 +193,35 @@ class Node : public std::enable_shared_from_this std::shared_ptr allocator = nullptr, IntraProcessSetting use_intra_process_comm = IntraProcessSetting::NodeDefault); + /// Create and return a Subscription. + /** + * \param[in] topic_name The topic to subscribe on. + * \param[in] callback The user-defined callback function to receive a message + * \param[in] qos_history_depth The depth of the subscription's incoming message queue. + * \param[in] options Additional options for the creation of the Subscription. + * \param[in] msg_mem_strat The message memory strategy to use for allocating messages. + * \return Shared pointer to the created subscription. + */ + /* TODO(jacquelinekay): + Windows build breaks when static member function passed as default + argument to msg_mem_strat, nullptr is a workaround. + */ + template< + typename MessageT, + typename CallbackT, + typename Alloc = std::allocator, + typename SubscriptionT = rclcpp::Subscription< + typename rclcpp::subscription_traits::has_message_type::type, Alloc>> + std::shared_ptr + create_subscription( + const std::string & topic_name, + CallbackT && callback, + size_t qos_history_depth, + const SubscriptionOptions & options = SubscriptionOptions(), + typename rclcpp::message_memory_strategy::MessageMemoryStrategy< + typename rclcpp::subscription_traits::has_message_type::type, Alloc>::SharedPtr + msg_mem_strat = nullptr); + /// Create and return a Subscription. /** * \param[in] topic_name The topic to subscribe on. @@ -204,6 +243,7 @@ class Node : public std::enable_shared_from_this typename Alloc = std::allocator, typename SubscriptionT = rclcpp::Subscription< typename rclcpp::subscription_traits::has_message_type::type, Alloc>> + [[deprecated]] std::shared_ptr create_subscription( const std::string & topic_name, @@ -238,6 +278,7 @@ class Node : public std::enable_shared_from_this typename Alloc = std::allocator, typename SubscriptionT = rclcpp::Subscription< typename rclcpp::subscription_traits::has_message_type::type, Alloc>> + [[deprecated]] std::shared_ptr create_subscription( const std::string & topic_name, diff --git a/rclcpp/include/rclcpp/node_impl.hpp b/rclcpp/include/rclcpp/node_impl.hpp index 0d35da8760..7ed8bea0ee 100644 --- a/rclcpp/include/rclcpp/node_impl.hpp +++ b/rclcpp/include/rclcpp/node_impl.hpp @@ -51,22 +51,6 @@ namespace rclcpp { -template -std::shared_ptr -Node::create_publisher( - const std::string & topic_name, size_t qos_history_depth, - std::shared_ptr allocator, - IntraProcessSetting use_intra_process_comm) -{ - if (!allocator) { - allocator = std::make_shared(); - } - rmw_qos_profile_t qos = rmw_qos_profile_default; - qos.depth = qos_history_depth; - return this->create_publisher(topic_name, qos, - allocator, use_intra_process_comm); -} - RCLCPP_LOCAL inline std::string @@ -82,15 +66,19 @@ extend_name_with_sub_namespace(const std::string & name, const std::string & sub template std::shared_ptr Node::create_publisher( - const std::string & topic_name, const rmw_qos_profile_t & qos_profile, - std::shared_ptr allocator, IntraProcessSetting use_intra_process_comm) + const std::string & topic_name, + size_t qos_history_depth, + const PublisherOptions & options) { + std::shared_ptr allocator = options.allocator; if (!allocator) { allocator = std::make_shared(); } + rmw_qos_profile_t qos_profile = options.qos_profile; + qos_profile.depth = qos_history_depth; bool use_intra_process; - switch (use_intra_process_comm) { + switch (options.use_intra_process_comm) { case IntraProcessSetting::Enable: use_intra_process = true; break; @@ -113,6 +101,35 @@ Node::create_publisher( allocator); } +template +std::shared_ptr +Node::create_publisher( + const std::string & topic_name, size_t qos_history_depth, + std::shared_ptr allocator, + IntraProcessSetting use_intra_process_comm) +{ + PublisherOptions pub_options; + pub_options.allocator = allocator; + pub_options.use_intra_process_comm = use_intra_process_comm; + return this->create_publisher( + topic_name, qos_history_depth, pub_options); +} + + +template +std::shared_ptr +Node::create_publisher( + const std::string & topic_name, const rmw_qos_profile_t & qos_profile, + std::shared_ptr allocator, IntraProcessSetting use_intra_process_comm) +{ + PublisherOptions pub_options; + pub_options.qos_profile = qos_profile; + pub_options.allocator = allocator; + pub_options.use_intra_process_comm = use_intra_process_comm; + return this->create_publisher( + topic_name, qos_profile.depth, pub_options); +} + template< typename MessageT, typename CallbackT, @@ -122,28 +139,29 @@ std::shared_ptr Node::create_subscription( const std::string & topic_name, CallbackT && callback, - const rmw_qos_profile_t & qos_profile, - rclcpp::callback_group::CallbackGroup::SharedPtr group, - bool ignore_local_publications, + size_t qos_history_depth, + const SubscriptionOptions & options, typename rclcpp::message_memory_strategy::MessageMemoryStrategy< typename rclcpp::subscription_traits::has_message_type::type, Alloc>::SharedPtr - msg_mem_strat, - std::shared_ptr allocator, - IntraProcessSetting use_intra_process_comm) + msg_mem_strat) { using CallbackMessageT = typename rclcpp::subscription_traits::has_message_type::type; + std::shared_ptr allocator = options.allocator; if (!allocator) { allocator = std::make_shared(); } + rmw_qos_profile_t qos_profile = options.qos_profile; + qos_profile.depth = qos_history_depth; + if (!msg_mem_strat) { using rclcpp::message_memory_strategy::MessageMemoryStrategy; msg_mem_strat = MessageMemoryStrategy::create_default(); } bool use_intra_process; - switch (use_intra_process_comm) { + switch (options.use_intra_process_comm) { case IntraProcessSetting::Enable: use_intra_process = true; break; @@ -163,8 +181,8 @@ Node::create_subscription( extend_name_with_sub_namespace(topic_name, this->get_sub_namespace()), std::forward(callback), qos_profile, - group, - ignore_local_publications, + options.callback_group, + options.ignore_local_publications, use_intra_process, msg_mem_strat, allocator); @@ -179,7 +197,7 @@ std::shared_ptr Node::create_subscription( const std::string & topic_name, CallbackT && callback, - size_t qos_history_depth, + const rmw_qos_profile_t & qos_profile, rclcpp::callback_group::CallbackGroup::SharedPtr group, bool ignore_local_publications, typename rclcpp::message_memory_strategy::MessageMemoryStrategy< @@ -188,18 +206,44 @@ Node::create_subscription( std::shared_ptr allocator, IntraProcessSetting use_intra_process_comm) { - rmw_qos_profile_t qos = rmw_qos_profile_default; - qos.depth = qos_history_depth; + SubscriptionOptions sub_options; + sub_options.qos_profile = qos_profile; + sub_options.callback_group = group; + sub_options.ignore_local_publications = ignore_local_publications; + sub_options.allocator = allocator; + sub_options.use_intra_process_comm = use_intra_process_comm; + + return this->create_subscription( + topic_name, std::forward(callback), qos_profile.depth, sub_options, msg_mem_strat); +} - return this->create_subscription( - topic_name, - std::forward(callback), - qos, - group, - ignore_local_publications, - msg_mem_strat, - allocator, - use_intra_process_comm); + +template< + typename MessageT, + typename CallbackT, + typename Alloc, + typename SubscriptionT> +std::shared_ptr +Node::create_subscription( + const std::string & topic_name, + CallbackT && callback, + size_t qos_history_depth, + rclcpp::callback_group::CallbackGroup::SharedPtr group, + bool ignore_local_publications, + typename rclcpp::message_memory_strategy::MessageMemoryStrategy< + typename rclcpp::subscription_traits::has_message_type::type, Alloc>::SharedPtr + msg_mem_strat, + std::shared_ptr allocator, + IntraProcessSetting use_intra_process_comm) +{ + SubscriptionOptions sub_options; + sub_options.callback_group = group; + sub_options.ignore_local_publications = ignore_local_publications; + sub_options.allocator = allocator; + sub_options.use_intra_process_comm = use_intra_process_comm; + + return this->create_subscription( + topic_name, std::forward(callback), qos_history_depth, sub_options, msg_mem_strat); } template diff --git a/rclcpp/include/rclcpp/publisher_options.hpp b/rclcpp/include/rclcpp/publisher_options.hpp new file mode 100644 index 0000000000..6c5f5e4d47 --- /dev/null +++ b/rclcpp/include/rclcpp/publisher_options.hpp @@ -0,0 +1,39 @@ +// Copyright 2019 Open Source Robotics Foundation, Inc. +// +// 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 +// +// http://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. + +#ifndef RCLCPP__PUBLISHER_OPTIONS_HPP_ +#define RCLCPP__PUBLISHER_OPTIONS_HPP_ + +#include +#include +#include + +#include "rclcpp/intra_process_setting.hpp" +#include "rclcpp/visibility_control.hpp" + +namespace rclcpp +{ + +/// Structure containing optional configuration for Publishers. +template> +struct PublisherOptions +{ + rmw_qos_profile_t qos_profile = rmw_qos_profile_default; + std::shared_ptr allocator = nullptr; + IntraProcessSetting use_intra_process_comm = IntraProcessSetting::NodeDefault; +}; + +} // namespace rclcpp + +#endif // RCLCPP__PUBLISHER_OPTIONS_HPP_ diff --git a/rclcpp/include/rclcpp/subscription_options.hpp b/rclcpp/include/rclcpp/subscription_options.hpp new file mode 100644 index 0000000000..2152d91e56 --- /dev/null +++ b/rclcpp/include/rclcpp/subscription_options.hpp @@ -0,0 +1,41 @@ +// Copyright 2019 Open Source Robotics Foundation, Inc. +// +// 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 +// +// http://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. + +#ifndef RCLCPP__SUBSCRIPTION_OPTIONS_HPP_ +#define RCLCPP__SUBSCRIPTION_OPTIONS_HPP_ + +#include +#include +#include + +#include "rclcpp/intra_process_setting.hpp" +#include "rclcpp/visibility_control.hpp" + +namespace rclcpp +{ + +/// Structure containing optional configuration for Subscriptions. +template> +struct SubscriptionOptions +{ + rmw_qos_profile_t qos_profile = rmw_qos_profile_default; + bool ignore_local_publications = false; + rclcpp::callback_group::CallbackGroup::SharedPtr callback_group = nullptr; + std::shared_ptr allocator = nullptr; + IntraProcessSetting use_intra_process_comm = IntraProcessSetting::NodeDefault; +}; + +} // namespace rclcpp + +#endif // RCLCPP__SUBSCRIPTION_OPTIONS_HPP_ diff --git a/rclcpp/test/test_pub_sub_option_interface.cpp b/rclcpp/test/test_pub_sub_option_interface.cpp new file mode 100644 index 0000000000..35d17f7c54 --- /dev/null +++ b/rclcpp/test/test_pub_sub_option_interface.cpp @@ -0,0 +1,57 @@ +// Copyright 2019 Open Source Robotics Foundation, Inc. +// +// 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 +// +// http://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 + +#include +#include + +#include "rclcpp/rclcpp.hpp" +#include "test_msgs/msg/empty.hpp" + +class TestPublisher : public ::testing::Test +{ +protected: + static void SetUpTestCase() + { + rclcpp::init(0, nullptr); + } + + void SetUp() + { + node = std::make_shared("my_node", "/ns"); + } + + void TearDown() + { + node.reset(); + } + + rclcpp::Node::SharedPtr node; +}; + +/* + Testing construction and destruction. + */ +TEST_F(TestPublisher, construction_and_destruction) { + rclcpp::PublisherOptions<> pub_options; + auto publisher = node->create_publisher("topic", 5, pub_options); + + rclcpp::SubscriptionOptions<> sub_options; + auto subscription = node->create_subscription( + "topic", + [](std::shared_ptr test_msg) {(void) test_msg;}, + 5, + sub_options); +}