ros2 / ros2/rclcpp

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

Open
#2,432 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

backlog
Dominant language
C++
Stars
805
Forks
564
Avg merge
1d 17h
Merged PRs (30d)
27

Description

Bug report

Required Info:

  • Client library (if applicable):
    rclcpp
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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading the rclcpp::experimental::TimersManager implementation and how its timerheap is used to select the first timer. Reproduce the reported case with wall timers and simulation timers running at different speeds, then verify that every ready timer can be considered rather than only the heap head.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
robotics
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.