ros2 / rclcpp

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

rclcpp::experimental::TimersManager most likely broken for mixed timers #2432

Open jmachowinski opened 7 months ago

jmachowinski commented 7 months ago

Bug report

Required Info:

Additional information

I went through the code of rclcpp::experimental::TimersManager , as I wanted to use it for my purposes, and noticed, there is a logical bug in the implementation. The class always tries to execute the first timer in the timerheap, going with the assumption, that this is the next one to be ready. However, this assumption is breaks, it you have wall timers and sim timers and the sim timers run with a different speed as the wall timers. In this case the head of the timerheap might not be ready, but other timers inside the heap might be.

@alsora As this is your code, you might want to have a look.

alsora commented 7 months ago

Yes, this is a known limitation. If you need to combine different types of timers, you currently need to create multiple exexutors

As far as I remember, this will throw an exception and won't just silently behave incorrectly.