From fb445645f619391ca27fb5656bf0d91a3135d35d Mon Sep 17 00:00:00 2001 From: Skyler Medeiros Date: Fri, 17 Jul 2026 00:13:07 -0700 Subject: [PATCH] Deprecate the experimental EventsExecutor (#3192) Signed-off-by: Skyler Medeiros Signed-off-by: Janosch Machowinski Co-authored-by: Skyler Medeiros Co-authored-by: Janosch Machowinski (cherry picked from commit 527275027cee68bf817a5d0824f15803114a4d2d) # Conflicts: # rclcpp/test/rclcpp/executors/test_events_executor.cpp --- .../events_executor/events_executor.hpp | 6 +- .../events_executor/events_executor.cpp | 6 ++ .../test/rclcpp/executors/executor_types.hpp | 7 ++ .../rclcpp/executors/test_events_executor.cpp | 66 +++++++++++++++++++ .../executors/test_executors_warmup.cpp | 6 ++ rclcpp_action/test/test_server.cpp | 7 +- .../src/component_container_event.cpp | 6 ++ 7 files changed, 100 insertions(+), 4 deletions(-) diff --git a/rclcpp/include/rclcpp/experimental/executors/events_executor/events_executor.hpp b/rclcpp/include/rclcpp/experimental/executors/events_executor/events_executor.hpp index aede14dbc9..b44fe6d6b9 100644 --- a/rclcpp/include/rclcpp/experimental/executors/events_executor/events_executor.hpp +++ b/rclcpp/include/rclcpp/experimental/executors/events_executor/events_executor.hpp @@ -57,7 +57,11 @@ namespace executors * executor.spin(); * executor.remove_node(node); */ -class EventsExecutor : public rclcpp::Executor +class +[[deprecated( + "rclcpp::experimental::executors::EventsExecutor is deprecated and will be removed in " + "m-turtle. use rclcpp::executors::EventsCBGExecutor in single-threaded mode instead")]] +EventsExecutor : public rclcpp::Executor { public: RCLCPP_SMART_PTR_DEFINITIONS(EventsExecutor) diff --git a/rclcpp/src/rclcpp/experimental/executors/events_executor/events_executor.cpp b/rclcpp/src/rclcpp/experimental/executors/events_executor/events_executor.cpp index 4a3f8f796f..85ad3d2ed2 100644 --- a/rclcpp/src/rclcpp/experimental/executors/events_executor/events_executor.cpp +++ b/rclcpp/src/rclcpp/experimental/executors/events_executor/events_executor.cpp @@ -18,10 +18,14 @@ #include #include +#include "rcpputils/compile_warnings.hpp" #include "rcpputils/scope_exit.hpp" using namespace std::chrono_literals; +// Disable deprecation warnings while maintaining the EventsExecutor +RCPPUTILS_DEPRECATION_WARNING_OFF_START + using rclcpp::experimental::executors::EventsExecutor; EventsExecutor::EventsExecutor( @@ -454,3 +458,5 @@ EventsExecutor::add_notify_waitable_to_collection( {this->notify_waitable_, weak_group_ptr} }); } + +RCPPUTILS_DEPRECATION_WARNING_OFF_STOP diff --git a/rclcpp/test/rclcpp/executors/executor_types.hpp b/rclcpp/test/rclcpp/executors/executor_types.hpp index f5dd175d2b..b5e857aeb3 100644 --- a/rclcpp/test/rclcpp/executors/executor_types.hpp +++ b/rclcpp/test/rclcpp/executors/executor_types.hpp @@ -20,11 +20,16 @@ #include #include +#include "rcpputils/compile_warnings.hpp" + #include "rclcpp/executors/events_cbg_executor/events_cbg_executor.hpp" #include "rclcpp/experimental/executors/events_executor/events_executor.hpp" #include "rclcpp/executors/single_threaded_executor.hpp" #include "rclcpp/executors/multi_threaded_executor.hpp" +// Disable deprecation warnings while maintaining the EventsExecutor +RCPPUTILS_DEPRECATION_WARNING_OFF_START + using ExecutorTypes = ::testing::Types< rclcpp::executors::SingleThreadedExecutor, @@ -65,4 +70,6 @@ using StandardExecutors = rclcpp::executors::EventsCBGExecutor, rclcpp::experimental::executors::EventsExecutor>; +RCPPUTILS_DEPRECATION_WARNING_OFF_STOP + #endif // RCLCPP__EXECUTORS__EXECUTOR_TYPES_HPP_ diff --git a/rclcpp/test/rclcpp/executors/test_events_executor.cpp b/rclcpp/test/rclcpp/executors/test_events_executor.cpp index 447567047d..16e098e285 100644 --- a/rclcpp/test/rclcpp/executors/test_events_executor.cpp +++ b/rclcpp/test/rclcpp/executors/test_events_executor.cpp @@ -20,9 +20,14 @@ #include "rclcpp/experimental/executors/events_executor/events_executor.hpp" +#include "rcpputils/compile_warnings.hpp" + #include "test_msgs/srv/empty.hpp" #include "test_msgs/msg/empty.hpp" +// Disable deprecation warnings while maintaining the EventsExecutor +RCPPUTILS_DEPRECATION_WARNING_OFF_START + using namespace std::chrono_literals; using rclcpp::experimental::executors::EventsExecutor; @@ -615,3 +620,64 @@ TEST_F(TestEventsExecutor, waitable_with_timer) EXPECT_TRUE(waitable->timerTriggeredWaitable()); } +<<<<<<< HEAD +======= + +// Regression test for https://github.com/ros2/rmw_cyclonedds/pull/584 +// EventsExecutor must receive transient-local cached messages +// when the subscriber uses KEEP_ALL history. +// +// Bug: rmw_cyclonedds reports depth=0 for KEEP_ALL, and +// rmw_subscription_set_on_new_message_callback clips unread_count to depth +// via std::min(unread_count, 0) == 0, causing the executor to never take the +// cached message. +// +// This test publishes before the subscriber exists (transient-local caching), +// then subscribes with KEEP_ALL + transient-local via EventsExecutor and +// verifies the cached message is delivered. +TEST_F(TestEventsExecutor, keep_all_transient_local_receives_cached_message) +{ + // Publisher node: transient-local, reliable, depth=1 (standard /tf_static pattern) + auto pub_node = std::make_shared("pub_node"); + rclcpp::QoS pub_qos(1); + pub_qos.transient_local().reliable(); + auto publisher = pub_node->create_publisher("tl_test_topic", pub_qos); + + // Publish BEFORE subscriber exists — DDS caches this for late joiners + publisher->publish(test_msgs::msg::Empty{}); + std::this_thread::sleep_for(500ms); + + // Subscriber node: KEEP_ALL + transient-local (what rosbag2 adapt_request_to_offers produces) + auto sub_node = std::make_shared("sub_node"); + rclcpp::QoS sub_qos(rclcpp::KeepAll{}); + sub_qos.transient_local().reliable(); + + std::atomic received{0}; + auto subscription = sub_node->create_subscription( + "tl_test_topic", sub_qos, + [&received](test_msgs::msg::Empty::ConstSharedPtr) { + received.fetch_add(1); + }); + + EventsExecutor executor; + executor.add_node(sub_node); + + auto start = std::chrono::steady_clock::now(); + while (received.load() == 0 && + (std::chrono::steady_clock::now() - start) < 5s) + { + executor.spin_some(100ms); + } + executor.remove_node(sub_node); + + // With the rmw fix, the cached message must be received. + // Without the fix, received == 0 because the callback fires with 0 events. + EXPECT_GE(received.load(), 1) + << "EventsExecutor did not receive the transient-local cached message " + "with KEEP_ALL QoS. This indicates a regression in " + "rmw_subscription_set_on_new_message_callback where unread_count " + "is incorrectly clipped to depth=0 for KEEP_ALL history."; +} + +RCPPUTILS_DEPRECATION_WARNING_OFF_STOP +>>>>>>> 5272750 (Deprecate the experimental EventsExecutor (#3192)) diff --git a/rclcpp/test/rclcpp/executors/test_executors_warmup.cpp b/rclcpp/test/rclcpp/executors/test_executors_warmup.cpp index ab5a362f17..257d009c9d 100644 --- a/rclcpp/test/rclcpp/executors/test_executors_warmup.cpp +++ b/rclcpp/test/rclcpp/executors/test_executors_warmup.cpp @@ -26,6 +26,8 @@ #include #include +#include "rcpputils/compile_warnings.hpp" + #include "rclcpp/rclcpp.hpp" #include "test_msgs/msg/empty.hpp" @@ -146,11 +148,13 @@ TYPED_TEST(TestExecutorsWarmup, spin_some_doesnt_require_warmup) // TODO(alsora): currently only the events-executor passes this test. // Enable single-threaded and multi-threaded executors // when https://github.com/ros2/rclcpp/pull/2595 gets merged + RCPPUTILS_DEPRECATION_WARNING_OFF_START if ( !std::is_same()) { GTEST_SKIP(); } + RCPPUTILS_DEPRECATION_WARNING_OFF_STOP ExecutorType executor; @@ -191,11 +195,13 @@ TYPED_TEST(TestExecutorsWarmup, spin_some_doesnt_require_warmup_with_cbgroup) // TODO(alsora): currently only the events-executor passes this test. // Enable single-threaded and multi-threaded executors // when https://github.com/ros2/rclcpp/pull/2595 gets merged + RCPPUTILS_DEPRECATION_WARNING_OFF_START if ( !std::is_same()) { GTEST_SKIP(); } + RCPPUTILS_DEPRECATION_WARNING_OFF_STOP ExecutorType executor; diff --git a/rclcpp_action/test/test_server.cpp b/rclcpp_action/test/test_server.cpp index a7c109d868..33a387a497 100644 --- a/rclcpp_action/test/test_server.cpp +++ b/rclcpp_action/test/test_server.cpp @@ -46,10 +46,11 @@ class TestServer : public ::testing::Test rclcpp::shutdown(); } + template std::shared_ptr send_goal_request( rclcpp::Node::SharedPtr node, GoalUUID uuid, - rclcpp::Executor & executor, + ExecutorType & executor, std::chrono::milliseconds timeout = std::chrono::milliseconds(-1), bool executor_owns_node = false) { @@ -1067,7 +1068,7 @@ TEST_F(TestServer, goals_expired_with_events_executor) rclcpp::ExecutorOptions opts; opts.context = node->get_node_base_interface()->get_context(); - rclcpp::experimental::executors::EventsExecutor executor(opts); + rclcpp::executors::EventsCBGExecutor executor(opts, 1); executor.add_node(node); const std::vector uuids{ {{1, 2, 3, 40, 5, 6, 70, 8, 9, 1, 11, 120, 13, 140, 15, 160}}, @@ -1132,7 +1133,7 @@ TEST_F(TestServer, goals_expired_with_events_executor) // Wait for the result request to be received ASSERT_EQ( rclcpp::FutureReturnCode::SUCCESS, - executor.spin_until_future_complete(future)); + executor.spin_until_future_complete(future, std::chrono::seconds(20))); auto response = future.get(); EXPECT_EQ(action_msgs::msg::GoalStatus::STATUS_SUCCEEDED, response->status); diff --git a/rclcpp_components/src/component_container_event.cpp b/rclcpp_components/src/component_container_event.cpp index 30d40baa39..fcf0a851ae 100644 --- a/rclcpp_components/src/component_container_event.cpp +++ b/rclcpp_components/src/component_container_event.cpp @@ -14,6 +14,7 @@ #include +#include "rcpputils/compile_warnings.hpp" #include "rclcpp/utilities.hpp" #include "rclcpp/experimental/executors/events_executor/events_executor.hpp" @@ -27,7 +28,12 @@ int main(int argc, char * argv[]) /// Component container with an events executor. rclcpp::init(argc, argv); + + // Disable deprecation warnings while maintaining the EventsExecutor + RCPPUTILS_DEPRECATION_WARNING_OFF_START auto exec = std::make_shared(); + RCPPUTILS_DEPRECATION_WARNING_OFF_STOP + auto node = std::make_shared(exec); exec->add_node(node); exec->spin();