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

Move speed limiter from ros2_control repo #212

Merged
merged 15 commits into from
Nov 20, 2024
Merged

Conversation

christophfroehlich
Copy link
Contributor

@christophfroehlich christophfroehlich commented Oct 13, 2024

I thought it would be beneficial to reuse the SpeedLimiter of the diff_drive controller in the steering_controllers_library.

The SpeedLimiter might be useful for other controllers as well, so I suggest to add it to the control_toolbox repo.

I renamed it to rate_limiter, because it just limits the value and some derivatives, and could be used as control_filter plugin for other variables than speed.

Additionally, I added some tests because there were none.

@codecov-commenter
Copy link

codecov-commenter commented Oct 13, 2024

Codecov Report

Attention: Patch coverage is 43.43434% with 56 lines in your changes missing coverage. Please review.

Project coverage is 49.91%. Comparing base (d4b317c) to head (7349e7d).

Files with missing lines Patch % Lines
include/control_filters/rate_limiter.hpp 0.00% 36 Missing ⚠️
include/control_toolbox/rate_limiter.hpp 68.25% 4 Missing and 16 partials ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           ros2-master     #212      +/-   ##
===============================================
- Coverage        51.20%   49.91%   -1.30%     
===============================================
  Files               10       12       +2     
  Lines              496      595      +99     
  Branches            59       78      +19     
===============================================
+ Hits               254      297      +43     
- Misses             215      255      +40     
- Partials            27       43      +16     
Flag Coverage Δ
unittests 49.91% <43.43%> (-1.30%) ⬇️

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

Files with missing lines Coverage Δ
include/control_toolbox/rate_limiter.hpp 68.25% <68.25%> (ø)
include/control_filters/rate_limiter.hpp 0.00% <0.00%> (ø)
---- 🚨 Try these New Features:

@bmagyar
Copy link
Member

bmagyar commented Nov 2, 2024

Very simple. We should relicense all bsd to apache.

bmagyar
bmagyar previously approved these changes Nov 18, 2024
Comment on lines 112 to 115
T min_value = NAN, T max_value = NAN,
T min_first_derivative_neg = NAN, T max_first_derivative_pos = NAN,
T min_first_derivative_pos = NAN, T max_first_derivative_neg = NAN,
T min_second_derivative = NAN, T max_second_derivative = NAN);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use the C++ NaN version and not C version.

Basically, the NaN should be changed to std::numeric_limits<double>::quiet_NaN() - and probably add #include <limits> file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@bmagyar bmagyar merged commit 358552e into ros2-master Nov 20, 2024
30 checks passed
@bmagyar bmagyar deleted the mv/speed_limiter branch November 20, 2024 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants