prevent opposite i-term accumulation in nav pid

[shota3527]

Closes #9026
replace #9159

New method here #9224
Prevent opposite iterm accumulation in nav pid controller when nav pid controller is saturated a lot. but this will deteriorate anti-windup performance. Still looking for a smarter may

From here we can see the real problem is opposite i-term accumulation, It should be positive all the time. But it is negative in this video
#9026 (comment)

TEST in SITL, waypoint altitude setting is 1000m
https://youtu.be/mPif0hbwtpg

[breadoven]

This appears to work better than the current situation but really the PID controller needs resetting when the target conditions change significantly, i.e. when a new position target is set, otherwise it's using PID values that are unrelated to the new target.

I assume this method works OK if you do a 1000m descent WP mission ?

[shota3527]

This appears to work better than the current situation but really the PID controller needs resetting when the target conditions change significantly, i.e. when a new position target is set, otherwise it's using PID values that are unrelated to the new target.

I assume this method works OK if you do a 1000m descent WP mission ?

I don't think a pid reset or iterm reset is needed when the target wp position changes significantly. The plane, The required pitch offset, The wind, The battery voltage will not change on a wp position change. In a cilmb_50m->climb_500m->climb_5000m situation, resetting the iterm will have a negative effect.
A wp position change is just a setpoint change, very similar to deflect the stick on the RC controller. For expamle, From full roll left to full roll right, we don't reset the roll pid controller.
If the windup is a problem in climb->desent->climb situation, The solution is to make a more robust pid controller, not sacrificing climb->climb->climb response and benefit other potential situations

[breadoven]

I don't think you can compare Nav PID behaviour to the usual flight control PID behaviour because of the way some Nav targets are set. The issue with WP altitude control is caused by the fact the altitude target is set on an incremental rolling basis to create a linear climb between waypoints. The problem with this is the error starts small at a new WP because the initial incremental altitude target is small and then grows if the craft can't change altitude as fast as increasing target altitude demands. The I term accumulates, even with the change in this PR (although now at least it's heading in the right direction), so that at the next WP the I term is initially out of sync with the new small incremental altitude target error ... potentially causing problems. This is an oddity of the way the Nav altitude target is set when flying between WPs.

I suspect with this change that there will be a potential issue if you have a large descent demand that can't be met followed by a climb. I'm guessing the craft will momentarily continue to descend below the target altitude until the I term unwinds and the error cancels it out at which point it will start to climb. I suppose it comes down to how accurately and smoothly you want to track the set targets.

However, this seems to improve things so maybe nothing more is needed. Needs to be tested for multirotor velocity control though.

[shota3527]

I think i have found a better solution in #9224, I will keep this pr open for a while for discussion

[Jetrell]

@shota3527 This method didn't play well with the MC navigation PID's.
I tested it by running a WP mission with varying altitudes and turns between each WP, in short succession.
The result of preventing opposite i-term accumulation. Was a multicopter that would bank outwards and backwards when attempting to take the WP turn and adjust altitude.
It was a wild ride.. LOL

I tested that same mission with the current method. And it performed the same as normal.

[shota3527]

@Jetrell
Thank you for testing, this method has a side effect that will suppress the original anti-windup back calculation.
I have tried a new method in #9224, The original anti-windup back calculation will work as normal until the opposite i-term is accumulated. It seems working well in the simulator

[shota3527]

replaced by #9224