@tbolin : Ideally, motor output would be propagated back to PIDs (inverse of mixer mapping). Then, it would be quite easy to handle I-term correctly. It was considered long time agon and IIRC deemed too complicated at that time.

@tbolin : Ideally, motor output would be propagated back to PIDs (inverse of mixer mapping). Then, it would be quite easy to handle I-term correctly. It was considered long time agon and IIRC deemed too complicated at that time.

I have already written a simple fix that appears to work pretty well. The result is in the fixed_limit_400.bbl file. Just wanted to make sure this is actually considered an issue (and add some unit tests) before I make a PR.

Thanks for finding this, your advice on how to fix it will be very much appreciated.
I've typically recommended using iterm_windup to control excessive iTerm in yaw.
I never really knew which of the parameters would be 'constrained' by a pidSum limiter.
Looks like only the 'sum' itself is constrained.... leaving iTerm to accumulate 'in the background', with no limit, effectively.
iterm_windup stops the iTerm accumulation itself in this situation. Perhaps we need something like iterm_windup in this situation, to limit the iTerm part, not the Term part.

I already made a PR with a suggested fix.
The fix applies exactly the same mechanism on the i term change as the mixer range method, but uses the PID-sum on that axis relative to the pid_sum_limit. Then the lowest i-term change from the two methods is used to determine the rate of change.

That way the result is identical if the pid_sum_limit of that axis is 500 or more (assuming the normal quad mixer).

The fixed_limit_400.bbl file in the zip I uploaded have this fix applied. The i-term behaves much more like with pid_sum_limit_yaw set to 500 in this log. The i-term does not wind up nearly as much and the overshoots are smaller than in the dev_limit_400.bbl log.

As you know, I've spent a lot of time thinking about this.
After some considerable effort, I haven't really got anywhere.
However some things I have formed some kind of conceptual basics. To summarise:

• the only current need for iTermWindup is on yaw
• it's needed on yaw because we don't apply iTermRelax to yaw. Apart from iterm_windup, yaw has no means to constrain iTerm growth on fast stick inputs.
• iTermWindup is probably not helpful on roll on pitch; it is a global factor that inhibits the iTerm on all axes just because one axis has saturated the mixer. That's not ideal. For instance, a roll flip that saturates motors will activate iTermWindup and affect iTerm on pitch, just when we want iTerm on pitch to stabilise the cross-axis wobble that we sometimes get in this situation.
• Hence, maybe what we really is a method for iTermRelax on Yaw, one that works across the full allowed range of values for iTermLimit and pidSumLimitYaw.

The old dynCi method and this method are both 'input gain limiting' methods of constraining iTerm growth. As pointed out by Petr Ledvina, they carry the possibility that if iTerm became the dominant contributor to pidSum, then iTerm could become large, reduce it's input gain factor via pidSum, and then be very slow to go away. In fact I think this was a problem with the dynCi method. It was fortunate that iTermLimit was 400, since that meant one axis could only command 80% of the motorMix, but if two axes went high in iTerm, they could saturate motorMix, and then iTerm could not fall. This possibility is high if iTermLimit is set to a value above pidSumLimit, in particular. And various configuration possibilities exist with our current array of iTerm and pidSum limits - one of which is identified by this PR.

Hence I am now tending to think that input limiting methods for iTerm windup are probably best avoided. And that means, rather than using motorMix / dynCi / iTermWindup methods being applied to all axes, we should focus primarily on an explicit iTermRelax option for yaw.

The main reason why currently there is no iTermRelax on Yaw is that in most 5in and above quads, yaw iTerm must grow, right from the start, to avoid excessive delay in achieving the target value. Particularly for larger quads, there was a need to accumulate a lot of iTerm on yaw. 'Relaxing' iTerm on yaw on these machines can result in long delay reaching the set yaw rate. The price we pay is some overshoot like shown above, which most people don't notice or care about very much.

I think it might be best to explore removing the dynCi method, certainly as a 'global' anti-windup method, and evaluate how best we can solve issues that are specific to yaw, perhaps via the existing iTermRelax code. To me that makes sense, since this specific issue relates only to iTerm related overshoots on commanded yaw inputs, which is exactly what iTerm Relax is about.