search

nobleo / static_executor

Library that adds alternative(s) to the default executor in ROS2
Apache License 2.0
2 stars 0 forks source link

Unit-tests for static executor #3

Closed alsora closed 4 years ago

alsora commented 4 years ago

Hi,

I created a bunch of "unit-tests" for the static executor. They test the following scenarios:

  1. add 1 pub node and 1 sub node to the executor and spin
  2. add 1 pub node to the executor and spin, then add 1 sub node while spinning
  3. add 1 pub node and 1 node with no entities to the executor and spin, then add 1 sub to the second node while spinning
  4. add 1 pub node and 1 sub node to the executor and spin, then while spinning remove the sub node from the executor

You can find them here https://github.com/alsora/ros2-code-examples/blob/master/simple_static_executor/static_executor_test.cpp

These tests are meant to be used with your rclcpp version (nobleo/feature/static_executor), I opened the ticket here since it was not possible in that repo.

To build colcon build --packages-select simple_static_executor To run ./install/simple_static_executor/lib/simple_static_executor/static_executor_test 2 Where the number at the end denotes the id of the test you want to run (1, 2, 3, 4)

The results are unfortunately not good.

Test 2 throws an exception 

[ERROR] [1584978899.809709877] [rclcpp]: Couldn't add guard_condition to wait set: guard_conditions set is full, at /root/ros2_ws/src/ros2/rcl/rcl/src/rcl/wait.c:456
terminate called after throwing an instance of 'std::runtime_error'
  what():  Couldn't fill wait set
Aborted (core dumped)

In test 4 the subscription keeps receiving messages after having been removed from the executor

Could you have a look at the test? Maybe I'm not using the executor as it is supposed to be used.

FYI the standard SingleThreadExecutor allows to pass all the tests successfully.

@MartinCornelis2 @mauropasse

mauropasse commented 4 years ago

The problem was that we were not using the executor's guard condition. Since the last commit we only check for node's guard condition triggered to refresh the executable list, but not executor's. I have already a fix with a new implementation with a waitable approach, I'll open the PR soon

MartinCornelis2 commented 4 years ago

Hi, Thanks for notifying me. I think I will have some time tomorrow to run the unit-tests. Would be good to recreate the issue first on my side and then test again with your new PR.

alsora commented 4 years ago

Closing this. I created real unit-tests from that file, unfortunately I have some issues when GTests are executed in parallel... As soon as I fix them I will open a PR to rclcpp