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
Plane: correct extened range airspeed scailing limits #21632
Conversation
@@ -12,14 +12,14 @@ float Plane::calc_speed_scaler(void) | |||
if (aspeed > auto_state.highest_airspeed && hal.util->get_soft_armed()) { | |||
auto_state.highest_airspeed = aspeed; | |||
} | |||
// ensure we have scaling over the full configured airspeed | |||
const float scale_min = MIN(0.5, g.scaling_speed / (2.0 * aparm.airspeed_max)); | |||
const float scale_max = MAX(2.0, g.scaling_speed / (0.7 * aparm.airspeed_min)); |
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.
This would ensure we don't see low speed oscillations thanks to this fix.
const float scale_max = MAX(2.0, g.scaling_speed / (0.7 * aparm.airspeed_min)); | |
const float scale_max = 2.0; |
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.
i think we do need to honor the users airspeed min
@@ -12,14 +12,14 @@ float Plane::calc_speed_scaler(void) | |||
if (aspeed > auto_state.highest_airspeed && hal.util->get_soft_armed()) { | |||
auto_state.highest_airspeed = aspeed; | |||
} | |||
// ensure we have scaling over the full configured airspeed | |||
const float scale_min = MIN(0.5, g.scaling_speed / (2.0 * aparm.airspeed_max)); | |||
const float scale_max = MAX(2.0, g.scaling_speed / (0.7 * aparm.airspeed_min)); |
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.
i think we do need to honor the users airspeed min
@IamPete1 thanks for finding this! |
This fixes a error from #9680
The current code uses airspeed min for scaling min and airspeed max for scaling max. This is backwards. Currently changing max air speed makes no difference to the lower scaling limit used at high airspeed, instead it changes the upper limit used at low airseed.
In order to not change the limits too much I have changed the scale factors from 0.5 and 1.5 to 0.7 and 2.0. This matches up the two limits fairly well with default param, but might not for others.
I wonder if we should just hard code the upper limit to 2 at low airspeed. That is the one that will likely cause issues. Due to the gradient of the line at low airspeed small changes will make a big scaling difference.