Remove deprecation of i_min/i_max PID parameters

[christophfroehlich]

History:

• we deprecated i clamping with Redefine the antiwindup strategy and their parameters interfacing (backport #400) #410 et al, which we backported to jazzy
• then we readded it/removed the deprecation on kilted/rolling with Remove legacy and deprecated PID parameters #436

No we have it on humble, deprecated on jazzy, and have it again in kilted/rolling.

I cherry-picked the changes from #436 to have that silenced and working in jazzy.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 72.22222% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.22%. Comparing base (e92fc15) to head (d875e13).

Files with missing lines	Patch %	Lines
control_toolbox/src/pid_ros.cpp	72.72%	3 Missing ⚠️
control_toolbox/include/control_toolbox/pid.hpp	50.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            jazzy     #507      +/-   ##
==========================================
- Coverage   78.35%   78.22%   -0.13%     
==========================================
  Files          28       28              
  Lines        2194     2191       -3     
  Branches      130      129       -1     
==========================================
- Hits         1719     1714       -5     
- Misses        402      404       +2     
  Partials       73       73              

Flag	Coverage Δ
unittests	78.22% <72.22%> (-0.13%)	⬇️

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

Files with missing lines	Coverage Δ
control_toolbox/src/pid.cpp	72.19% <100.00%> (+0.14%)	⬆️
control_toolbox/test/pid_ros_parameters_tests.cpp	99.04% <ø> (ø)
control_toolbox/test/pid_tests.cpp	100.00% <100.00%> (ø)
control_toolbox/include/control_toolbox/pid.hpp	66.99% <50.00%> (-1.88%)	⬇️
control_toolbox/src/pid_ros.cpp	63.07% <72.72%> (-0.37%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[christophfroehlich]

@ViktorCVS can you have a look please if this makes sense?

[ViktorCVS]

@ViktorCVS can you have a look please if this makes sense?

Yes, this makes sense, I support this change.

[saikishor]

Don't we want to remove the i_terms completely?

[christophfroehlich]

Don't we want to remove the i_terms completely?

This is jazzy, we don't want to remove things there?

[ViktorCVS]

Don't we want to remove the i_terms completely?

At first, yes, but other implementations (e.g., MATLAB) use it, and a user find it useful. Therefore, it was decided to remove it as a strategy and set it as a parameter with a default value of infinity.

#436 (comment)

[saikishor]

@christophfroehlich Shall we also add the condition for the i_min to be negative? Or not needed?

[christophfroehlich]

technically, it would work (std::clamp is only undefined if i_min>i_max), but it makes no sense for the PID. I'd change that on rolling and backport to all distros then.

[saikishor]

Rest looks fine to me