Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

timer accumulating delay #289

Closed
dirk-thomas opened this issue Aug 31, 2018 · 0 comments
Closed

timer accumulating delay #289

dirk-thomas opened this issue Aug 31, 2018 · 0 comments
Assignees
Labels
bug Something isn't working

Comments

@dirk-thomas
Copy link
Member

The current implementation of timers is accumulating the delay from each cycle which results in a callback frequency less then the specified rate. Since the delay per cycle is (on my machine) around 0.2ms that is most noticeable when trying to achieve a high rate like 1000Hz since then the delay per cycle becomes significant (~20%) so each callback is only triggered around every 1.2ms.

The problem is cause by the way the timers are implemented at the moment. The waitset aims to wake up when the specified cycle duration has elapsed since the callback N-1 has happened to invoke the callback N. When the timer is then called (rcl_timer_call) it gets the current time from the clock and uses that to record the time when the callback was called for the calculation when N+1 is due.

The approach to start each cycle time when the function rcl_timer_call is being called makes each cycle a little bit longer than desired (the delta between when the timer was expected to be ready, over the waitset waking up, and invoking the timer function).

The goal is that each cycle moves the expected time forward by exactly the specified duration (without accumulating the delay between ready / wake up / call). The only exception to this case is when the cycle time is that far behind that this would "queue up" calls. [I would suggest to handle this in the following way: as long as the actual timestamp for the callback N is within the interval of desired times for N and N+1 the timer sticks to the "fixed" schedule. If the actual time becomes larger than N+1 - which means the timer didn't fire until a second cycle had already passed - the timer "resets" itself and sets the desired time of N+1 to the actual time of N plus the specified cycle time.]

Some examples illustrating the current problem as well as the desired behavior.

  • N-1 happened at T=1ms
  • N is supposed to happen at T=2ms
  • the call of rcl_timer_call for N is actually happening at T=2.2ms
  • N+1 is then scheduled to be triggered at T=3.2ms

The goal is that N+1 is scheduled to be triggered at T=3ms (without including the "trigger delay" of the previous cycle).

The timer callback should still receive the duration since the actual previous invocation happened rather then the duration since the previous invocation was supposed to happen. So for the example above the callback for N+1 should receive the duration parameter of 0.8ms (not considering any delay of the invocation of N+1 itself for ease of understanding). In the real world the delay in each cycle will be very similar so the callback should be around the desired duration. The important difference is that is the callback would accumulate the received duration parameters from eah cycle the sum should exactly reflect the passed time (and not miss any hidden delays).

See ros2/ros2cli#141 for one symptom of this bug.

@ros2/team Please comment / review this proposal before we jump into implementing it.

@dirk-thomas dirk-thomas added bug Something isn't working in review Waiting for review (Kanban column) labels Aug 31, 2018
@dirk-thomas dirk-thomas self-assigned this Aug 31, 2018
@dirk-thomas dirk-thomas added in progress Actively being worked on (Kanban column) in review Waiting for review (Kanban column) and removed in review Waiting for review (Kanban column) in progress Actively being worked on (Kanban column) labels Sep 6, 2018
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Sep 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant