[PID] Add support for saving i-term when PID is reset #180
Conversation
Added save_iterm_ to PID gains Added save_iterm() and clear_saved_iterm() methods to update and clear save integral term Cleared saved integral term in constructor Called save_iterm() in reset() method Added saved integral term to PID output
Simplified saving of integral term
Co-authored-by: Dr. Denis <[email protected]>
github-actions
bot
requested review from
arne48,
bmagyar,
DasRoteSkelett,
erickisos,
livanov93 and
olivier-stasse
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## ros2-master #180 +/- ##
===============================================
+ Coverage 74.88% 75.02% +0.13%
===============================================
Files 24 24
Lines 1107 1133 +26
Branches 86 89 +3
===============================================
+ Hits 829 850 +21
- Misses 231 236 +5
Partials 47 47
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This reverts commit 92f7d6f.
There was a problem hiding this comment.
The changes in the PR look fine to me, just some minor changes here
Apart from that, just a conceptual question. I'm not sure if the save_i_term should be the part of the Gains itself or it should be a parameter to the reset method alone. If everyone is happy with the original choice, I'm fine with it.
Co-authored-by: Sai Kishor Kothakota <[email protected]>
There was a problem hiding this comment.
I applied the changes requested by @saikishor and had a focus on not breaking ABI -> check is green -> we can merge that into jazzy imho.
src/pid.cpp
Outdated
| // If last integral error is already zero, just return | ||
| if (std::abs(i_error_) < std::numeric_limits<double>::epsilon()) | ||
| { | ||
| return; | ||
| } |
There was a problem hiding this comment.
maybe not? it does not take more effort to just zero it.
There was a problem hiding this comment.
in the earlier version we had to use the RT buffer here, that's why
Co-authored-by: Sai Kishor Kothakota <[email protected]>
christophfroehlich
dismissed stale reviews from saikishor and themself
via
410c61d
Saves integral term on reset to maintain steady state offset output.
This is required for controlling of system without integral part to achieve correct steady-state output.
If intergral term is reset for some time there will be incorrect output in steady-state. This features avoids such situations and makes btter performance.