Add asyncio action server example

[tupiznak]

Added the ability to use the asyncio library inside the callback. It can be convenient.

[ivanpauno]

I don't see how this helps much to be honest.

This is calling run_until_complete(), which means that we're blocking.
Blocking the rclpy executor is not a good idea for the same reasons why it's not a good idea to block the asyncio executor.

I'm open to merge this if you can provide an example of when this is really useful, but of the top off my head I wouldn't recommend this pattern.

---

I agree with the spirit of this though, I would like to have an asycio friendly executor but that's not currently available.
My idea for that would be to have a thread always waiting on a waitset, and to have API
like async take_msg(self) in Subscription which would:

• Add the Subscription handle to the waitset, with an asyncio.Event associated to it. The future returned is the Event.
• The thread handling the waitset will trigger the event when the subscription is ready, and potentially delete the subscription handle from the waitset if no one else is waiting on it.

[ivanpauno]

cc @hidmic @sloretz

[tupiznak]

Yeah, you are right, this is not the best way to embed one executor into another.
But as you said the ideal option is not possible since it requires changes inside the rclpy library. Therefore I suggest a compromise. Thanks to such a hack, it became possible to use asyncio inside the node code (for example, the use of await asyncio.sleep(1) in the line 89).

Globally it is a blocking code, but only one thread is blocked when it is receiving a command from a client. Everething else inside this is asynchronous.

In my current job I have to write asynchronous code. But due to the impossibility of using asyncio inside the node, I had to reinvent the wheel and rewrite things like asyncio.sleep, asyncio.gather and so on. On top of that you can use pytest-asyncio, but that's another story.

[ivanpauno]

Sorry, I'm still not convinced of the value of the example.
I will let the maintainers of the package decide if they want to merge the example or close.

[ivanpauno]

But as you said the ideal option is not possible since it requires changes inside the rclpy library.

Thinking about this a bit more, there are ways to integrate the rclpy executor with the asyncio executor without blocking and without modifying rclpy.

e.g. you can run a coroutine from the ros callback with https://docs.python.org/3/library/asyncio-task.html#asyncio.run_coroutine_threadsafe and you will need to be both running the asycio loop and the ros loop (aka spinning) in different threads.
That approach is more unlikely to have issues, as it doesn't block the rclpy executor.

[tupiznak]

I tried many different options and that one is the best I could think of.
Moving asyncio loop and ros executor to different threads is not convenient because this approach doesn't allow to use something like await asyncio.sleep() inside a node callback. It will be necessary to handle all asyncio features outside of ros code.
If you could show another example that would solve that issues and have the same functionality, it would be great.

[hidmic]

Long term, I agree with @ivanpauno. To have RMW events being waited on a background thread and dispatched to an asyncio loop is probably the simplest approach. Incoming messages from subscriptions would be serviced in the loop, and not through a callback invoked from the background thread.

Short term, I'm not against local asyncio loops to bridge asynchronous code, so long as its limitations are understood (e.g. it's blocking, it's short lived, it's not safe for two loops in different threads to interact). To merge this patch and make that a recommendation is a different story.

[ivanpauno]

Moving asyncio loop and ros executor to different threads is not convenient because this approach doesn't allow to use something like await asyncio.sleep() inside a node callback

You can do that using https://docs.python.org/3/library/asyncio-task.html#asyncio.run_coroutine_threadsafe.

something like await asyncio.sleep() inside a node callback. It will be necessary to handle all asyncio features outside of ros code.

You can already do that in rclpy though https://github.com/ros2/rclpy/blob/95fc7495efded6463a0be47ce6fbed4d9ce26746/rclpy/test/test_executor.py#L166.
IIUC we run that in a non-blocking way correctly, @sloretz probably knows better.

The only problem is that not all asyncio coroutines can be called, because there's no asycnio event loop.

[tupiznak]

You can already do that in rclpy though https://github.com/ros2/rclpy/blob/95fc7495efded6463a0be47ce6fbed4d9ce26746/rclpy/test/test_executor.py#L166.

await asyncio.sleep(0) equivalent generator https://github.com/python/cpython/blob/5024dc1c6e08247693aea6ad6e225ec5dcaf0721/Lib/asyncio/tasks.py#L586. I have seen this code.

[sloretz]

Whoops, I forgot to submit my review. I added one more comment since I revisited this recently.

[sloretz]

Like @ivanpauno said, this is blocking the rclpy executor, which is not recommended.

[sloretz]

I understand that this allows using asyncio primitives, but this example is equivalent to getting rid of the asyncio loop and using time.sleep(1) in the callback.

[sloretz]

ros2/rclpy#962 has an idea where an event loop controls the execution, and spin_once() with a very small timeout is used to handle callbacks. ros2/rclpy#971 improves that use case. That avoids blocking the executor, but prevents using a MultiThreadedExecutor.