Add EventsExecutor

[irobot-ros]

This PR introduces the changes required to implement the EventsExecutor design in rmw.
See design and Discourse post.

The new executor uses an events queue and a timers manager as opposed to waitsets, to efficiently execute entities with work to do.

This new executor greatly reduces CPU usage of a ROS 2 application.
See the blog post for more details on the tests that we run.

The bulk of the changes for this implementation are in the rclcpp layer, with some minor changes in other repositories (rcl, rmw, rmw_implementation) for forwarding entities, the declaration of some data types in rcutils, and finally some additional changes in the vendor specific rmw implementations..
We currently implemented this only on top of the default ROS middleware fastrtps, while we provided stubs for other middlewares.

See the main PR to rclcpp ros2/rclcpp#1416.

The current implementation does not support ROS 2 actions, which will be added in a follow up PR.

Developed by iRobot
Mauro Passerino
Lenny Story
Alberto Soragna

---

Connects to:

• design:
  • Events Executor design design#305

[ros-discourse]

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros2-middleware-change-proposal/15863/20

[ros-discourse]

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros2-middleware-change-proposal/15863/29

[ivanpauno]

Thanks for working on this!

I have left some minor comments about the API, but I think it doesn't make a big difference.

[ivanpauno]

I think this looks good.

I still have a minimal preference for not having an rmw_listener_event_t argument in the listener callbacks and making that part of user_data (which will require registering not exactly the same function in all listeners of the event executor, but it can be an small wrapper of a common function).
Without the rmw_listener_event_t argument, the extra void * <type>_handle is also not required in the functions that register a listener.

Anyways, I think that adding this makes sense.
I would recommend ignoring my suggestions until @wjwwood also reviews (to avoid unnecessary iterations).

edit: I also think it would be fine to have two void * user data arguments (e.g.: void * arg1, void * arg2 and maybe, arg3), and in that case an extra wrapper is not needed.

[rpaaron]

I see there is some discussion over on ros2/design#305 about executors being supported out of tree. Will this PR get merged even if the other is still being discussed?

[wjwwood]

I've been testing these changes with some rebasing. I think things are looking good. I still have one item to follow up on design wise, but in the mean time we need to clean up the commit history. Ideally, we could organize the commits (maybe squash them into one or at least fewer commits), but at the very least we need to have all the commits passing the DCO checker. This goes for the other pull requests too.

I would offer to do this for you, but I really need you guys to do the DCO signing.

I cannot test it until master is merged in or rebased against master (my preference).

[wjwwood]

I see there is some discussion over on ros2/design#305 about executors being supported out of tree. Will this PR get merged even if the other is still being discussed?

That's the plan. Merge the rmw/rcl changes related to "listener" style API, and then iterate on the c++ executor designs asynchronously.

[wjwwood]

@irobot-ros or @mauropasse, is it ok if I push directly (maybe force push to rebase) to this pull request branch? Or should I make my own?

[irobot-ros]

@wjwwood yes sure, go ahead and push to this branch if you prefer.

FYI we just pushed a commit (here and in other PRs) that removes guard condition listeners as discussed in last Middleware WG.

[wjwwood]

CI:

• Linux
• Linux-aarch64
• macOS
• Windows

[wjwwood]

@irobot-ros @mauropasse there are some test failures (I can fix the uncrustify one for rmw_fastrtps), but the others need some investigation, so if you have any time that would be great:

https://ci.ros2.org/job/ci_linux/14198/testReport/

I know it must be getting close to you guy's weekend, so if not I'll try to investigate tomorrow some if I have time.

[irobot-ros]

I fixed the fastrtps uncrustify errors.
I had a quick look at the other failures, but I don't really know the internals of how QoS events work.
@mauropasse should be able to address them, but he will be back on Tuesday.

[mauropasse]

@irobot-ros @mauropasse there are some test failures (I can fix the uncrustify one for rmw_fastrtps), but the others need some investigation, so if you have any time that would be great: https://ci.ros2.org/job/ci_linux/14198/testReport/

projectroot.test.rclcpp.test_qos_event fails because assigning listener callbacks in CycloneDDS breaks the wait_set based executors notify mechanisms: see ros2/rmw_cyclonedds#256 (comment) (Issue 1).
A temporary workaround is done in this PR: eclipse-cyclonedds/cyclonedds#699
The test_qos_event is based on the SingleThreadedExecutor so is never notified about a happened event.

I'll take a look to the other failing tests.

[wjwwood]

I'm rerunning CI for this without the rclcpp pull request (ros2/rclcpp#1579):

• Linux
• Linux-aarch64
• macOS
• Windows

If it looks good, then we can at least take those API's in, and maybe we can do ABI stable additions to rclcpp after the release. I'm going to propose we extend our pimpl pattern usage before the release to make that possible.

[mauropasse]

I went through the failing tests

projectroot.test.test_graph__rmw_cyclonedds_cpp
projectroot.test.test_events__rmw_cyclonedds_cpp
rcl.NodeGraphMultiNodeFixture.test_node_info_subscriptions
rcl.NodeGraphMultiNodeFixture.test_node_info_publishers
rcl.NodeGraphMultiNodeFixture.test_node_info_services
rcl.NodeGraphMultiNodeFixture.test_node_info_clients
rcl.test_events__rmw_cyclonedds_cpp.gtest.missing_result

They are all fixed with my temporary workaround on CycloneDDS eclipse-cyclonedds/cyclonedds#699 , the issue is what I explained on #286 (comment)