-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Apply iterm_windup per-axis by limiting iTerm based on pidSum #13543
Open
ctzsnooze
wants to merge
2
commits into
betaflight:master
Choose a base branch
from
ctzsnooze:remove-iterm_windup-and-limit-yaw-iterm-to-pidsum_limit_yaw
base: master
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Apply iterm_windup per-axis by limiting iTerm based on pidSum #13543
ctzsnooze
wants to merge
2
commits into
betaflight:master
from
ctzsnooze:remove-iterm_windup-and-limit-yaw-iterm-to-pidsum_limit_yaw
+150
−68
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Do you want to test this code? You can flash it directly from Betaflight Configurator:
WARNING: It may be unstable. Use only for testing! |
ctzsnooze
force-pushed
the
remove-iterm_windup-and-limit-yaw-iterm-to-pidsum_limit_yaw
branch
4 times, most recently
from
April 18, 2024 08:09
2ea4754
to
698673c
Compare
ctzsnooze
force-pushed
the
remove-iterm_windup-and-limit-yaw-iterm-to-pidsum_limit_yaw
branch
from
April 20, 2024 10:31
032f3ce
to
6fb5577
Compare
ctzsnooze
force-pushed
the
remove-iterm_windup-and-limit-yaw-iterm-to-pidsum_limit_yaw
branch
from
April 23, 2024 10:20
6fb5577
to
59db260
Compare
ctzsnooze
changed the title
Remove motorMix-based iTerm gain attenuation, limit iTerm on Yaw to pidsum_limit_yaw
Apply iterm_windup per-axis by limiting iTerm based on pidSum
Apr 23, 2024
ctzsnooze
force-pushed
the
remove-iterm_windup-and-limit-yaw-iterm-to-pidsum_limit_yaw
branch
2 times, most recently
from
April 23, 2024 11:14
d72dea0
to
e68af9c
Compare
This was referenced Apr 23, 2024
ctzsnooze
force-pushed
the
remove-iterm_windup-and-limit-yaw-iterm-to-pidsum_limit_yaw
branch
from
April 24, 2024 02:39
4f6e8ce
to
94eb4dc
Compare
ctzsnooze
requested review from
ledvinap,
KarateBrot and
McGiverGim
and removed request for
KarateBrot
April 25, 2024 01:53
ctzsnooze
force-pushed
the
remove-iterm_windup-and-limit-yaw-iterm-to-pidsum_limit_yaw
branch
from
May 5, 2024 00:02
94eb4dc
to
bc6720d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR 're-purposes' iTerm windup, setting per-axis limits on iTerm based on pidSum for the axis, rather than limiting iTerm on all axes at once when motorMix is high. The old
iterm_limit
value is removed from the CLI, as is the olditerm_windup
code that was based on motorMix.It is a simpler PR that can probably be considered a bug fix for 4.5.
Probably won't make much difference in flight for most quads, but may be a bit more stable after minor impacts and should accumulate a little bit less iTerm on yaw on low authority builds. iTerm windup will have a stronger effect than before when constraining iTerm.
It replaces #13506, which I've closed. It is a companion PR to: PR #13576, which improves the feedforward element for yaw, over the top of this PR
This new feedforward method for yaw solves many of the issues related to iTerm accumulation on yaw by driving the yaw almost entirely from feedforward.
Instead of using MotorMix to constrain iTerm growth, on all axes, when motors approach saturation, this PR calculates an iTermLimit value for each axis based on the product of the user's
iterm_windup
andpidSumLimit
/pidSumLimitYaw
values.In contrast, previously, if any one axis caused MotorMix to reach 100%, iTerm growth on all axes was prevented. The argument in favour of this was that iTerm might grow inappropriately on the other axes because control could not be normal with motors maxing out. However iTerm is the primary factor in the return to the previous heading after an uncommanded disturbance. It is more likely to return to the correct heading, on a given axis if iTerm can accumulate and de-accumulate normally. For example, a roll on a quad with a centre of gravity issue, fast enough to cause motors to max out on roll, is often associated with a transient pitch error. The quad would be more likely to return to the previous pitch attitude, at the end of the roll, if iTerm on pitch accumulated normally despite the motors being maxed out due to Roll. This PR will keep pitch iTerm accumulation independent of roll, and do the same for yaw, so that at the end of a wonky roll, the quad should return to the previous heading and pitch angle.
iTerm limiting in this PR happens when pidSum gets close to a value which, for that axis, would max out the motors. How close it gets can be adjusted using the
iterm_windup
parameter.For example, with a default value for
pidSumLimit
of 500, and the new defaultiterm_windup
value of 80, the limit on iTerm for Roll and Pitch will be 400. This is the same as the olditerm_limit
value of 400. This will not allow iTerm to take more than 80% of the available pidSum space for a given axis.A lower than default
iterm_windup
would, in this PR, limit iTerm earlier in situations where the quad was unable to achieve a requested target yaw rate.Yaw has a default
pidsum_limit_yaw
value of 400, and hence yaw iTerm will be limited to 80% of 400, or 320. Previously the yaw iTerm limit would have been the same 400 value as per pitch and roll, so this is a little bit less than previously.In the unlikely event that more iTerm on yaw is required, the user can increase the overall
pidsum_limit_yaw
value. However, high iTerm on yaw, like other axes, tends to lead to overshoot and bounce-back. The new feedforward proposed for yaw greatly diminishes the need for high Term on yaw, and fits well with these changes.Note that previously it was possible for iTerm to grow on yaw to 400, even though the Pid Sum on yaw was limited to 400. This reduced the ability of yaw P to adjust yaw until iTerm fell considerably below 400. Now there is room for some P activity even when iTerm is at its limit, on yaw as well as on the other axes.