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

Integrator Windup #34

Closed
paulbovbel opened this issue Mar 18, 2015 · 8 comments
Closed

Integrator Windup #34

paulbovbel opened this issue Mar 18, 2015 · 8 comments

Comments

@paulbovbel
Copy link

Even though i_term is clamped to i_max, i_error will continue to windup as long as there's an error. I've found this troublesome in an application with quadrotor control, where I'm trying to migrate the bespoke PID methods to the control_toolbox ones.

Would you be open to a pull adding a "antiwindup" parameter to enable clamping i_error by i_max/i_gain_?

@adolfo-rt
Copy link
Member

Are you thinking of this as an opt-in behavior, or as an always-in addition?.

@paulbovbel
Copy link
Author

Opt-in

@adolfo-rt
Copy link
Member

OK, if you can prepare a pull-request we can review it. So there would be a boolean parameter for toggling the anti-windup behavior?.

@paulbovbel
Copy link
Author

I think that would make sense, yes.

@mgruhler
Copy link
Contributor

Seeing that #52 has been merged, should this be closed?

Did you change the API of initPid on purpose? In the constructor you added the default argument for windup to false, thus keeping the old behaviour. Wouldn't it make sense to do this for the initPid calls the same way?

(Yeah, I know I'm about six weeks late ;-) )

@paulbovbel
Copy link
Author

Thanks @ipa-mig, it would have made perfect sense to modify initPid the same way.

@mgruhler
Copy link
Contributor

@paulbovbel will this be done for others not stumbling upon the same? (Even though I actually appreciate having a proper antiwindup and would have probably not found this change without the build error :-) )

@paulbovbel
Copy link
Author

I'm totally open to it, although @bmagyar would be the one to ask. Would you be able to put up a PR?

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

No branches or pull requests

3 participants