Skip to content

Added Functionality to manually set the integral term. #22

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

padalev
Copy link
Contributor

@padalev padalev commented Oct 26, 2023

This might be important to set controller to previous state after an interruption or crash. It also allows for more manual manipulation of the PID controller if that is desired.
I have added a function set_integral_term() which is very similar to reset_integral_term() but takes the desired value as a parameter.
I have left the original reset_integral_term() in the code even though this kinda doubles functionality to not break behaviour that might be expected by people that have used the reset_integral_term() before.

…important to set controller to previous state after an interruption or crash.
@SwenBischof
Copy link

Hi @braincore this pull request would actually be very interesting for battery powered devices. So that the integral term can be restored after for example a deep sleep.

@braincore
Copy link
Owner

@SwenBischof That makes plenty of sense.

@padalev Sorry for the delay. Can we drop the dt changes and instead only add set_integral_term()? @SwenBischof Feel free to submit another PR if you can't wait.

@padalev
Copy link
Contributor Author

padalev commented Mar 22, 2024

Sure. I'm not insisting on anything here. I still believe it would make sense to properly implement dt as this is really a key part of a proper PID implementation but that's obviously not the point of this PR. So let's just drop that for now and just do the integral term part right now.

@SwenBischof
Copy link

@padalev Sorry for the delay. Can we drop the dt changes and instead only add set_integral_term()? @SwenBischof Feel free to submit another PR if you can't wait.

I am mainly interested in the manual setting of the integral term. I think for that reason it doesn't make sense to create another pull request..

@mtilda
Copy link
Collaborator

mtilda commented Mar 26, 2025

Since I am not able to push to this branch, I created a new branch and PR (#27) with 3ae1fe5. Happy to close my PR as a duplicate if this one is updated.

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