Proposal: `rcl_executors` - a unified, canonical reference executor package for all client libraries

I have been playing with the old and new (lyrical) executor and things at a lower level.

  • The wait_set is very messy, so I totally support getting rid of it.
  • The new Python implementation in lyrical is far better than jazzy (I guess it comes from the EventsCBGExecutor model), let me just explain so we are on the same page:
    • Actually the set_on_new_message_callback and take_message methods were added in lyrical that’s why there has never been any docs or discussion about them.
    • entity.set_on_new_message_callback(my_callback) bears no data and signals that an event happened on the entity (sub/srv…). This is (I guess) executed by the RMW thread for minimal overhead, so it must not be blocked, otherwise RMW will stall.
    • msg = entity.take_message() is what gets the data (from any thread you want), by taking it from the RMW queue.
    • That’s very good to separate the event from the data . Let’s say I just want to know that there’s a message, but I want to consume it later (typically for: batching messages and topics together, or avoiding deserialization, or building backpressure). It is now possible with this structure and was not possible before with data-bearing callbacks.
    • With this I can create a whole ROS application in rclpy without an executor and with executor back-pressure in 50 lines.

So set_on_new_message_callback and take_message are very very powerful and I think they should be exposed. However, I don’t think that counts as a " unified, canonical reference executor" because the user is in charge of scheduling and execution. If we want an executor to spin and normal callbacks, from this, it should be fairly easy to wake-up one (or many) thread to execute msg = entity.take_message() and the user callback. I however don’t know what’s happening on the C side of things.

Example of using rclpy with no executor using set_on_new_message_callback and take_message:

import asyncio
from contextlib import ExitStack, suppress

import rclpy
from rclpy.impl.implementation_singleton import rclpy_implementation as _rclpy
from rclpy.node import Node
from rclpy.qos import qos_profile_default
from rclpy.type_support import check_is_valid_msg_type
from std_msgs.msg import String


async def consume(ros_sub: _rclpy.Subscription, queue: asyncio.Queue):
    msg, info = ros_sub.take_message(String, False)
    while msg is not None:
        await queue.put(msg)
        msg, info = ros_sub.take_message(String, False)


async def main():
    with ExitStack() as es:
        rclpy.init()
        es.callback(rclpy.shutdown)
        n = Node("afor_dbg")
        es.callback(n.destroy_node)
        check_is_valid_msg_type(String)

        ros_sub = _rclpy.Subscription(
            n.handle,
            String,
            n.resolve_topic_name("example/talker"),
            qos_profile_default.get_c_qos_profile(),
        )
        es.callback(n.destroy_subscription, ros_sub)
        queue: asyncio.Queue[String] = asyncio.Queue()
        loop = asyncio.get_event_loop()

        def react(*_):
            asyncio.run_coroutine_threadsafe(consume(ros_sub, queue), loop)

        ros_sub.set_on_new_message_callback(react)
        es.callback(ros_sub.clear_on_new_message_callback)

        while 1:
            msg = await queue.get()
            print(msg)


if __name__ == "__main__":
    with suppress(KeyboardInterrupt):
        asyncio.run(main())

4 Likes

I think you are missing something. Running without an executor was always possible, even with the waitset approach. In this case you would need to manually create the waitset for polling.

Even though the idea of having per language tuned execution patterns etc sounds nice, up until recently the reality was different:
We had multiple times the almost identical implementation of executors in rclpy and rclcpp. Every time with different bugs and features.
From a pure maintenance point of view this situation is a nightmare.

Therefore I support the proposal from @skye.galaxy
From my point of view it makes total sense to have a set of ‘base executors’ that fully share the implementation somewhere above rcl. Especially to reduce the maintenance burden.

Just to clarify, language specific executors can still be a thing, but this it out of the scope of this proposal.

3 Likes

We had a productive discussion about this proposal in the client library working group today, and landed on the following:

  • We decided that we will probably be a lot more successful at our stated goals, and run into less issues overall, if we had the whole package in C++ instead of behind a C API, and include bindings for other languages.
  • A lot of the C API stuff from rcl / rmw / etc came from a supposed need to support constrained embedded systems. Nowadays, realistically if you are using ROS 2, you in all likelihood are already targeting a platform that needs a C++ compiler anyway. If you’re putting ROS on a microcontroller you’re probably using rclc / micro-ros anyways (or maybe you’re one of the cool kids trying out zenoh-pico or pico-ros!)
  • The landscape for embedded applications is a lot different nowadays. Lots of firmware is being written in Rust now, and even in C++ there are tools like etlcpp specifically designed for avoiding dynamic allocation on embedded systems while keeping the nice abstractions C++ provides.
  • Restricting ourselves to a pure C interface so that we could maybe support some of the more niche realtime / alloc constrained systems is a bit more scope than the original intention of “let’s dedup the highly similar C++ code paths that exist in rclpy and rclcpp today”.
  • We want to focus on de-duplicating the implementations first before we add additional features.

The draft REP I spin up out of all this will be split off into two stages of work: Part 1 will be mostly focused on the actual dedup work (and seems like a reasonably scoped task we could accomplish by Makoa Mata-Mata) and Part 2 will focus on how we can build on top of this new package with different execution policies sharing the same logic, and incorporate all of the new and missing features that we want to add.

5 Likes