Skip to content
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

Filter: SlewCalculator2D typo fix #29275

Merged
merged 1 commit into from
Feb 12, 2025
Merged

Conversation

rmackay9
Copy link
Contributor

@rmackay9 rmackay9 commented Feb 11, 2025

This fixes a typo in the 2D Slew Calculator used by the AC_PID_2D class which is used in Copter, Rover and Blimp's XY velocity controller. This means it's also used by QuadPlanes in QLoiter, etc but surely it has nothing to do with the recent questions around quad plane attitude control because the bug was in 4.5 as well.

This was added in PR #23981 but it is only used for reporting so I think this does not cause any actual performance issue. In fact, since this PR went in we've simplified the Rover autotune and we don't use this is anymore so we could potentially backout the slew reporting completely and save some flash and a very small amount of CPU

This resolves issue #29272

I've done some minor testing in copter in SITL and there is a difference in the logged slew rate. For example in the "before" we see consistently low slew rate while in "after" it is much much larger.
srate-before-vs-after

I think this is correct though because if I greatly reduce the PSC_VELXY_P and I values I can produce similarly huge numbers with master
image

@MattKear
Copy link
Contributor

Does anyone actually use the slew rate limiter in the position controller?
My vote would be to remove it.

@rmackay9 rmackay9 merged commit 4d424a8 into ArduPilot:master Feb 12, 2025
100 checks passed
@rmackay9
Copy link
Contributor Author

I've merged this but I'll likely raise another PR to remove the feature completely

@rmackay9 rmackay9 deleted the slew-calc-2d-fix branch February 12, 2025 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants