From abe55817352cb592cd6d8818e3d8e788d1809203 Mon Sep 17 00:00:00 2001 From: olesya Date: Sun, 19 Jan 2025 15:54:59 +0000 Subject: [PATCH 1/7] Bug with creating a custom subscriber was fixed. Signed-off-by: olesya --- rclcpp/include/rclcpp/create_subscription.hpp | 5 ++++- rclcpp/include/rclcpp/node_impl.hpp | 3 ++- rclcpp/include/rclcpp/subscription_factory.hpp | 7 ++----- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/rclcpp/include/rclcpp/create_subscription.hpp b/rclcpp/include/rclcpp/create_subscription.hpp index 3a1e4b1a13..6d67e93a64 100644 --- a/rclcpp/include/rclcpp/create_subscription.hpp +++ b/rclcpp/include/rclcpp/create_subscription.hpp @@ -118,7 +118,10 @@ create_subscription( subscription_topic_stats->set_publisher_timer(timer); } - auto factory = rclcpp::create_subscription_factory( + auto factory = rclcpp::create_subscription_factory( std::forward(callback), options, msg_mem_strat, diff --git a/rclcpp/include/rclcpp/node_impl.hpp b/rclcpp/include/rclcpp/node_impl.hpp index 5b81bdcba4..4ff4732d92 100644 --- a/rclcpp/include/rclcpp/node_impl.hpp +++ b/rclcpp/include/rclcpp/node_impl.hpp @@ -97,7 +97,8 @@ Node::create_subscription( const SubscriptionOptionsWithAllocator & options, typename MessageMemoryStrategyT::SharedPtr msg_mem_strat) { - return rclcpp::create_subscription( + return rclcpp::create_subscription< + MessageT, CallbackT, AllocatorT, SubscriptionT, MessageMemoryStrategyT>( *this, extend_name_with_sub_namespace(topic_name, this->get_sub_namespace()), qos, diff --git a/rclcpp/include/rclcpp/subscription_factory.hpp b/rclcpp/include/rclcpp/subscription_factory.hpp index 0e9d9fefe5..ae60f05b4a 100644 --- a/rclcpp/include/rclcpp/subscription_factory.hpp +++ b/rclcpp/include/rclcpp/subscription_factory.hpp @@ -100,10 +100,7 @@ create_subscription_factory( const rclcpp::QoS & qos ) -> rclcpp::SubscriptionBase::SharedPtr { - using rclcpp::Subscription; - using rclcpp::SubscriptionBase; - - auto sub = Subscription::make_shared( + auto sub = std::make_shared( node_base, rclcpp::get_message_type_support_handle(), topic_name, @@ -116,7 +113,7 @@ create_subscription_factory( // require this->shared_from_this() which cannot be called from // the constructor. sub->post_init_setup(node_base, qos, options); - auto sub_base_ptr = std::dynamic_pointer_cast(sub); + auto sub_base_ptr = std::dynamic_pointer_cast(sub); return sub_base_ptr; } }; From 4171b69d6660e3f3a4a3d7793cd36d86cfe333ed Mon Sep 17 00:00:00 2001 From: olesya Date: Sun, 19 Jan 2025 16:22:15 +0000 Subject: [PATCH 2/7] Added a test to check if a custom subscriber was created correctly. Signed-off-by: olesya --- rclcpp/test/rclcpp/CMakeLists.txt | 7 ++ .../test_create_custom_subscription.cpp | 72 +++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 rclcpp/test/rclcpp/test_create_custom_subscription.cpp diff --git a/rclcpp/test/rclcpp/CMakeLists.txt b/rclcpp/test/rclcpp/CMakeLists.txt index 35fa8fdd32..e1c2bdf75e 100644 --- a/rclcpp/test/rclcpp/CMakeLists.txt +++ b/rclcpp/test/rclcpp/CMakeLists.txt @@ -95,6 +95,13 @@ if(TARGET test_generic_service) ${test_msgs_TARGETS} ) endif() +ament_add_gtest(test_create_custom_subscription test_create_custom_subscription.cpp) +if(TARGET test_create_custom_subscription) + target_link_libraries(test_create_custom_subscription ${PROJECT_NAME}) + ament_target_dependencies(test_create_custom_subscription + "test_msgs" + ) +endif() ament_add_gtest(test_client_common test_client_common.cpp) ament_add_test_label(test_client_common mimick) if(TARGET test_client_common) diff --git a/rclcpp/test/rclcpp/test_create_custom_subscription.cpp b/rclcpp/test/rclcpp/test_create_custom_subscription.cpp new file mode 100644 index 0000000000..dfea57f0a4 --- /dev/null +++ b/rclcpp/test/rclcpp/test_create_custom_subscription.cpp @@ -0,0 +1,72 @@ +#include + +#include +#include +#include + +#include "rclcpp/subscription.hpp" +#include "rclcpp/create_subscription.hpp" +#include "rclcpp/node.hpp" +#include "test_msgs/msg/empty.hpp" +#include "test_msgs/msg/empty.h" + +using namespace std::chrono_literals; + +class TestCreateSubscription : public ::testing::Test +{ +public: + void SetUp() override + { + rclcpp::init(0, nullptr); + } + + void TearDown() override + { + rclcpp::shutdown(); + } +}; + +template< + typename MessageT, + typename AllocatorT = std::allocator, + typename SubscribedT = typename rclcpp::TypeAdapter::custom_type, + typename ROSMessageT = typename rclcpp::TypeAdapter::ros_message_type, + typename MessageMemoryStrategyT = rclcpp::message_memory_strategy::MessageMemoryStrategy< + ROSMessageT, + AllocatorT + >> +class CustomSubscription : public rclcpp::Subscription< + MessageT, AllocatorT, SubscribedT, ROSMessageT, MessageMemoryStrategyT> +{ +public: + + template + CustomSubscription(Args &&...args) : rclcpp::Subscription< + MessageT, AllocatorT, SubscribedT, ROSMessageT, MessageMemoryStrategyT>( + std::forward(args)...) {} +}; + +TEST_F(TestCreateSubscription, create) { + using MessageT = test_msgs::msg::Empty; + + auto node = std::make_shared("my_node", "/ns"); + const rclcpp::QoS qos(10); + auto options = rclcpp::SubscriptionOptions(); + auto callback = [](MessageT::ConstSharedPtr) {}; + + using CallbackT = std::decay_t; + using AllocatorT = std::allocator; + using SubscriptionT = CustomSubscription; + using CallbackMessageT = + typename rclcpp::subscription_traits::has_message_type::type; + using MessageMemoryStrategyT = + rclcpp::message_memory_strategy::MessageMemoryStrategy; + + auto subscription = rclcpp::create_subscription< + MessageT, CallbackT, AllocatorT, SubscriptionT, MessageMemoryStrategyT>( + node, "topic_name", qos, std::move(callback), options); + + ASSERT_NE(nullptr, subscription); + EXPECT_STREQ("/ns/topic_name", subscription->get_topic_name()); + static_assert(std::is_same_v, SubscriptionT>); +} \ No newline at end of file From 9fd93a2524dbf01eef6671a9021b97f4bdb59c94 Mon Sep 17 00:00:00 2001 From: olesya Date: Mon, 3 Feb 2025 19:27:28 +0000 Subject: [PATCH 3/7] Style was fixed. Signed-off-by: olesya --- rclcpp/include/rclcpp/create_subscription.hpp | 8 +++--- .../test_create_custom_subscription.cpp | 28 ++++++++++++++----- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/rclcpp/include/rclcpp/create_subscription.hpp b/rclcpp/include/rclcpp/create_subscription.hpp index 6d67e93a64..de4a4331d5 100644 --- a/rclcpp/include/rclcpp/create_subscription.hpp +++ b/rclcpp/include/rclcpp/create_subscription.hpp @@ -119,14 +119,14 @@ create_subscription( } auto factory = rclcpp::create_subscription_factory( + SubscriptionT, MessageMemoryStrategyT, + ROSMessageType + >( std::forward(callback), options, msg_mem_strat, subscription_topic_stats - ); + ); const rclcpp::QoS & actual_qos = options.qos_overriding_options.get_policy_kinds().size() ? rclcpp::detail::declare_qos_parameters( diff --git a/rclcpp/test/rclcpp/test_create_custom_subscription.cpp b/rclcpp/test/rclcpp/test_create_custom_subscription.cpp index dfea57f0a4..894d14a3a2 100644 --- a/rclcpp/test/rclcpp/test_create_custom_subscription.cpp +++ b/rclcpp/test/rclcpp/test_create_custom_subscription.cpp @@ -1,3 +1,17 @@ +// Copyright 2025 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 @@ -36,14 +50,14 @@ template< AllocatorT >> class CustomSubscription : public rclcpp::Subscription< - MessageT, AllocatorT, SubscribedT, ROSMessageT, MessageMemoryStrategyT> + MessageT, AllocatorT, SubscribedT, ROSMessageT, MessageMemoryStrategyT> { public: - - template - CustomSubscription(Args &&...args) : rclcpp::Subscription< - MessageT, AllocatorT, SubscribedT, ROSMessageT, MessageMemoryStrategyT>( - std::forward(args)...) {} + template + explicit CustomSubscription(Args &&... args) + : rclcpp::Subscription< + MessageT, AllocatorT, SubscribedT, ROSMessageT, MessageMemoryStrategyT>( + std::forward(args)...) {} }; TEST_F(TestCreateSubscription, create) { @@ -69,4 +83,4 @@ TEST_F(TestCreateSubscription, create) { ASSERT_NE(nullptr, subscription); EXPECT_STREQ("/ns/topic_name", subscription->get_topic_name()); static_assert(std::is_same_v, SubscriptionT>); -} \ No newline at end of file +} From 301312652224360f920a496bc654940b35f5f3e7 Mon Sep 17 00:00:00 2001 From: olesya Date: Wed, 5 Mar 2025 15:41:39 +0000 Subject: [PATCH 4/7] Building for rolling was fixed. Signed-off-by: olesya --- rclcpp/include/rclcpp/create_subscription.hpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rclcpp/include/rclcpp/create_subscription.hpp b/rclcpp/include/rclcpp/create_subscription.hpp index de4a4331d5..d6972c57cf 100644 --- a/rclcpp/include/rclcpp/create_subscription.hpp +++ b/rclcpp/include/rclcpp/create_subscription.hpp @@ -119,8 +119,7 @@ create_subscription( } auto factory = rclcpp::create_subscription_factory( std::forward(callback), options, From 5c5e9aebe1afb47a7b48c42c9d9d3dddfeaecd5d Mon Sep 17 00:00:00 2001 From: olesya <66834330+bks-ol@users.noreply.github.com> Date: Sun, 9 Mar 2025 11:22:21 +0300 Subject: [PATCH 5/7] Update rclcpp/test/rclcpp/test_create_custom_subscription.cpp Co-authored-by: Tomoya Fujita Signed-off-by: olesya <66834330+bks-ol@users.noreply.github.com> --- rclcpp/test/rclcpp/test_create_custom_subscription.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclcpp/test/rclcpp/test_create_custom_subscription.cpp b/rclcpp/test/rclcpp/test_create_custom_subscription.cpp index 894d14a3a2..8e8d6e3838 100644 --- a/rclcpp/test/rclcpp/test_create_custom_subscription.cpp +++ b/rclcpp/test/rclcpp/test_create_custom_subscription.cpp @@ -26,7 +26,7 @@ using namespace std::chrono_literals; -class TestCreateSubscription : public ::testing::Test +class TestCreateCustomSubscription : public ::testing::Test { public: void SetUp() override From 26923f5905fa3dc2cd66c2811055175013e69e7f Mon Sep 17 00:00:00 2001 From: olesya Date: Fri, 14 Mar 2025 11:53:27 +0000 Subject: [PATCH 6/7] Update rclcpp/test/rclcpp/test_create_custom_subscription.cpp Signed-off-by: olesya --- rclcpp/test/rclcpp/test_create_custom_subscription.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclcpp/test/rclcpp/test_create_custom_subscription.cpp b/rclcpp/test/rclcpp/test_create_custom_subscription.cpp index 8e8d6e3838..b1ceb4f320 100644 --- a/rclcpp/test/rclcpp/test_create_custom_subscription.cpp +++ b/rclcpp/test/rclcpp/test_create_custom_subscription.cpp @@ -60,7 +60,7 @@ class CustomSubscription : public rclcpp::Subscription< std::forward(args)...) {} }; -TEST_F(TestCreateSubscription, create) { +TEST_F(TestCreateCustomSubscription, create) { using MessageT = test_msgs::msg::Empty; auto node = std::make_shared("my_node", "/ns"); From 365baac41f616521682de9b19e4e03ae3a6666ea Mon Sep 17 00:00:00 2001 From: olesya Date: Sun, 20 Apr 2025 15:38:55 +0300 Subject: [PATCH 7/7] Style was fixed Signed-off-by: olesya --- rclcpp/include/rclcpp/create_subscription.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclcpp/include/rclcpp/create_subscription.hpp b/rclcpp/include/rclcpp/create_subscription.hpp index d6972c57cf..d41ba5a892 100644 --- a/rclcpp/include/rclcpp/create_subscription.hpp +++ b/rclcpp/include/rclcpp/create_subscription.hpp @@ -125,7 +125,7 @@ create_subscription( options, msg_mem_strat, subscription_topic_stats - ); + ); const rclcpp::QoS & actual_qos = options.qos_overriding_options.get_policy_kinds().size() ? rclcpp::detail::declare_qos_parameters(