From 4d32bec50f51a95485258fd0639d1505812c7e6c Mon Sep 17 00:00:00 2001 From: mini-1235 Date: Fri, 7 Nov 2025 18:55:32 +0000 Subject: [PATCH 1/4] Add static assert for removed subscription callback signatures Signed-off-by: mini-1235 --- .../rclcpp/any_subscription_callback.hpp | 28 +++++++ .../rclcpp/test_any_subscription_callback.cpp | 79 +------------------ ...ption_publisher_with_same_type_adapter.cpp | 16 ++-- 3 files changed, 38 insertions(+), 85 deletions(-) diff --git a/rclcpp/include/rclcpp/any_subscription_callback.hpp b/rclcpp/include/rclcpp/any_subscription_callback.hpp index da5abe6c53..e721fed272 100644 --- a/rclcpp/include/rclcpp/any_subscription_callback.hpp +++ b/rclcpp/include/rclcpp/any_subscription_callback.hpp @@ -392,6 +392,34 @@ class AnySubscriptionCallback // automatically with lambda functions in cases where the arguments can be // converted to one another, e.g. shared_ptr and unique_ptr. using scbth = detail::SubscriptionCallbackTypeHelper; + constexpr auto is_invalid_signature = + rclcpp::function_traits::same_arguments< + typename scbth::callback_type, + std::function)> + >::value || + rclcpp::function_traits::same_arguments< + typename scbth::callback_type, + std::function, const rclcpp::MessageInfo &)> + >::value || + rclcpp::function_traits::same_arguments< + typename scbth::callback_type, + std::function)> + >::value || + rclcpp::function_traits::same_arguments< + typename scbth::callback_type, + std::function, const rclcpp::MessageInfo &)> + >::value || + rclcpp::function_traits::same_arguments< + typename scbth::callback_type, + std::function)> + >::value || + rclcpp::function_traits::same_arguments< + typename scbth::callback_type, + std::function, const rclcpp::MessageInfo &)> + >::value; + static_assert(!is_invalid_signature, + "std::shared_ptr<> callback signature is unsupported " + "Use std::shared_ptr or std::unique_ptr<> instead."); callback_variant_ = static_cast(callback); diff --git a/rclcpp/test/rclcpp/test_any_subscription_callback.cpp b/rclcpp/test/rclcpp/test_any_subscription_callback.cpp index 58a9211937..1aff5a0629 100644 --- a/rclcpp/test/rclcpp/test_any_subscription_callback.cpp +++ b/rclcpp/test/rclcpp/test_any_subscription_callback.cpp @@ -177,7 +177,7 @@ TEST_F(TestAnySubscriptionCallback, is_serialized_message_callback) { } { rclcpp::AnySubscriptionCallback asc; - asc.set([](std::shared_ptr) {}); + asc.set([](std::shared_ptr) {}); EXPECT_TRUE(asc.is_serialized_message_callback()); EXPECT_NO_THROW( asc.dispatch( @@ -186,7 +186,7 @@ TEST_F(TestAnySubscriptionCallback, is_serialized_message_callback) { } { rclcpp::AnySubscriptionCallback asc; - asc.set([](std::shared_ptr, const rclcpp::MessageInfo &) {}); + asc.set([](std::shared_ptr, const rclcpp::MessageInfo &) {}); EXPECT_TRUE(asc.is_serialized_message_callback()); EXPECT_NO_THROW( asc.dispatch( @@ -646,78 +646,3 @@ INSTANTIATE_TEST_SUITE_P( ), format_parameter_with_ta ); - -// -// Versions of `std::shared_ptr` -// -void shared_ptr_free_func(std::shared_ptr) {} -void shared_ptr_w_info_free_func( - std::shared_ptr, const rclcpp::MessageInfo &) -{} - -INSTANTIATE_TEST_SUITE_P( - SharedPtrCallbackTests, - DispatchTests, - ::testing::Values( - // lambda - InstanceContext{"lambda", rclcpp::AnySubscriptionCallback().set( - [](std::shared_ptr) {})}, - InstanceContext{"lambda_with_info", - rclcpp::AnySubscriptionCallback().set( - [](std::shared_ptr, const rclcpp::MessageInfo &) {})}, - // free function - InstanceContext{"free_function", rclcpp::AnySubscriptionCallback().set( - shared_ptr_free_func)}, - InstanceContext{"free_function_with_info", - rclcpp::AnySubscriptionCallback().set( - shared_ptr_w_info_free_func)}, - // bind function - BindContext>("bind_method"), - BindContext, - const rclcpp::MessageInfo &>( - "bind_method_with_info") - ), - format_parameter -); - -void shared_ptr_ta_free_func(std::shared_ptr) {} -void shared_ptr_ta_w_info_free_func( - std::shared_ptr, const rclcpp::MessageInfo &) -{} - -INSTANTIATE_TEST_SUITE_P( - SharedPtrCallbackTests, - DispatchTestsWithTA, - ::testing::Values( - // lambda - InstanceContext{"lambda_ta", rclcpp::AnySubscriptionCallback().set( - [](std::shared_ptr) {})}, - InstanceContext{"lambda_ta_with_info", - rclcpp::AnySubscriptionCallback().set( - [](std::shared_ptr, const rclcpp::MessageInfo &) {})}, - InstanceContext{"lambda", rclcpp::AnySubscriptionCallback().set( - [](std::shared_ptr) {})}, - InstanceContext{"lambda_with_info", - rclcpp::AnySubscriptionCallback().set( - [](std::shared_ptr, const rclcpp::MessageInfo &) {})}, - // free function - InstanceContext{"free_function_ta", rclcpp::AnySubscriptionCallback().set( - shared_ptr_ta_free_func)}, - InstanceContext{"free_function_ta_with_info", - rclcpp::AnySubscriptionCallback().set( - shared_ptr_ta_w_info_free_func)}, - InstanceContext{"free_function", rclcpp::AnySubscriptionCallback().set( - shared_ptr_free_func)}, - InstanceContext{"free_function_with_info", - rclcpp::AnySubscriptionCallback().set( - shared_ptr_w_info_free_func)}, - // bind function - BindContext>("bind_method_ta"), - BindContext, const rclcpp::MessageInfo &>( - "bind_method_ta_with_info"), - BindContext>("bind_method"), - BindContext, const rclcpp::MessageInfo &>( - "bind_method_with_info") - ), - format_parameter_with_ta -); diff --git a/rclcpp/test/rclcpp/test_subscription_publisher_with_same_type_adapter.cpp b/rclcpp/test/rclcpp/test_subscription_publisher_with_same_type_adapter.cpp index ffb47e9468..d733d9758b 100644 --- a/rclcpp/test/rclcpp/test_subscription_publisher_with_same_type_adapter.cpp +++ b/rclcpp/test/rclcpp/test_subscription_publisher_with_same_type_adapter.cpp @@ -212,7 +212,7 @@ TEST_F( bool is_received = false; auto callback = [message_data, &is_received]( - std::shared_ptr msg) -> void { + std::shared_ptr msg) -> void { is_received = true; EXPECT_STREQ(message_data.c_str(), (*msg).c_str()); }; @@ -230,7 +230,7 @@ TEST_F( bool is_received = false; auto callback = [message_data, &is_received]( - std::shared_ptr msg, + std::shared_ptr msg, const rclcpp::MessageInfo & message_info) -> void { is_received = true; EXPECT_STREQ(message_data.c_str(), (*msg).c_str()); @@ -422,7 +422,7 @@ TEST_F( bool is_received = false; auto callback = [message_data, &is_received]( - std::shared_ptr msg) -> void { + std::shared_ptr msg) -> void { is_received = true; EXPECT_STREQ(message_data.c_str(), (*msg).c_str()); }; @@ -440,7 +440,7 @@ TEST_F( bool is_received = false; auto callback = [message_data, &is_received]( - std::shared_ptr msg, + std::shared_ptr msg, const rclcpp::MessageInfo & message_info) -> void { is_received = true; EXPECT_STREQ(message_data.c_str(), (*msg).c_str()); @@ -637,7 +637,7 @@ TEST_F( bool is_received = false; auto callback = [message_data, &is_received]( - std::shared_ptr msg) -> void { + std::shared_ptr msg) -> void { is_received = true; ASSERT_EQ(message_data, *msg); }; @@ -656,7 +656,7 @@ TEST_F( bool is_received = false; auto callback = [message_data, &is_received]( - std::shared_ptr msg, + std::shared_ptr msg, const rclcpp::MessageInfo & message_info) -> void { is_received = true; ASSERT_EQ(message_data, *msg); @@ -862,7 +862,7 @@ TEST_F( bool is_received = false; auto callback = [message_data, &is_received]( - std::shared_ptr msg) -> void { + std::shared_ptr msg) -> void { is_received = true; ASSERT_EQ(message_data, *msg); }; @@ -882,7 +882,7 @@ TEST_F( bool is_received = false; auto callback = [message_data, &is_received]( - std::shared_ptr msg, + std::shared_ptr msg, const rclcpp::MessageInfo & message_info) -> void { is_received = true; ASSERT_EQ(message_data, *msg); From fef15181cb4a256780b07f90ebff234eb82e337d Mon Sep 17 00:00:00 2001 From: mini-1235 Date: Wed, 26 Nov 2025 17:51:58 +0000 Subject: [PATCH 2/4] Deprecate shared_ptr Signed-off-by: mini-1235 --- .../rclcpp/any_subscription_callback.hpp | 33 ++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/rclcpp/include/rclcpp/any_subscription_callback.hpp b/rclcpp/include/rclcpp/any_subscription_callback.hpp index e721fed272..ec3a3bd5cc 100644 --- a/rclcpp/include/rclcpp/any_subscription_callback.hpp +++ b/rclcpp/include/rclcpp/any_subscription_callback.hpp @@ -417,16 +417,41 @@ class AnySubscriptionCallback typename scbth::callback_type, std::function, const rclcpp::MessageInfo &)> >::value; - static_assert(!is_invalid_signature, - "std::shared_ptr<> callback signature is unsupported " - "Use std::shared_ptr or std::unique_ptr<> instead."); - callback_variant_ = static_cast(callback); + // Use the discovered type to force the type of callback when assigning + // into the variant. + if constexpr (is_invalid_signature) { + // If deprecated, call sub-routine that is deprecated. + set_deprecated(static_cast(callback)); + } else { + // Otherwise just assign it. + callback_variant_ = static_cast(callback); + } // Return copy of self for easier testing, normally will be compiled out. return *this; } + template + /// Function for shared_ptr to non-const MessageT, which is deprecated. + [[deprecated("use 'void(std::shared_ptr)' instead")]] + void + set_deprecated(std::function)> callback) + { + callback_variant_ = callback; + } + + /// Function for shared_ptr to non-const MessageT with MessageInfo, which is deprecated. + template + [[deprecated( + "use 'void(std::shared_ptr, const rclcpp::MessageInfo &)' instead" + )]] + void + set_deprecated(std::function, const rclcpp::MessageInfo &)> callback) + { + callback_variant_ = callback; + } + std::unique_ptr create_ros_unique_ptr_from_ros_shared_ptr_message( const std::shared_ptr & message) From b1b18581255f92877ee0cc888d014a6e63641788 Mon Sep 17 00:00:00 2001 From: mini-1235 Date: Sun, 7 Dec 2025 11:23:29 +0000 Subject: [PATCH 3/4] Add back tests Signed-off-by: mini-1235 --- .../rclcpp/any_subscription_callback.hpp | 16 +++- .../rclcpp/test_any_subscription_callback.cpp | 77 +++++++++++++++++++ 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/rclcpp/include/rclcpp/any_subscription_callback.hpp b/rclcpp/include/rclcpp/any_subscription_callback.hpp index 1b6c54d750..661e6b97fd 100644 --- a/rclcpp/include/rclcpp/any_subscription_callback.hpp +++ b/rclcpp/include/rclcpp/any_subscription_callback.hpp @@ -406,7 +406,9 @@ class AnySubscriptionCallback // automatically with lambda functions in cases where the arguments can be // converted to one another, e.g. shared_ptr and unique_ptr. using scbth = detail::SubscriptionCallbackTypeHelper; - constexpr auto is_invalid_signature = + + // Determine if the given CallbackT is a deprecated signature or not. + constexpr auto is_deprecated = rclcpp::function_traits::same_arguments< typename scbth::callback_type, std::function)> @@ -434,7 +436,7 @@ class AnySubscriptionCallback // Use the discovered type to force the type of callback when assigning // into the variant. - if constexpr (is_invalid_signature) { + if constexpr (is_deprecated) { // If deprecated, call sub-routine that is deprecated. set_deprecated(static_cast(callback)); } else { @@ -446,9 +448,12 @@ class AnySubscriptionCallback return *this; } - template /// Function for shared_ptr to non-const MessageT, which is deprecated. + template + #if !defined(RCLCPP_AVOID_DEPRECATIONS_FOR_UNIT_TESTS) + // suppress deprecation warnings in `test_any_subscription_callback.cpp` [[deprecated("use 'void(std::shared_ptr)' instead")]] + #endif void set_deprecated(std::function)> callback) { @@ -457,13 +462,18 @@ class AnySubscriptionCallback /// Function for shared_ptr to non-const MessageT with MessageInfo, which is deprecated. template + #if !defined(RCLCPP_AVOID_DEPRECATIONS_FOR_UNIT_TESTS) + // suppress deprecation warnings in `test_any_subscription_callback.cpp` [[deprecated( "use 'void(std::shared_ptr, const rclcpp::MessageInfo &)' instead" )]] + #endif void set_deprecated(std::function, const rclcpp::MessageInfo &)> callback) { callback_variant_ = callback; + } + /// Disable the callback from being called during dispatch. void disable() { diff --git a/rclcpp/test/rclcpp/test_any_subscription_callback.cpp b/rclcpp/test/rclcpp/test_any_subscription_callback.cpp index 8565d65394..a256e39ae1 100644 --- a/rclcpp/test/rclcpp/test_any_subscription_callback.cpp +++ b/rclcpp/test/rclcpp/test_any_subscription_callback.cpp @@ -20,6 +20,7 @@ #include #include +#define RCLCPP_AVOID_DEPRECATIONS_FOR_UNIT_TESTS 1 #include "rclcpp/any_subscription_callback.hpp" #include "test_msgs/msg/empty.hpp" @@ -767,3 +768,79 @@ INSTANTIATE_TEST_SUITE_P( ), format_parameter_with_ta ); + + +// +// Versions of `std::shared_ptr` +// +void shared_ptr_free_func(std::shared_ptr) {} +void shared_ptr_w_info_free_func( + std::shared_ptr, const rclcpp::MessageInfo &) +{} + +INSTANTIATE_TEST_SUITE_P( + SharedPtrCallbackTests, + DispatchTests, + ::testing::Values( + // lambda + InstanceContext{"lambda", rclcpp::AnySubscriptionCallback().set( + [](std::shared_ptr) {})}, + InstanceContext{"lambda_with_info", + rclcpp::AnySubscriptionCallback().set( + [](std::shared_ptr, const rclcpp::MessageInfo &) {})}, + // free function + InstanceContext{"free_function", rclcpp::AnySubscriptionCallback().set( + shared_ptr_free_func)}, + InstanceContext{"free_function_with_info", + rclcpp::AnySubscriptionCallback().set( + shared_ptr_w_info_free_func)}, + // bind function + BindContext>("bind_method"), + BindContext, + const rclcpp::MessageInfo &>( + "bind_method_with_info") + ), + format_parameter +); + +void shared_ptr_ta_free_func(std::shared_ptr) {} +void shared_ptr_ta_w_info_free_func( + std::shared_ptr, const rclcpp::MessageInfo &) +{} + +INSTANTIATE_TEST_SUITE_P( + SharedPtrCallbackTests, + DispatchTestsWithTA, + ::testing::Values( + // lambda + InstanceContext{"lambda_ta", rclcpp::AnySubscriptionCallback().set( + [](std::shared_ptr) {})}, + InstanceContext{"lambda_ta_with_info", + rclcpp::AnySubscriptionCallback().set( + [](std::shared_ptr, const rclcpp::MessageInfo &) {})}, + InstanceContext{"lambda", rclcpp::AnySubscriptionCallback().set( + [](std::shared_ptr) {})}, + InstanceContext{"lambda_with_info", + rclcpp::AnySubscriptionCallback().set( + [](std::shared_ptr, const rclcpp::MessageInfo &) {})}, + // free function + InstanceContext{"free_function_ta", rclcpp::AnySubscriptionCallback().set( + shared_ptr_ta_free_func)}, + InstanceContext{"free_function_ta_with_info", + rclcpp::AnySubscriptionCallback().set( + shared_ptr_ta_w_info_free_func)}, + InstanceContext{"free_function", rclcpp::AnySubscriptionCallback().set( + shared_ptr_free_func)}, + InstanceContext{"free_function_with_info", + rclcpp::AnySubscriptionCallback().set( + shared_ptr_w_info_free_func)}, + // bind function + BindContext>("bind_method_ta"), + BindContext, const rclcpp::MessageInfo &>( + "bind_method_ta_with_info"), + BindContext>("bind_method"), + BindContext, const rclcpp::MessageInfo &>( + "bind_method_with_info") + ), + format_parameter_with_ta +); From 9904ebb076eec3ce9b2a59b3d96be90b4bbc73e5 Mon Sep 17 00:00:00 2001 From: mini-1235 Date: Sun, 7 Dec 2025 12:09:45 +0000 Subject: [PATCH 4/4] Format Signed-off-by: mini-1235 --- rclcpp/test/rclcpp/test_any_subscription_callback.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/rclcpp/test/rclcpp/test_any_subscription_callback.cpp b/rclcpp/test/rclcpp/test_any_subscription_callback.cpp index a256e39ae1..f292399695 100644 --- a/rclcpp/test/rclcpp/test_any_subscription_callback.cpp +++ b/rclcpp/test/rclcpp/test_any_subscription_callback.cpp @@ -769,7 +769,6 @@ INSTANTIATE_TEST_SUITE_P( format_parameter_with_ta ); - // // Versions of `std::shared_ptr` //