Add three new anti-windup techniques and a Saturation feature #298

ViktorCVS · 2025-03-11T22:54:46Z

Overview

This PR adds three new anti-windup techniques: back‑calculation, the conditioning technique, and conditional integration. It also adds a saturation feature for the PID output. New parameters have been introduced, and additional overloads have been implemented to ensure compatibility.

What was added/changed in this PR

Added three new anti-windup techniques
- back-calculation
- conditioning technique
- conditional integration
Added saturation feature to PID output

About compatibility

The packages compile correctly and have passed the pre‑commit and colcon tests (packages with dependencies continue to show the same number of failures before and after my modifications). If the new parameters are not used, the package retains its old behavior.

About the older anti-windup technique

My plan, either by the end of this PR or in a subsequent one, is to completely remove the older anti‑windup technique that has been used so far. This method, which is a form of conditional integration, has several disadvantages:

If set incorrectly, it may cause a steady‑state error.
If set incorrectly, it may not affect the system at all.
Even if it is set between the steady‑state error limit and the value beyond which it has no effect, it is still difficult to find a configuration that improves the system as effectively as the other techniques.

Additionally, regardless of whether the 'antiwindup' parameter is set to true or false, the anti-windup technique is applied (using the same method with a different approach), so the user does not have the option to disable it.

About unit tests

I've added 10 new unit tests for the new features and updated the existing ones to accommodate the new parameters.

Related PR's

Important notes

These three techniques are common anti‑windup strategies used to mitigate the windup effect and are widely employed in control applications: back‑calculation [1], the conditioning technique [1,2], and conditional integration [1,3].

The default values for the tracking time constant are defined in [3,4] for back‑calculation and in [1] for the conditioning technique.

Both back‑calculation and the conditioning technique use forward Euler discretization; this may change before merging this PR.

Graphs

I tested it on ros2_control_demos to better illustrate this feature and test it on simulation to valide the equations. The tests were conducted using a modified version of Example 1: RRBot, which uses a PID controller instead of the default forward position controller. It was tested on Docker, Ubuntu Noble, and Jazzy.

PID values: p = 4.0, i = 25.0, d = 0.5; u_max = 13, u_min = -13; and the tracking time constant was left at its default value.

The standard response with a settling time (ts) of 5.2 seconds, the response affected by saturation, resulting in a settling time (ts_sat) of 8.6 seconds (+65.4% increase) and the response using the back-calculation technique, which improves performance with a settling time (ts_back) of 4.1 seconds (–21.2% decrease), even lower than the standard response.

Those figures compares three anti-windup methods applied to the step response, a zoomed-in view of the step response is provided here to clearly distinguish between the three anti-windup strategies. They are all very similar due to the system and PID values, but they may vary significantly between applications.

The standard control output, the control output affected by saturation, with a recovery time from saturation of 6.8s and the control output using the back-calculation technique, with a recovery time from saturation of 2s (-70.6%).

Those figures compares three control outputs using anti-windup methods, a zoomed-in view of the control output is provided here to clearly distinguish between the three anti-windup strategies. They are all very similar due to the system and PID values, but they may vary significantly between applications.

All the equations have been validated with these simulations, providing a feature with three techniques to address windup.

Final notes

I'm very open to any recommendations to improve this code.

References

[1] VISIOLI, A. Pratical PID Control. London: Springer-Verlag London Limited, 2006. 476 p.
[2] VRANCIC, D et al. Antiwindup, Bumpless, and Conditioned Transfer Techniques for PID Controllers. IEEE Control Systems Magazine, 1996. 16(4):48 - 57.
[3] BOHN, C.; ATHERTON, D. An analysis package comparing pid anti-windup strategies.
IEEE Control Systems Magazine, p. 34–40, 1995.
[4] ASTRöM, K.; HäGGLUND, T. PID Controllers: Theory, Design and Tuning. ISA Press.
Research Triangle Park, USA: Springer-Verlag London Limited, 1995. 343 p.

mergify · 2025-03-12T10:55:24Z

This pull request is in conflict. Could you fix it @ViktorCVS?

ViktorCVS · 2025-03-13T18:42:06Z

@christophfroehlich, it appears that no reviewers have been assigned to this PR. Could you please help with that? If you have time, I'd appreciate it if you could also take a look at the changes.

christophfroehlich

Thx for this thorough PR, but it will need some time to properly review it

src/pid.cpp

control_toolbox.rolling.repos

codecov-commenter · 2025-03-14T22:18:13Z

Codecov Report

Attention: Patch coverage is 90.48991% with 33 lines in your changes missing coverage. Please review.

Project coverage is 80.24%. Comparing base (516eccd) to head (7cc756b).

Files with missing lines	Patch %	Lines
control_toolbox/src/pid_ros.cpp	76.71%	12 Missing and 5 partials ⚠️
control_toolbox/src/pid.cpp	82.35%	7 Missing and 2 partials ⚠️
control_toolbox/include/control_toolbox/pid.hpp	87.50%	6 Missing and 1 partial ⚠️

Additional details and impacted files

@@               Coverage Diff               @@
##           ros2-master     #298      +/-   ##
===============================================
+ Coverage        78.17%   80.24%   +2.06%     
===============================================
  Files               30       30              
  Lines             1338     1655     +317     
  Branches            87       98      +11     
===============================================
+ Hits              1046     1328     +282     
- Misses             245      273      +28     
- Partials            47       54       +7

Flag	Coverage Δ
unittests	`80.24% <90.48%> (+2.06%)`	⬆️

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

Files with missing lines	Coverage Δ
...ontrol_toolbox/include/control_toolbox/pid_ros.hpp	`100.00% <ø> (ø)`
control_toolbox/test/pid_ros_parameters_tests.cpp	`100.00% <100.00%> (ø)`
control_toolbox/test/pid_ros_publisher_tests.cpp	`95.00% <100.00%> (ø)`
control_toolbox/test/pid_tests.cpp	`100.00% <100.00%> (ø)`
control_toolbox/include/control_toolbox/pid.hpp	`85.71% <87.50%> (+7.93%)`	⬆️
control_toolbox/src/pid.cpp	`86.61% <82.35%> (-4.40%)`	⬇️
control_toolbox/src/pid_ros.cpp	`73.17% <76.71%> (-0.72%)`	⬇️

🚀 New features to boost your workflow:

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

bmagyar · 2025-04-24T10:57:20Z

@ViktorCVS could you please resolve the current conflicts?

ViktorCVS · 2025-04-24T17:54:59Z

@ViktorCVS could you please resolve the current conflicts?

done

christophfroehlich

Thanks a lot for the nice work including tests etc. Please fix the pre-commit errors, and only some minor comments in the code

control_toolbox/include/control_toolbox/pid_ros.hpp

control_toolbox/src/pid_ros.cpp

control_toolbox/src/pid.cpp

control_toolbox/test/pid_tests.cpp

christophfroehlich · 2025-04-26T12:12:31Z

My plan, either by the end of this PR or in a subsequent one, is to completely remove the older anti‑windup technique that has been used so far. This method, which is a form of conditional integration, has several disadvantages:

Please add a deprecation notice to the code, as well as a warning on std::cout if "none" is configured, for example "xxx is deprecated. This option will be removed by the ROS 2 Kilted Kaiju release."

control_toolbox/include/control_toolbox/pid.hpp

control_toolbox/src/pid.cpp

control_toolbox/include/control_toolbox/pid_ros.hpp

christophfroehlich

For the future: Please don't force push to PRs because it makes it harder for reviewers to check the changes since the last review ;) The history does not have to be linear, because we squash that anyways.

We get closer to the finish line, but some of our comments haven't been addressed yet.

control_toolbox/include/control_toolbox/pid.hpp

control_toolbox/src/pid_ros.cpp

control_toolbox/src/pid.cpp

control_toolbox/include/control_toolbox/pid.hpp

control_toolbox/include/control_toolbox/pid_ros.hpp

christophfroehlich

The tests are failing now, can you have a look please?

control_toolbox/include/control_toolbox/pid.hpp

Refactor antiwindup strategy variable implementation replacing string usage with a dedicated enum improve type safety.

Use iszero(x) to compare cmd_ and cmd_unsat_

ViktorCVS · 2025-05-30T02:07:29Z

I merged them already in, now the diff view looks fixed again.

Can you take a look again, please? My Git didn't work without push with force, so I push the next commit with /f.

ViktorCVS · 2025-05-30T02:37:31Z

The API and structure looks good now, thanks! I cross-checked the equations with the cited literature and have some questions, see my comments.

I recently added documentation integrated in control.ros.org. Could you please summarize your changes in

the release notes. You can use the style from the other repos.

and add necessary migration steps here. What will be the default after the deprecation cleanup? conditional integration?

Not necessarily in this PR, but could you please summarize the PID class in structured text format and add it here?

I have a question about the migration steps: this PR was designed so that it doesn’t break existing implementations, but the next PR will actually remove the deprecated technique. Should I list the migration steps now?

christophfroehlich · 2025-05-30T08:04:21Z

Can you take a look again, please? My Git didn't work without push with force, so I push the next commit with /f.

Why? Have you pulled the changes (after my merge via Github UI?). It maybe won't let you pull if you have uncommited changes, either commit them and do a git pull --rebase or stash them before the pull.

I have a question about the migration steps: this PR was designed so that it doesn’t break existing implementations, but the next PR will actually remove the deprecated technique. Should I list the migration steps now?

Let's add the migration notes before we remove it. We will release the changes, wait for the next sync, and then we can remove it. So users have one release cycle to see the deprecation warnings before any behavior changes are applied.

doc/release_notes.rst

christophfroehlich

Thanks again!

Could you please submit two follow-up PRs:

One which updates the documentation, basically the summary of your initial message here. But you don't need to document the old behavior, just the result after the deprecation cleanup. Here you can add the migration notes.
Another one cleaning up the deprecated code. You can draft it for now, and we will merge after the next sync to kilted/rolling.

christophfroehlich

Oh, I see that the downstream PidController tests fail now. Can you have a look please and update ros-controls/ros2_controllers#1585?

ViktorCVS · 2025-05-30T13:23:47Z

Oh, I see that the downstream PidController tests fail now. Can you have a look please and update ros-controls/ros2_controllers#1585?

Fixed

christophfroehlich · 2025-05-30T15:37:26Z

Fixed

Thanks. I see now that antiwindup parameter is not used any more if a strategy other than NONE is configured. Shouldn't we add this flag there also?

saikishor

@ViktorCVS Very good work

I've only one question, what will the bool antiwindup parameter be used for from now on?

saikishor · 2025-05-30T15:43:59Z

control_toolbox/src/pid_ros.cpp

+    if (antiwindup_strat == AntiwindupStrategy::NONE)
+    {
+      std::cout << "Old anti-windup technique is deprecated. "
+                   "This option will be removed by the ROS 2 Kilted Kaiju release."


Suggested change

"This option will be removed by the ROS 2 Kilted Kaiju release."

"This option will be removed after the ROS 2 Kilted Kaiju releases."

Should it be after? @christophfroehlich

I'd like to remove things for kilted until we say we have "released" it. But not sure if we should backport this PR to jazzy for users having a chance to update it before they update the distro and ending in a broken setup.

I would opt for backporting it as it comes with very nice features + easy transition

it does not break API but ABI, and that's what I try to avoid :/

ViktorCVS · 2025-05-30T16:07:30Z

@ViktorCVS Very good work

I've only one question, what will the bool antiwindup parameter be used for from now on?

Thanks! Until I remove it, it will still maintain the old behavior. This PR just adds more options. I’ll open a follow-up PR today to remove it.

ViktorCVS · 2025-05-30T16:20:23Z

Fixed

Thanks. I see now that antiwindup parameter is not used any more if a strategy other than NONE is configured. Shouldn't we add this flag there also?

In ros2_controllers? I agree. I’ll update the anti-windup parameter description accordingly.

ViktorCVS · 2025-05-30T16:21:34Z

Thanks again!

Could you please submit two follow-up PRs:

One which updates the documentation, basically the summary of your initial message here. But you don't need to document the old behavior, just the result after the deprecation cleanup. Here you can add the migration notes.

Another one cleaning up the deprecated code. You can draft it for now, and we will merge after the next sync to kilted/rolling.

Should I wait for the merge?

christophfroehlich · 2025-05-30T16:43:02Z

control_toolbox/src/pid.cpp

+  if (gains.antiwindup_strat_ == AntiwindupStrategy::BACK_CALCULATION && !is_zero(gains.i_gain_))
+  {
+    i_term_ += dt_s * (gains.i_gain_ * error + 1 / gains.trk_tc_ * (cmd_ - cmd_unsat_));
+  }
+  else if (gains.antiwindup_strat_ == AntiwindupStrategy::CONDITIONAL_INTEGRATION)
+  {
+    if (!(!iszero(cmd_unsat_ - cmd_) && error * cmd_unsat_ > 0))
+    {
+      i_term_ += dt_s * gains.i_gain_ * error;
+    }
+  }


I mean using gains.antiwindup_ here? How else can it be deactivated? Or is the plan to leave the NONE for this case?

Yes! if the user doesn’t want any anti-windup effect on the PID, they should set the value to “none"

Then wouldn't it make sense to remove the Boolean parameter completely? Atleast from the new constructors, this way we won't break again in future. For backward compatibility, if the parameter is set, we can automatically override the internal anti windup type to None.

How does it sound?

Removing the anti-windup parameter from the constructor that accepts multiple arguments would render it inaccessible to users. I suggest we preserve the existing functionality for this PR. What do you think?

I would not use this parameter in the future, if there is the opportunity to use just the NONE instead. The only question is, how to gracefully transition/deprecate it? Can we add deprecated qualifier to constructor overloads, and just add the same ones without the antiwindup?

Sorry for the delay. I agree; this will make it clearer what will be removed and help clean things up.

What about moving this AntiwindupStrategy as a non-optional arg of the methods, this should give us some flexibility. I think it makes sense to be explicitly set.

the other overloads are here just for backward compatibility, whenever a new parameter was added. do you propose to deprecate+remove them all?

@christophfroehlich I’ve been reviewing the code, and it seems that if we drop the bool antiwindup parameter from the full overload, we’ll lose any way to pass that flag while it still exists. The wrapper that takes only the bool currently forwards its value to the full overload. If the full overload no longer accepts that argument, there’s nowhere for it to go. Am I overlooking something?

why not adding the future full-overload, and deprecating the current full-overload with the bool? You can use gcc pragmas to avoid deprecation warnings within our own code:

#pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdeprecated-declarations" <deprecated code..> #pragma GCC diagnostic pop

This was referenced Mar 11, 2025

Add new members for PID controller parameters ros-controls/ros2_controllers#1585

Draft

Remove members to pid state message ros-controls/control_msgs#178

Merged

ViktorCVS changed the title ~~Add three new anti-windup techniques~~ Add three new anti-windup techniques and a Saturation feature Mar 12, 2025

ViktorCVS mentioned this pull request Mar 12, 2025

Understanding the Anti-Windup Action #276

Open

ViktorCVS force-pushed the ros2-master branch 2 times, most recently from d0feb10 to bd7c8f0 Compare March 13, 2025 16:56

ViktorCVS marked this pull request as ready for review March 13, 2025 18:06

ViktorCVS force-pushed the ros2-master branch from 812dab4 to 8931acc Compare March 14, 2025 10:25

christophfroehlich reviewed Mar 14, 2025

View reviewed changes

src/pid.cpp Outdated Show resolved Hide resolved

control_toolbox.rolling.repos Outdated Show resolved Hide resolved

ViktorCVS force-pushed the ros2-master branch from 5b076ea to 2e9e320 Compare March 17, 2025 11:14

ViktorCVS requested a review from christophfroehlich March 20, 2025 12:30

ViktorCVS force-pushed the ros2-master branch from c359b23 to 9021434 Compare March 31, 2025 13:27

ViktorCVS force-pushed the ros2-master branch from 9021434 to 1a3a441 Compare April 24, 2025 17:52

ViktorCVS force-pushed the ros2-master branch from 1a3a441 to fc07bd3 Compare April 25, 2025 18:22

christophfroehlich requested changes Apr 26, 2025

View reviewed changes

christophfroehlich reviewed Apr 26, 2025

View reviewed changes

control_toolbox/include/control_toolbox/pid.hpp Outdated Show resolved Hide resolved

saikishor reviewed Apr 26, 2025

View reviewed changes

ViktorCVS force-pushed the ros2-master branch from fc07bd3 to 71baea5 Compare April 28, 2025 10:13

christophfroehlich linked an issue Apr 28, 2025 that may be closed by this pull request

Understanding the Anti-Windup Action #276

Open

christophfroehlich requested changes Apr 28, 2025

View reviewed changes

ViktorCVS force-pushed the ros2-master branch from 51b3d0e to 5513ff0 Compare April 29, 2025 12:45

christophfroehlich requested changes Apr 30, 2025

View reviewed changes

control_toolbox/include/control_toolbox/pid.hpp Outdated Show resolved Hide resolved

control_toolbox/include/control_toolbox/pid.hpp Outdated Show resolved Hide resolved

ViktorCVS force-pushed the ros2-master branch 2 times, most recently from 5513ff0 to 55f3505 Compare May 5, 2025 10:49

ViktorCVS added 11 commits May 30, 2025 01:24

remove is_not_zero() function

d603abc

Refactor PID antiwindup strategy variable

ad16f36

Refactor antiwindup strategy variable implementation replacing string usage with a dedicated enum improve type safety.

Add deprecated message for the old anti-windup

c5096d5

Add deprecated message for the old anti-windup

60f35fb

Calculate tracking time constant in set_gains

560276e

Discard unused comments and fix comment formatting

98f7ad7

Add comment for gains_buffer_ in pid.hpp

dd9be8c

Add comment for gains_buffer_ in pid.hpp

7d7311a

Add comment for gains_buffer_ in pid.hpp

47b8639

Remove CONDITIONING_TECHNIQUE from all files

9610d41

Remove CONDITIONING_TECHNIQUE from all files

5b3b630

ViktorCVS force-pushed the ros2-master branch from 9c04d13 to 5b3b630 Compare May 30, 2025 01:47

Use iszero(x) to compare variables

a61cf71

Use iszero(x) to compare cmd_ and cmd_unsat_

Add release note for this pull request

46f0e46

Add release note for this pull request

1c15588

christophfroehlich reviewed May 30, 2025

View reviewed changes

doc/release_notes.rst Outdated Show resolved Hide resolved

Update doc/release_notes.rst

7cc756b

christophfroehlich approved these changes May 30, 2025

View reviewed changes

christophfroehlich requested changes May 30, 2025

View reviewed changes

saikishor approved these changes May 30, 2025

View reviewed changes

christophfroehlich reviewed May 30, 2025

View reviewed changes

Merge branch 'ros2-master' into ros2-master

4706ede

	"This option will be removed by the ROS 2 Kilted Kaiju release."
	"This option will be removed after the ROS 2 Kilted Kaiju releases."

Add three new anti-windup techniques and a Saturation feature #298

Are you sure you want to change the base?

Add three new anti-windup techniques and a Saturation feature #298

Uh oh!

Conversation

ViktorCVS commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

What was added/changed in this PR

About compatibility

About the older anti-windup technique

About unit tests

Related PR's

Important notes

Graphs

Final notes

References

Uh oh!

mergify bot commented Mar 12, 2025

Uh oh!

ViktorCVS commented Mar 13, 2025

Uh oh!

christophfroehlich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bmagyar commented Apr 24, 2025

Uh oh!

ViktorCVS commented Apr 24, 2025

Uh oh!

christophfroehlich left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

christophfroehlich commented Apr 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

christophfroehlich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

christophfroehlich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ViktorCVS commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ViktorCVS commented May 30, 2025

Uh oh!

christophfroehlich commented May 30, 2025

Uh oh!

Uh oh!

christophfroehlich left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

ViktorCVS commented Mar 11, 2025 •

edited

Loading

codecov-commenter commented Mar 14, 2025 •

edited

Loading

christophfroehlich left a comment •

edited

Loading

christophfroehlich commented Apr 26, 2025 •

edited

Loading

ViktorCVS commented May 30, 2025 •

edited

Loading

christophfroehlich left a comment •

edited

Loading

christophfroehlich commented May 30, 2025 •

edited

Loading

saikishor May 30, 2025 •

edited

Loading