Proper controller update rate

[saikishor]

This PR adds a logic to have a proper controller update rate without needing to have a perfect divisor with respect to the controller manager's update rate

[christophfroehlich]

Do I understand this correctly? With this change, the average update rate over a longer period is the desired one, in contrast to previous version with the nearest integer division factor resulting in a constant error of the update rate.

Although, a warning would be maybe fine if the configured update rate cannot be ensured exactly (having a jitter in the commands)

[saikishor]

Do I understand this correctly? With this change, the average update rate over a longer period is the desired one, in contrast to previous version with the nearest integer division factor resulting in a constant error of the update rate

Yes, you are right @christophfroehlich , with the previous version, if your controller is configured to 40Hz and your controller manager runs at 100Hz, then it prints a warning that it would run it ar 50 Hz, and the controller is updated at 50Hz instead of the desired 40Hz. With this PR, you will ensure the 40 Hz rate, but then your period between the update cycles won't be constant as it is not a perfect divisor

Although, a warning would be maybe fine if the configured update rate cannot be ensured exactly (having a jitter in the commands)

Yes for the previous implementation, it is already printed and now, as we achieve what we have configured, this is removed in this PR.

[codecov-commenter]

Codecov Report

Merging #1105 (3b18dc1) into master (8141212) will decrease coverage by 0.08%.
The diff coverage is 31.57%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1105      +/-   ##
==========================================
- Coverage   31.83%   31.75%   -0.08%     
==========================================
  Files          94       94              
  Lines       10483    10486       +3     
  Branches     7136     7146      +10     
==========================================
- Hits         3337     3330       -7     
  Misses        801      801              
- Partials     6345     6355      +10     

Flag	Coverage Δ
unittests	31.75% <31.57%> (-0.08%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
.../include/controller_manager/controller_manager.hpp	18.18% <0.00%> (-0.57%)	⬇️
...ontroller_manager/test/test_controller_manager.cpp	27.20% <50.00%> (-1.49%)	⬇️
controller_manager/src/controller_manager.cpp	38.20% <25.00%> (-0.36%)	⬇️

[christophfroehlich]

Although, a warning would be maybe fine if the configured update rate cannot be ensured exactly (having a jitter in the commands)

Yes for the previous implementation, it is already printed and now, as we achieve what we have configured, this is removed in this PR.

I'd still throw a warning, because the update rate won't be constant and exactly as expected.

[saikishor]

I'd still throw a warning, because the update rate won't be constant and exactly as expected.

Sure, makes sense. I can add something like, The update period of the controller won't be constant for every update cycle, unless the update rate is a perfect divisor.

Thanks for the feedback @christophfroehlich. Appreciate it.

[destogl]

Great work, just few comments.

[destogl]

Thanks for the following up on this!