ros2 / rclcpp

rclcpp (ROS Client Library for C++)
Apache License 2.0
513 stars 412 forks source link

[documentation] Explain when `MultiThreadedExecutor` is running multiple threads #2454

Open Cakem1x opened 3 months ago

Cakem1x commented 3 months ago

This is a tiny PR that adds a class docstring for the MultiThreadedExecutor. It includes a note about which methods support multithreaded execution.

Please let me know if this is not the right way to suggest additions to the documentation. Also, please double check if the note is actually true or can be improved.

My context: I have a unit test with two nodes that communicate with each other and I use the MultiThreadedExecutor to spin both of these nodes. One node publishes a TF, the other one regularly waits for that TF.

In the individual test cases, I have been using multi_threaded_executor_.spin_until_future_complete(some_future);. This leads to a flaky test that sometimes succeeds and sometimes fails. The reason for that seems to be that the MultiThreadedExecutor is only using a single thread when spin_until_future_complete is used. The test would fail (timeout) if the work item from node that waits for the TF gets executed first. Waiting for the TF blocks the main thread until timeout, while the other node that should supply that TF will not get executed.

After changing my test setup to use multi_threaded_executor_.spin(); in a separate thread and just do some_future.wait_for(...) in the test cases, everything is working fine.

So, after finding all that out, I thought I could maybe help others by adding a note somewhere. Just skimming through the API docs, that behavior was not clear to me.

Thanks for having a look!

fujitatomoya commented 3 months ago

@Cakem1x btw, can you also address DCO error?