-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
Remove TPA from rate control #13208
Remove TPA from rate control #13208
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine with me. I never had any vehicle where I had to use it - the thrust curve solves it cleanly.
Will you update the docs as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me, but some people will initially be unhappy. Can we prepare a brief line or two to include in the release notes and when this inevitably comes up in issues and discuss?
Sure
How about the explanation I posted in the description? Someone who can write better English than me can reword it 😇 |
Yes that's fine, it's more that I want to make sure it's highlighted at release time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Less parameters, less code, less problems.
Something is not happy:
|
On it. |
because it's mostly unused and we have thrust curve correction see parameter THR_MDL_FAC
Rebased and corrected the unit test. |
Need a docs update, FYI @hamishwillee |
Drafted doc change here: PX4/PX4-user_guide#592 |
Describe problem solved by this pull request
Throttle PID Attenuation (TPA) is a concept used by racing autopilots to get rid of oscillations that occur with different thrust levels than hover thrust. These oscillations are usually because ESCs do not control thrust but rather voltage duty cycle (or in some cases RPM) of the motor and the effective thrust the propeller generates is not just linearly dependent on the actuator output command of the autopilot. TPA is in my eyes a hacky way of working around this problem which is certainly better than not compensating at all but worse than correcting the actual thrust curve because it's hard to tune. And it's overhead like @dagar wrote here: #13173 (comment)
Describe your solution
I suggest to remove TPA from the rate controller since the thrust curve compensation https://docs.px4.io/v1.9.0/en/config_mc/pid_tuning_guide_multicopter.html#thrust_curve is the prefered method of compensation.
This is a proposal and I don't want to step on anyones toes, that's also why I didn't suggest this in #13173 but suggest it now separately.
Describe possible alternatives
A clear and concise description of alternative solutions or features you've considered.
Test data / coverage
SITL gazebo tested.
Additional context
Original feature pr: #5861
Fix for when it was broken for a while and no one noticed: #9166