ros2_controllers Move speed limiter to control

https://github.com/ros-controls/control_toolbox/pull/212

Oct 13 '24 21:10 christophfroehlich

Codecov Report

Attention: Patch coverage is 68.08511% with 30 lines in your changes missing coverage. Please review.

Project coverage is 83.73%. Comparing base (950c9c1) to head (95a618f). Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...iff_drive_controller/src/diff_drive_controller.cpp	10.34%	20 Missing and 6 partials :warning:
diff_drive_controller/src/odometry.cpp	0.00%	4 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1315      +/-   ##
==========================================
+ Coverage   83.62%   83.73%   +0.10%     
==========================================
  Files         122      122              
  Lines       11018    11047      +29     
  Branches      934      937       +3     
==========================================
+ Hits         9214     9250      +36     
+ Misses       1495     1489       -6     
+ Partials      309      308       -1

Flag	Coverage Δ
unittests	`83.73% <68.08%> (+0.10%)`	:arrow_up:

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

Files with missing lines	Coverage Δ
...de/diff_drive_controller/diff_drive_controller.hpp	`100.00% <ø> (ø)`
...troller/include/diff_drive_controller/odometry.hpp	`100.00% <ø> (ø)`
...er/include/diff_drive_controller/speed_limiter.hpp	`100.00% <100.00%> (ø)`
...ive_controller/test/test_diff_drive_controller.cpp	`95.19% <100.00%> (+1.55%)`	:arrow_up:
diff_drive_controller/src/odometry.cpp	`43.24% <0.00%> (ø)`
...iff_drive_controller/src/diff_drive_controller.cpp	`70.68% <10.34%> (-6.30%)`	:arrow_down:

... and 1 file with indirect coverage changes

Oct 13 '24 21:10 codecov[bot]

@tpoignonec as you are currently working on this topic, could you please test this and leave a review here?

Nov 28 '24 07:11 christophfroehlich

Maybe I missed something, but shouldn't the SpeedLimiter be also modified at this point to accept the new parameters as constructor arguments? With the new parameters (i.e., max_acceleration_reverse and max_deceleration_reverse), everything is already ready to call

Totally right, I was refactoring that several times because I could not decide what to deprecate and where to use the RateLimiter class directly. Finally, I left the diff_drive_controller::SpeedLimiter class member variable to avoid breaking ABI (not sure though, if anyone would have linked against this library). The diff_drive_controller::SpeedLimiter is now a wrapper around RateLimiter, where the old constructor with the booleans is deprecated. (it is more likely that someone used the limiter class outside of this repo).

Dec 01 '24 20:12 christophfroehlich

Functionnality-wise, seems all good.

With parameters

...
linear.x.max_velocity: 1.5
linear.x.min_velocity: -1.5
linear.x.max_acceleration: 2.0
linear.x.max_deceleration: -3.0
linear.x.max_acceleration_reverse: -4.0
linear.x.max_deceleration_reverse: 5.0
linear.x.max_jerk: .NAN
linear.x.min_jerk: .NAN

The asymmetric acceleration clipping works as expected (i.e., max(acc) = 2, -3, -4, and 5, respectively): Screenshot 2024-12-02 162550