Dangerous change in behaviour of TECS_PITCH_MIN

[LupusTheCanine]

Bug report

Issue details

Prior to commit 880ebbc
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

00000012.zip

[rmackay9]

Thanks for the report, I've added it to the 4.6 issues list

[tridge]

@Georacer we could discuss

[LupusTheCanine]

I can join EU dev call.

[Georacer]

Hello! Thanks for the report!
I'll try to be there too.

[Georacer]

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?