Dangerous change in behaviour of TECS_PITCH_MIN
Bug report
Issue details
Prior to commit https://github.com/ArduPilot/ardupilot/commit/880ebbcdad7bcbee92c8dbef197535c94bc70979 TECS_PITCH_MIN set to positive value was treated as do not use, after the commit positive values are used with no sanity checks.
Version 4.6.0 Platform [ ] All [ ] AntennaTracker [ ] Copter [ x ] Plane [ ] Rover [ ] Submarine
Airframe type Any airframe
Hardware type Cube Orange + and SITL Logs
Thanks for the report, I've added it to the 4.6 issues list
@Georacer we could discuss
I can join EU dev call.
Hello! Thanks for the report! I'll try to be there too.
So, looking at the changes, this is what was before:
if (_pitch_min >= 0) {
_PITCHminf = aparm.pitch_limit_min;
} else {
_PITCHminf = MAX(_pitch_min, aparm.pitch_limit_min);
}
and this is how it is now:
if (_pitch_min == 0) {
_PITCHminf = aparm.pitch_limit_min;
} else {
_PITCHminf = _pitch_min;
}
External pitch limits are applied afterwards and can always constrain more, not relax.
It's been 8 months since I've made this change, so it's not very fresh in my mind.
On the one hand it seems quite purposeful that I've converted >= into ==, judging by the change in the else branch.
But I don't see any benefit in doing so. Except perhaps my own satisfaction that the parameter should be ignored if it's left unset. And if the user wants to set it positive, then I should let them, trusting they have a reason.
This falls onto the same bin as the other discussion with the stick nudging during AUTO modes. What if the pilot keeps the nose high during AUTO mode? The behaviour will be the same as in this ticket. Should we not allow that as well?