plane: restore thr_min behavior

[IamPete1]

fix for #11030

This now constrains throttle to between thr_min and thr_max in FBWA/SATABILIZE ect. I couldn't track down how this was done before. Possibly the throttle was interpolated to between thr_min and thr_max? This currently just constrains so some throttle bellow thr_min has no change.

[IamPete1]

The throttle was originaly constrained so the fix is correct

[Jaaaky]

It was constrained previously unless throttle_passthru_stabilize is set.
If throttle_passthru_stabilize is set, it works like manual.

[Jaaaky]

I've added one more related problem in the original bug report;
#11030 (comment)

I think those 3 lines here

ardupilot/ArduPlane/servos.cpp

Line 423 in 6dd05db

SRV_Channels::set_output_scaled(SRV_Channel::k_throttle, 0);

Should be changed as follows;

            SRV_Channels::set_output_scaled(SRV_Channel::k_throttle, min_throttle);
            SRV_Channels::set_output_scaled(SRV_Channel::k_throttleLeft, min_throttle);
            SRV_Channels::set_output_scaled(SRV_Channel::k_throttleRight, min_throttle);

[IamPete1]

This now also outputs throttle min when disarmed as per ARMING_REQUIRE description. This is abit odd tho so now THR_MIN is respected when disarmed and in all modes except manual. I wonder if we would be better moving THR_MIN over to the ICE library for this use case (ie idling with THR_MIN). It seems that THR_MIN when disarmed may have been broken for some time and it now has the dual purpose of enabling the reverse thrust stuff.

[DavidIngraham]

I think that the parameter description is broken at this point. It would be very dangerous to change this behavior now to make it output throttle_min when disarmed. I can't think of a use case where it would be useful for it to do that, even with an ICE engine.

[IamPete1]

@DavidIngraham Yeah I think I agree, thr_min should be for armed only and a new parameter linked to the ignition switch functionality should be set up in the ICE library.

[IamPete1]

I think we should either remove the arming require thr_min option and replace it with #11109 in the ICE lib (this also gives added functionality of having engine start and stop on a switch) and/or fix the behavior here but change the default to arming require zero throttle.

[DavidIngraham]

Yes, that is a much more sensible approach and will be much safer

…

On Sat, Apr 13, 2019 at 3:43 AM Peter Hall ***@***.***> wrote: I think we should either remove the arming require thr_min option and replace it with #11109 <#11109> in the ICE lib (this also gives added functionality of having engine start and stop on a switch) and/or fix the behavior here but change the default to arming require zero throttle. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11059 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACt5Gdci_Zgfr8rB99_Ymvw-7Pl9qp7Nks5vgbTLgaJpZM4cjV_f> .