Skip to content

Deprecate the experimental EventsExecutor#3192

Merged
ahcorde merged 6 commits into
rollingfrom
skyegalaxy/deprecate-events-executor
Jul 17, 2026
Merged

Deprecate the experimental EventsExecutor#3192
ahcorde merged 6 commits into
rollingfrom
skyegalaxy/deprecate-events-executor

Conversation

@skyegalaxy

@skyegalaxy skyegalaxy commented Jul 13, 2026

Copy link
Copy Markdown
Member

Description

This PR adds a compile-time deprecation warning when using the experimental EventsExecutor, warning to switch to the new Callback Group Events Executor in single-threaded mode instead.

Tests which exercise the EventsExecutor have deprecation warnings disabled while it is still in the source tree.
Also replaces one usage of the old executor with the new one in rclcpp_action/test_server.cpp.

image

Fixes # (issue)

Is this user-facing behavior change?

Users will now get a compiler warning when using the experimental EventsExecutor in their application code

Did you use Generative AI?

Claude Opus 4.8 was used to identify spots in the codebase where the deprecation warning suppression should be introduced.

Additional Information

This ought to be backported to at least Lyrical

@skyegalaxy
skyegalaxy requested a review from jmachowinski July 13, 2026 22:37
@mergify

mergify Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@ahcorde

ahcorde commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Pulls: #3192
Gist: https://gist.githubusercontent.com/ahcorde/1a920ab6b77eb06bacef590a582b7738/raw/9ce7ddf78505ff1ed14376ae22de2afa6bd97923/ros2.repos
BUILD args: --packages-above-and-dependencies rclcpp rclcpp_action
TEST args: --packages-above rclcpp rclcpp_action
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/19837

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@ahcorde ahcorde left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skyegalaxy
skyegalaxy requested a review from ahcorde July 14, 2026 16:53
@skyegalaxy

Copy link
Copy Markdown
Member Author

I can see one warning https://ci.ros2.org/job/ci_linux-aarch64/22485/clang-tidy/

fixed in 3cb984c

@skyegalaxy

Copy link
Copy Markdown
Member Author
  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@fujitatomoya fujitatomoya left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm with green CI

@jmachowinski

Copy link
Copy Markdown
Collaborator

Pulls: #3192
Gist: https://gist.githubusercontent.com/jmachowinski/f03f0e83fe0a7cd86f05ef9ecfdbafd7/raw/9ce7ddf78505ff1ed14376ae22de2afa6bd97923/ros2.repos
BUILD args:
TEST args:
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/19864

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

Skyler Medeiros and others added 6 commits July 15, 2026 23:35
Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>
Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>
Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>
Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>
@skyegalaxy
skyegalaxy force-pushed the skyegalaxy/deprecate-events-executor branch from 7ff14d3 to 0bee264 Compare July 16, 2026 07:55
@skyegalaxy

Copy link
Copy Markdown
Member Author
  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@skyegalaxy

skyegalaxy commented Jul 16, 2026

Copy link
Copy Markdown
Member Author

@jmachowinski - I think the hanging rclcpp_action/test_server case actually exposes a polymorphism issue with the EventsCBGExecutor. Calling spin_until_future_complete on an EventsCBGExecutor passed in a function as an rclcpp::Executor means that the cbg executor's own spin_until_future_complete is not exercised. Executor::spin_until_future_complete_impl calls spin_once_impl, which the EventsExecutor actually overrides (it leans on more common executor code) but the EventsCBGExecutor does not, resulting in the base class version being called instead (erroneously using the waitsets).

I can get the test to pass by changing the hanging send_goal_request to use a templated arg for the executor so the proper spin_until_future_complete is called, but that feels like a workaround. Perhaps before merging this PR we ought to look into some of the code cleanup / more use of common executor code paths we've been talking about in some of the other PRs, or just have that test continue to use the EventsExecutor and change it in a future PR once we have a proper fix

@ahcorde

ahcorde commented Jul 16, 2026

Copy link
Copy Markdown
Contributor
  • Linux-aarch64 Build Status

@ahcorde
ahcorde merged commit 5272750 into rolling Jul 17, 2026
3 checks passed
@ahcorde
ahcorde deleted the skyegalaxy/deprecate-events-executor branch July 17, 2026 07:13
@skyegalaxy

Copy link
Copy Markdown
Member Author

https://github.com/Mergifyio backport lyrical

@mergify

mergify Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

backport lyrical

✅ Backports have been created

Details

Cherry-pick of 5272750 has failed:

On branch mergify/bp/lyrical/pr-3192
Your branch is up to date with 'origin/lyrical'.

You are currently cherry-picking commit 5272750.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   rclcpp/include/rclcpp/experimental/executors/events_executor/events_executor.hpp
	modified:   rclcpp/src/rclcpp/experimental/executors/events_executor/events_executor.cpp
	modified:   rclcpp/test/rclcpp/executors/executor_types.hpp
	modified:   rclcpp/test/rclcpp/executors/test_executors_warmup.cpp
	modified:   rclcpp_action/test/test_server.cpp
	modified:   rclcpp_components/src/component_container_event.cpp

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   rclcpp/test/rclcpp/executors/test_events_executor.cpp

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants