From 190f3076ce2569e44a994547c51be948041132c0 Mon Sep 17 00:00:00 2001 From: Emerson Knapp <537409+emersonknapp@users.noreply.github.com> Date: Fri, 7 Jul 2023 10:10:27 -0700 Subject: [PATCH 1/5] Add new node interface TypeDescriptionsInterface to provide GetTypeDescription service (#2224) * TypeDescriptions interface with readonly param configuration * Add parameter descriptor, to make read only * example of spinning in thread for get_type_description service * Add a basic test for the new interface * Fix tests with new parameter * Add comments about builtin parameters Signed-off-by: Emerson Knapp Signed-off-by: William Woodall --- rclcpp/CMakeLists.txt | 1 + rclcpp/include/rclcpp/node.hpp | 7 + .../node_interfaces/node_interfaces.hpp | 2 + .../node_type_descriptions.hpp | 63 +++++++ .../node_type_descriptions_interface.hpp | 44 +++++ rclcpp/src/rclcpp/node.cpp | 13 ++ .../src/rclcpp/node_interfaces/node_base.cpp | 1 + .../node_type_descriptions.cpp | 157 ++++++++++++++++++ rclcpp/test/rclcpp/CMakeLists.txt | 5 + .../node_interfaces/test_node_parameters.cpp | 4 +- .../test_node_type_descriptions.cpp | 63 +++++++ rclcpp/test/rclcpp/test_parameter_client.cpp | 14 +- .../rclcpp_lifecycle/lifecycle_node.hpp | 6 + rclcpp_lifecycle/src/lifecycle_node.cpp | 7 + rclcpp_lifecycle/test/test_lifecycle_node.cpp | 30 ++-- 15 files changed, 400 insertions(+), 17 deletions(-) create mode 100644 rclcpp/include/rclcpp/node_interfaces/node_type_descriptions.hpp create mode 100644 rclcpp/include/rclcpp/node_interfaces/node_type_descriptions_interface.hpp create mode 100644 rclcpp/src/rclcpp/node_interfaces/node_type_descriptions.cpp create mode 100644 rclcpp/test/rclcpp/node_interfaces/test_node_type_descriptions.cpp diff --git a/rclcpp/CMakeLists.txt b/rclcpp/CMakeLists.txt index b2d1023064..bd705be06d 100644 --- a/rclcpp/CMakeLists.txt +++ b/rclcpp/CMakeLists.txt @@ -92,6 +92,7 @@ set(${PROJECT_NAME}_SRCS src/rclcpp/node_interfaces/node_time_source.cpp src/rclcpp/node_interfaces/node_timers.cpp src/rclcpp/node_interfaces/node_topics.cpp + src/rclcpp/node_interfaces/node_type_descriptions.cpp src/rclcpp/node_interfaces/node_waitables.cpp src/rclcpp/node_options.cpp src/rclcpp/parameter.cpp diff --git a/rclcpp/include/rclcpp/node.hpp b/rclcpp/include/rclcpp/node.hpp index 7ecb67e9b1..9193e3f3f9 100644 --- a/rclcpp/include/rclcpp/node.hpp +++ b/rclcpp/include/rclcpp/node.hpp @@ -56,6 +56,7 @@ #include "rclcpp/node_interfaces/node_time_source_interface.hpp" #include "rclcpp/node_interfaces/node_timers_interface.hpp" #include "rclcpp/node_interfaces/node_topics_interface.hpp" +#include "rclcpp/node_interfaces/node_type_descriptions_interface.hpp" #include "rclcpp/node_interfaces/node_waitables_interface.hpp" #include "rclcpp/node_options.hpp" #include "rclcpp/parameter.hpp" @@ -1454,6 +1455,11 @@ class Node : public std::enable_shared_from_this rclcpp::node_interfaces::NodeTimeSourceInterface::SharedPtr get_node_time_source_interface(); + /// Return the Node's internal NodeTypeDescriptionsInterface implementation. + RCLCPP_PUBLIC + rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr + get_node_type_descriptions_interface(); + /// Return the sub-namespace, if this is a sub-node, otherwise an empty string. /** * The returned sub-namespace is either the accumulated sub-namespaces which @@ -1586,6 +1592,7 @@ class Node : public std::enable_shared_from_this rclcpp::node_interfaces::NodeClockInterface::SharedPtr node_clock_; rclcpp::node_interfaces::NodeParametersInterface::SharedPtr node_parameters_; rclcpp::node_interfaces::NodeTimeSourceInterface::SharedPtr node_time_source_; + rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr node_type_descriptions_; rclcpp::node_interfaces::NodeWaitablesInterface::SharedPtr node_waitables_; const rclcpp::NodeOptions node_options_; diff --git a/rclcpp/include/rclcpp/node_interfaces/node_interfaces.hpp b/rclcpp/include/rclcpp/node_interfaces/node_interfaces.hpp index bb4886d592..a7c7b7af96 100644 --- a/rclcpp/include/rclcpp/node_interfaces/node_interfaces.hpp +++ b/rclcpp/include/rclcpp/node_interfaces/node_interfaces.hpp @@ -30,6 +30,7 @@ rclcpp::node_interfaces::NodeTimeSourceInterface, \ rclcpp::node_interfaces::NodeTimersInterface, \ rclcpp::node_interfaces::NodeTopicsInterface, \ + rclcpp::node_interfaces::NodeTypeDescriptionsInterface, \ rclcpp::node_interfaces::NodeWaitablesInterface @@ -118,6 +119,7 @@ class NodeInterfaces * - rclcpp::node_interfaces::NodeTimeSourceInterface * - rclcpp::node_interfaces::NodeTimersInterface * - rclcpp::node_interfaces::NodeTopicsInterface + * - rclcpp::node_interfaces::NodeTypeDescriptionsInterface * - rclcpp::node_interfaces::NodeWaitablesInterface * * Or you use custom interfaces as long as you make a template specialization diff --git a/rclcpp/include/rclcpp/node_interfaces/node_type_descriptions.hpp b/rclcpp/include/rclcpp/node_interfaces/node_type_descriptions.hpp new file mode 100644 index 0000000000..8aa563bba2 --- /dev/null +++ b/rclcpp/include/rclcpp/node_interfaces/node_type_descriptions.hpp @@ -0,0 +1,63 @@ +// Copyright 2023 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__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_HPP_ +#define RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_HPP_ + +#include + +#include "rclcpp/macros.hpp" +#include "rclcpp/node_interfaces/node_base_interface.hpp" +#include "rclcpp/node_interfaces/node_logging_interface.hpp" +#include "rclcpp/node_interfaces/node_parameters_interface.hpp" +#include "rclcpp/node_interfaces/node_services_interface.hpp" +#include "rclcpp/node_interfaces/node_topics_interface.hpp" +#include "rclcpp/node_interfaces/node_type_descriptions_interface.hpp" +#include "rclcpp/visibility_control.hpp" + +namespace rclcpp +{ +namespace node_interfaces +{ + +/// Implementation of the NodeTypeDescriptions part of the Node API. +class NodeTypeDescriptions : public NodeTypeDescriptionsInterface +{ +public: + RCLCPP_SMART_PTR_ALIASES_ONLY(NodeTypeDescriptions) + + RCLCPP_PUBLIC + explicit NodeTypeDescriptions( + NodeBaseInterface::SharedPtr node_base, + NodeLoggingInterface::SharedPtr node_logging, + NodeParametersInterface::SharedPtr node_parameters, + NodeServicesInterface::SharedPtr node_services); + + RCLCPP_PUBLIC + virtual + ~NodeTypeDescriptions(); + +private: + RCLCPP_DISABLE_COPY(NodeTypeDescriptions) + + // Pimpl hides helper types and functions used for wrapping a C service, which would be + // awkward to expose in this header. + class NodeTypeDescriptionsImpl; + std::unique_ptr impl_; +}; + +} // namespace node_interfaces +} // namespace rclcpp + +#endif // RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_HPP_ diff --git a/rclcpp/include/rclcpp/node_interfaces/node_type_descriptions_interface.hpp b/rclcpp/include/rclcpp/node_interfaces/node_type_descriptions_interface.hpp new file mode 100644 index 0000000000..e7e0b0af2e --- /dev/null +++ b/rclcpp/include/rclcpp/node_interfaces/node_type_descriptions_interface.hpp @@ -0,0 +1,44 @@ +// Copyright 2023 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__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_INTERFACE_HPP_ +#define RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_INTERFACE_HPP_ + +#include "rclcpp/macros.hpp" +#include "rclcpp/node_interfaces/detail/node_interfaces_helpers.hpp" +#include "rclcpp/visibility_control.hpp" + +namespace rclcpp +{ +namespace node_interfaces +{ + +/// Pure virtual interface class for the NodeTypeDescriptions part of the Node API. +class NodeTypeDescriptionsInterface +{ +public: + RCLCPP_SMART_PTR_ALIASES_ONLY(NodeTypeDescriptionsInterface) + + RCLCPP_PUBLIC + virtual + ~NodeTypeDescriptionsInterface() = default; +}; + +} // namespace node_interfaces +} // namespace rclcpp + +RCLCPP_NODE_INTERFACE_HELPERS_SUPPORT( + rclcpp::node_interfaces::NodeTypeDescriptionsInterface, type_descriptions) + +#endif // RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_INTERFACE_HPP_ diff --git a/rclcpp/src/rclcpp/node.cpp b/rclcpp/src/rclcpp/node.cpp index 53e77dd796..52012439e8 100644 --- a/rclcpp/src/rclcpp/node.cpp +++ b/rclcpp/src/rclcpp/node.cpp @@ -36,6 +36,7 @@ #include "rclcpp/node_interfaces/node_time_source.hpp" #include "rclcpp/node_interfaces/node_timers.hpp" #include "rclcpp/node_interfaces/node_topics.hpp" +#include "rclcpp/node_interfaces/node_type_descriptions.hpp" #include "rclcpp/node_interfaces/node_waitables.hpp" #include "rclcpp/qos_overriding_options.hpp" @@ -206,6 +207,12 @@ Node::Node( options.clock_qos(), options.use_clock_thread() )), + node_type_descriptions_(new rclcpp::node_interfaces::NodeTypeDescriptions( + node_base_, + node_logging_, + node_parameters_, + node_services_ + )), node_waitables_(new rclcpp::node_interfaces::NodeWaitables(node_base_.get())), node_options_(options), sub_namespace_(""), @@ -591,6 +598,12 @@ Node::get_node_topics_interface() return node_topics_; } +rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr +Node::get_node_type_descriptions_interface() +{ + return node_type_descriptions_; +} + rclcpp::node_interfaces::NodeServicesInterface::SharedPtr Node::get_node_services_interface() { diff --git a/rclcpp/src/rclcpp/node_interfaces/node_base.cpp b/rclcpp/src/rclcpp/node_interfaces/node_base.cpp index 6544d69214..36e1afb932 100644 --- a/rclcpp/src/rclcpp/node_interfaces/node_base.cpp +++ b/rclcpp/src/rclcpp/node_interfaces/node_base.cpp @@ -20,6 +20,7 @@ #include "rclcpp/node_interfaces/node_base.hpp" #include "rcl/arguments.h" +#include "rcl/node_type_cache.h" #include "rclcpp/exceptions.hpp" #include "rcutils/logging_macros.h" #include "rmw/validate_namespace.h" diff --git a/rclcpp/src/rclcpp/node_interfaces/node_type_descriptions.cpp b/rclcpp/src/rclcpp/node_interfaces/node_type_descriptions.cpp new file mode 100644 index 0000000000..f4b5e20d30 --- /dev/null +++ b/rclcpp/src/rclcpp/node_interfaces/node_type_descriptions.cpp @@ -0,0 +1,157 @@ +// Copyright 2023 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/node_interfaces/node_type_descriptions.hpp" +#include "rclcpp/parameter_client.hpp" + +#include "type_description_interfaces/srv/get_type_description.h" + +namespace +{ +// Helper wrapper for rclcpp::Service to access ::Request and ::Response types for allocation. +struct GetTypeDescription__C +{ + using Request = type_description_interfaces__srv__GetTypeDescription_Request; + using Response = type_description_interfaces__srv__GetTypeDescription_Response; + using Event = type_description_interfaces__srv__GetTypeDescription_Event; +}; +} // namespace + +// Helper function for C typesupport. +namespace rosidl_typesupport_cpp +{ +template<> +rosidl_service_type_support_t const * +get_service_type_support_handle() +{ + return ROSIDL_GET_SRV_TYPE_SUPPORT(type_description_interfaces, srv, GetTypeDescription); +} +} // namespace rosidl_typesupport_cpp + +namespace rclcpp +{ +namespace node_interfaces +{ + +class NodeTypeDescriptions::NodeTypeDescriptionsImpl +{ +public: + using ServiceT = GetTypeDescription__C; + + rclcpp::Logger logger_; + rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base_; + rclcpp::Service::SharedPtr type_description_srv_; + + NodeTypeDescriptionsImpl( + rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base, + rclcpp::node_interfaces::NodeLoggingInterface::SharedPtr node_logging, + rclcpp::node_interfaces::NodeParametersInterface::SharedPtr node_parameters, + rclcpp::node_interfaces::NodeServicesInterface::SharedPtr node_services) + : logger_(node_logging->get_logger()), + node_base_(node_base) + { + const std::string enable_param_name = "start_type_description_service"; + + bool enabled = false; + try { + auto enable_param = node_parameters->declare_parameter( + enable_param_name, + rclcpp::ParameterValue(true), + rcl_interfaces::msg::ParameterDescriptor() + .set__name(enable_param_name) + .set__type(rclcpp::PARAMETER_BOOL) + .set__description("Start the ~/get_type_description service for this node.") + .set__read_only(true)); + enabled = enable_param.get(); + } catch (const rclcpp::exceptions::InvalidParameterTypeException & exc) { + RCLCPP_ERROR(logger_, "%s", exc.what()); + throw; + } + + if (enabled) { + auto rcl_node = node_base->get_rcl_node_handle(); + rcl_ret_t rcl_ret = rcl_node_type_description_service_init(rcl_node); + if (rcl_ret != RCL_RET_OK) { + RCLCPP_ERROR( + logger_, "Failed to initialize ~/get_type_description_service: %s", + rcl_get_error_string().str); + throw std::runtime_error( + "Failed to initialize ~/get_type_description service."); + } + + rcl_service_t * rcl_srv = nullptr; + rcl_ret = rcl_node_get_type_description_service(rcl_node, &rcl_srv); + if (rcl_ret != RCL_RET_OK) { + throw std::runtime_error( + "Failed to get initialized ~/get_type_description service from rcl."); + } + + rclcpp::AnyServiceCallback cb; + cb.set( + [this]( + std::shared_ptr header, + std::shared_ptr request, + std::shared_ptr response + ) { + rcl_node_type_description_service_handle_request( + node_base_->get_rcl_node_handle(), + header.get(), + request.get(), + response.get()); + }); + + type_description_srv_ = std::make_shared>( + node_base_->get_shared_rcl_node_handle(), + rcl_srv, + cb); + node_services->add_service( + std::dynamic_pointer_cast(type_description_srv_), + nullptr); + } + } + + ~NodeTypeDescriptionsImpl() + { + if ( + type_description_srv_ && + RCL_RET_OK != rcl_node_type_description_service_fini(node_base_->get_rcl_node_handle())) + { + RCLCPP_ERROR( + logger_, + "Error in shutdown of get_type_description service: %s", rcl_get_error_string().str); + } + } +}; + +NodeTypeDescriptions::NodeTypeDescriptions( + rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base, + rclcpp::node_interfaces::NodeLoggingInterface::SharedPtr node_logging, + rclcpp::node_interfaces::NodeParametersInterface::SharedPtr node_parameters, + rclcpp::node_interfaces::NodeServicesInterface::SharedPtr node_services) +: impl_(new NodeTypeDescriptionsImpl( + node_base, + node_logging, + node_parameters, + node_services)) +{} + +NodeTypeDescriptions::~NodeTypeDescriptions() +{} + +} // namespace node_interfaces +} // namespace rclcpp diff --git a/rclcpp/test/rclcpp/CMakeLists.txt b/rclcpp/test/rclcpp/CMakeLists.txt index dcb1d36d8e..8c31a95415 100644 --- a/rclcpp/test/rclcpp/CMakeLists.txt +++ b/rclcpp/test/rclcpp/CMakeLists.txt @@ -262,6 +262,11 @@ if(TARGET test_node_interfaces__node_topics) "test_msgs") target_link_libraries(test_node_interfaces__node_topics ${PROJECT_NAME} mimick) endif() +ament_add_gtest(test_node_interfaces__node_type_descriptions + node_interfaces/test_node_type_descriptions.cpp) +if(TARGET test_node_interfaces__node_type_descriptions) + target_link_libraries(test_node_interfaces__node_type_descriptions ${PROJECT_NAME} mimick) +endif() ament_add_gtest(test_node_interfaces__node_waitables node_interfaces/test_node_waitables.cpp) if(TARGET test_node_interfaces__node_waitables) diff --git a/rclcpp/test/rclcpp/node_interfaces/test_node_parameters.cpp b/rclcpp/test/rclcpp/node_interfaces/test_node_parameters.cpp index 58bf010767..9113c96ca5 100644 --- a/rclcpp/test/rclcpp/node_interfaces/test_node_parameters.cpp +++ b/rclcpp/test/rclcpp/node_interfaces/test_node_parameters.cpp @@ -77,9 +77,9 @@ TEST_F(TestNodeParameters, list_parameters) std::vector prefixes; const auto list_result = node_parameters->list_parameters(prefixes, 1u); - // Currently the only default parameter is 'use_sim_time', but that may change. + // Currently the default parameters are 'use_sim_time' and 'start_type_description_service' size_t number_of_parameters = list_result.names.size(); - EXPECT_GE(1u, number_of_parameters); + EXPECT_GE(2u, number_of_parameters); const std::string parameter_name = "new_parameter"; const rclcpp::ParameterValue value(true); diff --git a/rclcpp/test/rclcpp/node_interfaces/test_node_type_descriptions.cpp b/rclcpp/test/rclcpp/node_interfaces/test_node_type_descriptions.cpp new file mode 100644 index 0000000000..79ef6c3bcf --- /dev/null +++ b/rclcpp/test/rclcpp/node_interfaces/test_node_type_descriptions.cpp @@ -0,0 +1,63 @@ +// Copyright 2023 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 "rclcpp/node.hpp" +#include "rclcpp/node_interfaces/node_type_descriptions.hpp" + +class TestNodeTypeDescriptions : public ::testing::Test +{ +public: + void SetUp() + { + rclcpp::init(0, nullptr); + } + + void TearDown() + { + rclcpp::shutdown(); + } +}; + +TEST_F(TestNodeTypeDescriptions, interface_created) +{ + rclcpp::Node node{"node", "ns"}; + ASSERT_NE(nullptr, node.get_node_type_descriptions_interface()); +} + +TEST_F(TestNodeTypeDescriptions, disabled_no_service) +{ + rclcpp::NodeOptions node_options; + node_options.append_parameter_override("start_type_description_service", false); + rclcpp::Node node{"node", "ns", node_options}; + + rcl_node_t * rcl_node = node.get_node_base_interface()->get_rcl_node_handle(); + rcl_service_t * srv = nullptr; + rcl_ret_t ret = rcl_node_get_type_description_service(rcl_node, &srv); + ASSERT_EQ(RCL_RET_NOT_INIT, ret); +} + +TEST_F(TestNodeTypeDescriptions, enabled_creates_service) +{ + rclcpp::NodeOptions node_options; + node_options.append_parameter_override("start_type_description_service", true); + rclcpp::Node node{"node", "ns", node_options}; + + rcl_node_t * rcl_node = node.get_node_base_interface()->get_rcl_node_handle(); + rcl_service_t * srv = nullptr; + rcl_ret_t ret = rcl_node_get_type_description_service(rcl_node, &srv); + ASSERT_EQ(RCL_RET_OK, ret); + ASSERT_NE(nullptr, srv); +} diff --git a/rclcpp/test/rclcpp/test_parameter_client.cpp b/rclcpp/test/rclcpp/test_parameter_client.cpp index 2ce414d327..afc48a652b 100644 --- a/rclcpp/test/rclcpp/test_parameter_client.cpp +++ b/rclcpp/test/rclcpp/test_parameter_client.cpp @@ -59,6 +59,8 @@ class TestParameterClient : public ::testing::Test node_with_option.reset(); } + // "start_type_description_service" and "use_sim_time" + const uint64_t builtin_param_count = 2; rclcpp::Node::SharedPtr node; rclcpp::Node::SharedPtr node_with_option; }; @@ -925,6 +927,7 @@ TEST_F(TestParameterClient, sync_parameter_delete_parameters) { Coverage for async load_parameters */ TEST_F(TestParameterClient, async_parameter_load_parameters) { + const uint64_t expected_param_count = 4 + builtin_param_count; auto load_node = std::make_shared( "load_node", "namespace", @@ -944,12 +947,13 @@ TEST_F(TestParameterClient, async_parameter_load_parameters) { auto list_parameters = asynchronous_client->list_parameters({}, 3); rclcpp::spin_until_future_complete( load_node, list_parameters, std::chrono::milliseconds(100)); - ASSERT_EQ(list_parameters.get().names.size(), static_cast(5)); + ASSERT_EQ(list_parameters.get().names.size(), expected_param_count); } /* Coverage for sync load_parameters */ TEST_F(TestParameterClient, sync_parameter_load_parameters) { + const uint64_t expected_param_count = 4 + builtin_param_count; auto load_node = std::make_shared( "load_node", "namespace", @@ -964,13 +968,14 @@ TEST_F(TestParameterClient, sync_parameter_load_parameters) { ASSERT_EQ(load_future[0].successful, true); // list parameters auto list_parameters = synchronous_client->list_parameters({}, 3); - ASSERT_EQ(list_parameters.names.size(), static_cast(5)); + ASSERT_EQ(list_parameters.names.size(), static_cast(expected_param_count)); } /* Coverage for async load_parameters with complicated regex expression */ TEST_F(TestParameterClient, async_parameter_load_parameters_complicated_regex) { + const uint64_t expected_param_count = 5 + builtin_param_count; auto load_node = std::make_shared( "load_node", "namespace", @@ -990,7 +995,7 @@ TEST_F(TestParameterClient, async_parameter_load_parameters_complicated_regex) { auto list_parameters = asynchronous_client->list_parameters({}, 3); rclcpp::spin_until_future_complete( load_node, list_parameters, std::chrono::milliseconds(100)); - ASSERT_EQ(list_parameters.get().names.size(), static_cast(6)); + ASSERT_EQ(list_parameters.get().names.size(), expected_param_count); // to check the parameter "a_value" std::string param_name = "a_value"; auto param = load_node->get_parameter(param_name); @@ -1020,6 +1025,7 @@ TEST_F(TestParameterClient, async_parameter_load_no_valid_parameter) { Coverage for async load_parameters from maps with complicated regex expression */ TEST_F(TestParameterClient, async_parameter_load_parameters_from_map) { + const uint64_t expected_param_count = 5 + builtin_param_count; auto load_node = std::make_shared( "load_node", "namespace", @@ -1068,7 +1074,7 @@ TEST_F(TestParameterClient, async_parameter_load_parameters_from_map) { auto list_parameters = asynchronous_client->list_parameters({}, 3); rclcpp::spin_until_future_complete( load_node, list_parameters, std::chrono::milliseconds(100)); - ASSERT_EQ(list_parameters.get().names.size(), static_cast(6)); + ASSERT_EQ(list_parameters.get().names.size(), expected_param_count); // to check the parameter "a_value" std::string param_name = "a_value"; auto param = load_node->get_parameter(param_name); diff --git a/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp b/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp index 197ecf5c19..d3654ab2b2 100644 --- a/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp +++ b/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp @@ -72,6 +72,7 @@ #include "rclcpp/node_interfaces/node_time_source_interface.hpp" #include "rclcpp/node_interfaces/node_timers_interface.hpp" #include "rclcpp/node_interfaces/node_topics_interface.hpp" +#include "rclcpp/node_interfaces/node_type_descriptions_interface.hpp" #include "rclcpp/node_interfaces/node_waitables_interface.hpp" #include "rclcpp/parameter.hpp" #include "rclcpp/publisher.hpp" @@ -823,6 +824,10 @@ class LifecycleNode : public node_interfaces::LifecycleNodeInterface, rclcpp::node_interfaces::NodeTimeSourceInterface::SharedPtr get_node_time_source_interface(); + RCLCPP_LIFECYCLE_PUBLIC + rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr + get_node_type_descriptions_interface(); + /// Return the Node's internal NodeWaitablesInterface implementation. /** * \sa rclcpp::Node::get_node_waitables_interface @@ -1085,6 +1090,7 @@ class LifecycleNode : public node_interfaces::LifecycleNodeInterface, rclcpp::node_interfaces::NodeClockInterface::SharedPtr node_clock_; rclcpp::node_interfaces::NodeParametersInterface::SharedPtr node_parameters_; rclcpp::node_interfaces::NodeTimeSourceInterface::SharedPtr node_time_source_; + rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr node_type_descriptions_; rclcpp::node_interfaces::NodeWaitablesInterface::SharedPtr node_waitables_; const rclcpp::NodeOptions node_options_; diff --git a/rclcpp_lifecycle/src/lifecycle_node.cpp b/rclcpp_lifecycle/src/lifecycle_node.cpp index 4c0b94cb42..94629911a3 100644 --- a/rclcpp_lifecycle/src/lifecycle_node.cpp +++ b/rclcpp_lifecycle/src/lifecycle_node.cpp @@ -43,6 +43,7 @@ #include "rclcpp/node_interfaces/node_time_source.hpp" #include "rclcpp/node_interfaces/node_timers.hpp" #include "rclcpp/node_interfaces/node_topics.hpp" +#include "rclcpp/node_interfaces/node_type_descriptions.hpp" #include "rclcpp/node_interfaces/node_waitables.hpp" #include "rclcpp/parameter_service.hpp" #include "rclcpp/qos.hpp" @@ -113,6 +114,12 @@ LifecycleNode::LifecycleNode( options.clock_qos(), options.use_clock_thread() )), + node_type_descriptions_(new rclcpp::node_interfaces::NodeTypeDescriptions( + node_base_, + node_logging_, + node_parameters_, + node_services_ + )), node_waitables_(new rclcpp::node_interfaces::NodeWaitables(node_base_.get())), node_options_(options), impl_(new LifecycleNodeInterfaceImpl(node_base_, node_services_)) diff --git a/rclcpp_lifecycle/test/test_lifecycle_node.cpp b/rclcpp_lifecycle/test/test_lifecycle_node.cpp index 31850c589b..db24bd0ee7 100644 --- a/rclcpp_lifecycle/test/test_lifecycle_node.cpp +++ b/rclcpp_lifecycle/test/test_lifecycle_node.cpp @@ -427,11 +427,15 @@ TEST_F(TestDefaultStateMachine, lifecycle_subscriber) { // Parameters are tested more thoroughly in rclcpp's test_node.cpp // These are provided for coverage of lifecycle node's API TEST_F(TestDefaultStateMachine, declare_parameters) { + // "start_type_description_service" and "use_sim_time" + const uint64_t builtin_param_count = 2; + const uint64_t expected_param_count = 6 + builtin_param_count; auto test_node = std::make_shared("testnode"); auto list_result = test_node->list_parameters({}, 0u); - EXPECT_EQ(list_result.names.size(), 1u); - EXPECT_STREQ(list_result.names[0].c_str(), "use_sim_time"); + EXPECT_EQ(list_result.names.size(), builtin_param_count); + EXPECT_STREQ(list_result.names[0].c_str(), "start_type_description_service"); + EXPECT_STREQ(list_result.names[1].c_str(), "use_sim_time"); const std::string bool_name = "test_boolean"; const std::string int_name = "test_int"; @@ -469,10 +473,11 @@ TEST_F(TestDefaultStateMachine, declare_parameters) { test_node->declare_parameters("test_double", double_parameters); list_result = test_node->list_parameters({}, 0u); - EXPECT_EQ(list_result.names.size(), 7u); + EXPECT_EQ(list_result.names.size(), expected_param_count); // The order of these names is not controlled by lifecycle_node, doing set equality std::set expected_names = { + "start_type_description_service", "test_boolean", "test_double.double_one", "test_double.double_two", @@ -487,11 +492,13 @@ TEST_F(TestDefaultStateMachine, declare_parameters) { } TEST_F(TestDefaultStateMachine, check_parameters) { + const uint64_t builtin_param_count = 2; auto test_node = std::make_shared("testnode"); auto list_result = test_node->list_parameters({}, 0u); - EXPECT_EQ(list_result.names.size(), 1u); - EXPECT_STREQ(list_result.names[0].c_str(), "use_sim_time"); + EXPECT_EQ(list_result.names.size(), builtin_param_count); + EXPECT_STREQ(list_result.names[0].c_str(), "start_type_description_service"); + EXPECT_STREQ(list_result.names[1].c_str(), "use_sim_time"); const std::string bool_name = "test_boolean"; const std::string int_name = "test_int"; @@ -549,8 +556,7 @@ TEST_F(TestDefaultStateMachine, check_parameters) { std::map parameter_map; EXPECT_TRUE(test_node->get_parameters({}, parameter_map)); - // int param, bool param, and use_sim_time - EXPECT_EQ(parameter_map.size(), 3u); + EXPECT_EQ(parameter_map.size(), parameter_names.size() + builtin_param_count); // Check parameter types auto parameter_types = test_node->get_parameter_types(parameter_names); @@ -585,10 +591,12 @@ TEST_F(TestDefaultStateMachine, check_parameters) { // List parameters list_result = test_node->list_parameters({}, 0u); - EXPECT_EQ(list_result.names.size(), 3u); - EXPECT_STREQ(list_result.names[0].c_str(), parameter_names[0].c_str()); - EXPECT_STREQ(list_result.names[1].c_str(), parameter_names[1].c_str()); - EXPECT_STREQ(list_result.names[2].c_str(), "use_sim_time"); + EXPECT_EQ(list_result.names.size(), parameter_names.size() + builtin_param_count); + size_t index = 0; + EXPECT_STREQ(list_result.names[index++].c_str(), "start_type_description_service"); + EXPECT_STREQ(list_result.names[index++].c_str(), parameter_names[0].c_str()); + EXPECT_STREQ(list_result.names[index++].c_str(), parameter_names[1].c_str()); + EXPECT_STREQ(list_result.names[index++].c_str(), "use_sim_time"); // Undeclare parameter test_node->undeclare_parameter(bool_name); From 75e180c808aeb13fe00d824086132c5a13f01507 Mon Sep 17 00:00:00 2001 From: Emerson Knapp Date: Fri, 7 Jul 2023 15:19:07 -0700 Subject: [PATCH 2/5] Hide new member for ABI stability Signed-off-by: Emerson Knapp --- rclcpp/include/rclcpp/node.hpp | 4 +- rclcpp/src/rclcpp/node.cpp | 70 +++++++++++++++++-- .../src/rclcpp/node_interfaces/node_base.cpp | 1 - .../rclcpp_lifecycle/lifecycle_node.hpp | 5 +- rclcpp_lifecycle/src/lifecycle_node.cpp | 16 +++-- .../src/lifecycle_node_interface_impl.cpp | 16 ++++- .../src/lifecycle_node_interface_impl.hpp | 8 +++ 7 files changed, 103 insertions(+), 17 deletions(-) diff --git a/rclcpp/include/rclcpp/node.hpp b/rclcpp/include/rclcpp/node.hpp index 9193e3f3f9..18c6c16744 100644 --- a/rclcpp/include/rclcpp/node.hpp +++ b/rclcpp/include/rclcpp/node.hpp @@ -1592,12 +1592,14 @@ class Node : public std::enable_shared_from_this rclcpp::node_interfaces::NodeClockInterface::SharedPtr node_clock_; rclcpp::node_interfaces::NodeParametersInterface::SharedPtr node_parameters_; rclcpp::node_interfaces::NodeTimeSourceInterface::SharedPtr node_time_source_; - rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr node_type_descriptions_; rclcpp::node_interfaces::NodeWaitablesInterface::SharedPtr node_waitables_; const rclcpp::NodeOptions node_options_; const std::string sub_namespace_; const std::string effective_namespace_; + + class BackportMemberMaps; + static BackportMemberMaps backport_member_maps_; }; } // namespace rclcpp diff --git a/rclcpp/src/rclcpp/node.cpp b/rclcpp/src/rclcpp/node.cpp index 52012439e8..32aab85227 100644 --- a/rclcpp/src/rclcpp/node.cpp +++ b/rclcpp/src/rclcpp/node.cpp @@ -110,6 +110,65 @@ create_effective_namespace(const std::string & node_namespace, const std::string } // namespace +/// \brief Associates new hidden backported members with instances of Node. +class Node::BackportMemberMaps +{ +public: + BackportMemberMaps() = default; + + /// \brief Add all backported members for a new Node. + /** + * \param[in] key Raw pointer to the Node instance that will use new members. + */ + void add(Node * key) + { + // Adding a new instance to the maps requires exclusive access + std::unique_lock lock(map_access_mutex_); + type_descriptions_map_.emplace( + key, + std::make_shared( + key->get_node_base_interface(), + key->get_node_logging_interface(), + key->get_node_parameters_interface(), + key->get_node_services_interface())); + } + + /// \brief Remove the members for an instance of Node + /** + * \param[in] key Raw pointer to the Node + */ + void remove(const Node * key) + { + // Removing an instance from the maps requires exclusive access + std::unique_lock lock(map_access_mutex_); + type_descriptions_map_.erase(key); + } + + /// \brief Retrieve the NodeTypeDescriptionsInterface for a Node. + /** + * \param[in] key Raw pointer to an instance of Node. + * \return A shared ptr to this Node's NodeTypeDescriptionsInterface instance. + */ + rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr + get_node_type_descriptions_interface(const Node * key) const + { + // Multiple threads can retrieve from the maps at the same time + std::shared_lock lock(map_access_mutex_); + return type_descriptions_map_.at(key); + } + +private: + /// \brief Map that stored TypeDescriptionsInterface members + std::unordered_map< + const Node *, rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr + > type_descriptions_map_; + + /// \brief Controls access to all private maps + mutable std::shared_mutex map_access_mutex_; +}; +// Definition of static member declaration +Node::BackportMemberMaps Node::backport_member_maps_; + Node::Node( const std::string & node_name, const NodeOptions & options) @@ -207,17 +266,13 @@ Node::Node( options.clock_qos(), options.use_clock_thread() )), - node_type_descriptions_(new rclcpp::node_interfaces::NodeTypeDescriptions( - node_base_, - node_logging_, - node_parameters_, - node_services_ - )), node_waitables_(new rclcpp::node_interfaces::NodeWaitables(node_base_.get())), node_options_(options), sub_namespace_(""), effective_namespace_(create_effective_namespace(this->get_namespace(), sub_namespace_)) { + backport_member_maps_.add(this); + // we have got what we wanted directly from the overrides, // but declare the parameters anyway so they are visible. rclcpp::detail::declare_qos_parameters( @@ -279,6 +334,7 @@ Node::Node( Node::~Node() { // release sub-interfaces in an order that allows them to consult with node_base during tear-down + backport_member_maps_.remove(this); node_waitables_.reset(); node_time_source_.reset(); node_parameters_.reset(); @@ -601,7 +657,7 @@ Node::get_node_topics_interface() rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr Node::get_node_type_descriptions_interface() { - return node_type_descriptions_; + return backport_member_maps_.get_node_type_descriptions_interface(this); } rclcpp::node_interfaces::NodeServicesInterface::SharedPtr diff --git a/rclcpp/src/rclcpp/node_interfaces/node_base.cpp b/rclcpp/src/rclcpp/node_interfaces/node_base.cpp index 36e1afb932..6544d69214 100644 --- a/rclcpp/src/rclcpp/node_interfaces/node_base.cpp +++ b/rclcpp/src/rclcpp/node_interfaces/node_base.cpp @@ -20,7 +20,6 @@ #include "rclcpp/node_interfaces/node_base.hpp" #include "rcl/arguments.h" -#include "rcl/node_type_cache.h" #include "rclcpp/exceptions.hpp" #include "rcutils/logging_macros.h" #include "rmw/validate_namespace.h" diff --git a/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp b/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp index d3654ab2b2..72581e2b22 100644 --- a/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp +++ b/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp @@ -824,6 +824,10 @@ class LifecycleNode : public node_interfaces::LifecycleNodeInterface, rclcpp::node_interfaces::NodeTimeSourceInterface::SharedPtr get_node_time_source_interface(); + /// Return the Node's internal NodeTypeDescriptionsInterface implementation. + /** + * \sa rclcpp::Node::get_node_type_descriptions_interface + */ RCLCPP_LIFECYCLE_PUBLIC rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr get_node_type_descriptions_interface(); @@ -1090,7 +1094,6 @@ class LifecycleNode : public node_interfaces::LifecycleNodeInterface, rclcpp::node_interfaces::NodeClockInterface::SharedPtr node_clock_; rclcpp::node_interfaces::NodeParametersInterface::SharedPtr node_parameters_; rclcpp::node_interfaces::NodeTimeSourceInterface::SharedPtr node_time_source_; - rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr node_type_descriptions_; rclcpp::node_interfaces::NodeWaitablesInterface::SharedPtr node_waitables_; const rclcpp::NodeOptions node_options_; diff --git a/rclcpp_lifecycle/src/lifecycle_node.cpp b/rclcpp_lifecycle/src/lifecycle_node.cpp index 94629911a3..71deded6b1 100644 --- a/rclcpp_lifecycle/src/lifecycle_node.cpp +++ b/rclcpp_lifecycle/src/lifecycle_node.cpp @@ -114,15 +114,13 @@ LifecycleNode::LifecycleNode( options.clock_qos(), options.use_clock_thread() )), - node_type_descriptions_(new rclcpp::node_interfaces::NodeTypeDescriptions( + node_waitables_(new rclcpp::node_interfaces::NodeWaitables(node_base_.get())), + node_options_(options), + impl_(new LifecycleNodeInterfaceImpl( node_base_, node_logging_, node_parameters_, - node_services_ - )), - node_waitables_(new rclcpp::node_interfaces::NodeWaitables(node_base_.get())), - node_options_(options), - impl_(new LifecycleNodeInterfaceImpl(node_base_, node_services_)) + node_services_)) { impl_->init(enable_communication_interface); @@ -481,6 +479,12 @@ LifecycleNode::get_node_services_interface() return node_services_; } +rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr +LifecycleNode::get_node_type_descriptions_interface() +{ + return impl_->get_node_type_descriptions_interface(); +} + rclcpp::node_interfaces::NodeParametersInterface::SharedPtr LifecycleNode::get_node_parameters_interface() { diff --git a/rclcpp_lifecycle/src/lifecycle_node_interface_impl.cpp b/rclcpp_lifecycle/src/lifecycle_node_interface_impl.cpp index 5c5f7797e1..8d3855aacf 100644 --- a/rclcpp_lifecycle/src/lifecycle_node_interface_impl.cpp +++ b/rclcpp_lifecycle/src/lifecycle_node_interface_impl.cpp @@ -30,6 +30,7 @@ #include "rclcpp/node_interfaces/node_base_interface.hpp" #include "rclcpp/node_interfaces/node_services_interface.hpp" +#include "rclcpp/node_interfaces/node_type_descriptions.hpp" #include "rclcpp_lifecycle/node_interfaces/lifecycle_node_interface.hpp" @@ -50,9 +51,16 @@ namespace rclcpp_lifecycle LifecycleNode::LifecycleNodeInterfaceImpl::LifecycleNodeInterfaceImpl( std::shared_ptr node_base_interface, + std::shared_ptr node_logging_interface, + std::shared_ptr node_parameters_interface, std::shared_ptr node_services_interface) : node_base_interface_(node_base_interface), - node_services_interface_(node_services_interface) + node_services_interface_(node_services_interface), + node_type_descriptions_(new rclcpp::node_interfaces::NodeTypeDescriptions( + node_base_interface, + node_logging_interface, + node_parameters_interface, + node_services_interface)) { } @@ -581,4 +589,10 @@ LifecycleNode::LifecycleNodeInterfaceImpl::on_deactivate() const } } +rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr +LifecycleNode::LifecycleNodeInterfaceImpl::get_node_type_descriptions_interface() +{ + return node_type_descriptions_; +} + } // namespace rclcpp_lifecycle diff --git a/rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp b/rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp index d09f44831c..5db7c31d99 100644 --- a/rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp +++ b/rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp @@ -52,6 +52,8 @@ class LifecycleNode::LifecycleNodeInterfaceImpl final public: LifecycleNodeInterfaceImpl( std::shared_ptr node_base_interface, + std::shared_ptr node_logging_interface, + std::shared_ptr node_parameters_interface, std::shared_ptr node_services_interface); ~LifecycleNodeInterfaceImpl(); @@ -102,6 +104,9 @@ class LifecycleNode::LifecycleNodeInterfaceImpl final void add_timer_handle(std::shared_ptr timer); + rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr + get_node_type_descriptions_interface(); + private: RCLCPP_DISABLE_COPY(LifecycleNodeInterfaceImpl) @@ -172,6 +177,9 @@ class LifecycleNode::LifecycleNodeInterfaceImpl final // to controllable things std::vector> weak_managed_entities_; std::vector> weak_timers_; + + // Backported members hidden in impl + rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr node_type_descriptions_; }; } // namespace rclcpp_lifecycle From 1d8c616dad3a34192563e20c8d4d09c1d5f0af8b Mon Sep 17 00:00:00 2001 From: Emerson Knapp Date: Fri, 7 Jul 2023 15:31:31 -0700 Subject: [PATCH 3/5] Add typedescription interface smoke test Signed-off-by: Emerson Knapp --- rclcpp/test/rclcpp/test_node.cpp | 1 + rclcpp_lifecycle/test/test_lifecycle_node.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/rclcpp/test/rclcpp/test_node.cpp b/rclcpp/test/rclcpp/test_node.cpp index 512c2c1384..2f74a998c3 100644 --- a/rclcpp/test/rclcpp/test_node.cpp +++ b/rclcpp/test/rclcpp/test_node.cpp @@ -78,6 +78,7 @@ TEST_F(TestNode, construction_and_destruction) { EXPECT_NE(nullptr, node->get_node_options().get_rcl_node_options()); EXPECT_NE(nullptr, node->get_graph_event()); EXPECT_NE(nullptr, node->get_clock()); + EXPECT_NE(nullptr, node->get_node_type_descriptions_interface()); } { diff --git a/rclcpp_lifecycle/test/test_lifecycle_node.cpp b/rclcpp_lifecycle/test/test_lifecycle_node.cpp index db24bd0ee7..8a09884e70 100644 --- a/rclcpp_lifecycle/test/test_lifecycle_node.cpp +++ b/rclcpp_lifecycle/test/test_lifecycle_node.cpp @@ -641,6 +641,7 @@ TEST_F(TestDefaultStateMachine, test_getters) { EXPECT_LT(0u, test_node->now().nanoseconds()); EXPECT_STREQ("testnode", test_node->get_logger().get_name()); EXPECT_NE(nullptr, const_cast(test_node.get())->get_clock()); + EXPECT_NE(nullptr, test_node->get_node_type_descriptions_interface()); } TEST_F(TestDefaultStateMachine, test_graph_topics) { From 037b8db853a9fc7b47a2a6bd05f2efd91e10ac75 Mon Sep 17 00:00:00 2001 From: Emerson Knapp Date: Wed, 12 Jul 2023 14:02:02 -0700 Subject: [PATCH 4/5] include what you use Signed-off-by: Emerson Knapp --- rclcpp/src/rclcpp/node.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rclcpp/src/rclcpp/node.cpp b/rclcpp/src/rclcpp/node.cpp index 32aab85227..101c280b00 100644 --- a/rclcpp/src/rclcpp/node.cpp +++ b/rclcpp/src/rclcpp/node.cpp @@ -17,7 +17,10 @@ #include #include #include +#include +#include #include +#include #include #include From ea2db3123e9bb00b1fc767fcb0609110a2971373 Mon Sep 17 00:00:00 2001 From: Emerson Knapp Date: Mon, 17 Jul 2023 11:14:02 -0700 Subject: [PATCH 5/5] Add more detailed doc comments on BackportMembers, slight name change Signed-off-by: Emerson Knapp --- rclcpp/include/rclcpp/node.hpp | 6 ++++-- rclcpp/src/rclcpp/node.cpp | 21 ++++++++++++++------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/rclcpp/include/rclcpp/node.hpp b/rclcpp/include/rclcpp/node.hpp index 18c6c16744..59863df91e 100644 --- a/rclcpp/include/rclcpp/node.hpp +++ b/rclcpp/include/rclcpp/node.hpp @@ -1598,8 +1598,10 @@ class Node : public std::enable_shared_from_this const std::string sub_namespace_; const std::string effective_namespace_; - class BackportMemberMaps; - static BackportMemberMaps backport_member_maps_; + /// Static map(s) containing extra member variables for Node without changing its ABI. + // See node.cpp for more details. + class BackportMembers; + static BackportMembers backport_members_; }; } // namespace rclcpp diff --git a/rclcpp/src/rclcpp/node.cpp b/rclcpp/src/rclcpp/node.cpp index 101c280b00..e837637dde 100644 --- a/rclcpp/src/rclcpp/node.cpp +++ b/rclcpp/src/rclcpp/node.cpp @@ -113,11 +113,18 @@ create_effective_namespace(const std::string & node_namespace, const std::string } // namespace -/// \brief Associates new hidden backported members with instances of Node. -class Node::BackportMemberMaps +/// \brief Associate new extra member variables with instances of Node without changing ABI. +/** + * It is used only for bugfixes or backported features that require new members. + * Atomically constructs/destroys all extra members. + * Node instance will register and remove itself, and use its methods to retrieve members. + * Note for performance consideration that accessing these members uses a map lookup. + */ +class Node::BackportMembers { public: - BackportMemberMaps() = default; + BackportMembers() = default; + ~BackportMembers() = default; /// \brief Add all backported members for a new Node. /** @@ -170,7 +177,7 @@ class Node::BackportMemberMaps mutable std::shared_mutex map_access_mutex_; }; // Definition of static member declaration -Node::BackportMemberMaps Node::backport_member_maps_; +Node::BackportMembers Node::backport_members_; Node::Node( const std::string & node_name, @@ -274,7 +281,7 @@ Node::Node( sub_namespace_(""), effective_namespace_(create_effective_namespace(this->get_namespace(), sub_namespace_)) { - backport_member_maps_.add(this); + backport_members_.add(this); // we have got what we wanted directly from the overrides, // but declare the parameters anyway so they are visible. @@ -337,7 +344,7 @@ Node::Node( Node::~Node() { // release sub-interfaces in an order that allows them to consult with node_base during tear-down - backport_member_maps_.remove(this); + backport_members_.remove(this); node_waitables_.reset(); node_time_source_.reset(); node_parameters_.reset(); @@ -660,7 +667,7 @@ Node::get_node_topics_interface() rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr Node::get_node_type_descriptions_interface() { - return backport_member_maps_.get_node_type_descriptions_interface(this); + return backport_members_.get_node_type_descriptions_interface(this); } rclcpp::node_interfaces::NodeServicesInterface::SharedPtr