AC_AttitudeControl: ensure yaw gets a FLTD default #26263
Conversation
|
I have it on the tuning guide, but a default is even better |
| @@ -137,7 +137,7 @@ class AC_AttitudeControl_Multi : public AC_AttitudeControl { | |||
| .imax = AC_ATC_MULTI_RATE_YAW_IMAX, | |||
| .filt_T_hz = AC_ATC_MULTI_RATE_RP_FILT_HZ, | |||
| .filt_E_hz = AC_ATC_MULTI_RATE_YAW_FILT_HZ, | |||
| .filt_D_hz = 0.0f, | |||
| .filt_D_hz = AC_ATC_MULTI_RATE_RP_FILT_HZ, | |||
There was a problem hiding this comment.
Its a bit upsetting that its using RATE_RP (roll pitch) but filt T is the same....
In fact filt_E is useing AC_ATC_MULTI_RATE_YAW_FILT_HZ and filt_T_hz is using AC_ATC_MULTI_RATE_RP_FILT_HZ its all a bit odd....
There was a problem hiding this comment.
Yeh, I can see how this is a little confusing or unclear. What would you suggest?
The end result is we want roll, pitch and yaw filter T and D to be the same (20.0)
roll and pitch filter E to be 0.0
yaw filter E to be 2.0
There is an opportunity here to make this clear for the future.
There was a problem hiding this comment.
LOL, I originally did a bigger change that changed the names but figured this was better for 4.5 :-P
|
After the meeting I believe the only change needed for clarity is the name of the variable to include Y for yaw. |
peterbarker
force-pushed
the
pr-fltd-yaw-default
branch
from
a32cac3 to
aa2d638
Compare
|
I've cherry-picked Leonard's fixes for the naming into this PR. In addition to fixing the defines in AP_CustomControl he's fixed the same defines in Copter. I've tested that there's no compiler output change vs Andy's patch, so I believe that the one objection brought up at DevCall has been addressed. I'm marking this as MergeOnCIPass. |
When people come to tune yaw D they need to have a FLTD set. There is no downside to not setting this by default