THR_MIN is not enforced on FBWA mode since c290b1f3b5525f8

[Jaaaky]

Bug report

Issue details

_Throttle Minimum - THRMIN - is not enforced on FBWA/STABILIZE mode as of master since commit c290b1f3b5525f8 , minimum is always zero. This was not the behavior previously. On Auto mode it works fine

Version master since commit c290b1f3b5525f8

Platform [ ] All [ ] AntennaTracker [ ] Copter [ x ] Plane [ ] Rover [ ] Submarine

Airframe type N/A

Hardware type What autopilot hardware was used? (Pixhawk, Cube, Pixracer, Navio2, etc) Pixhawk1

I've used the git bisect tool to investigate the issue and found that the 1st bad commit is c290b1f3b5525f8

[IamPete1]

this is the commit in question https://github.com/ArduPilot/ardupilot/commit/c290b1f3b5525f8f9b0dcd2efe76547869c89bc7

looks like the change is due to going from get_throttle_input(true) to channel_throttle->get_control_in()

it does say Minimum throttle percentage used in automatic throttle modes.

FBWA is not a automatic throttle mode. What is the behaviour you were expecting?

[Jaaaky]

As I said, long time I've been using arduplane thr_min on fbwa/stabilize was enforced, so I am really surprised you say FBWA is not auto throttle mode. I think me, and I know some other people I know, expect minimum to go to zero only on manual mode and on landing in AUTO, otherwise thr_min should be used. As I understand the main purpose of this - setting thr_min to > 0 - is to allow petrol engines to completely turn off on landing, or using manual in case of some emergency, otherwise when thr_min is higher than zero it should still be working and doesn't turn off.

[Jaaaky]

@IamPete1 Per documentation my expected behavior is what is documented. Read here http://ardupilot.org/plane/docs/parameters.html#thr-pass-stab-throttle-passthru-in-stabilize

THR_MIN & THR_MAX should be enforced in FBWA/STABILIZE/ACRO unless THR_PASS_STAB is enabled.

[Jaaaky]

Other references saying the same; http://ardupilot.org/plane/docs/fbwa-mode.html?highlight=thr_min http://ardupilot.org/plane/docs/stabilize-mode.html?highlight=thr_min http://ardupilot.org/plane/docs/automatic-landing.html?highlight=thr_min http://ardupilot.org/plane/docs/common-ice.html?highlight=thr_min

[vierfuffzig]

i too think this is a relevant change from previous / documented behaviour. imho it makes sense to keep throttle limits on stabilized modes too. among other things this allows limiting THR_MAX in all relevant flight modes when using low C / high capacity batteries like Liion packs.

basti.

[IamPete1]

@Jaaaky This was discussed in the DevCall and it was decided that the parameter description was wrong and the behavior should be as you described. I have done a fix. However I couldn't find how it was done originally. Was the throttle constrained? such that at bellow THR_MIN % the throttle stick does nothing. Or was it interpolated so that the full stick range was remapped between THR_MIN and THR_MAX?

[vierfuffzig]

constrained:

[IamPete1]

@vierfuffzig Great, the fix is correct then, thanks

[vierfuffzig]

checked out https://github.com/ArduPilot/ardupilot/pull/11059 , thanks for fixing @IamPete1 !

basti.

[Jaaaky]

I've found another problem with THR_MIN; Per documentation; When ARMING_REQUIRE is set to 1 it should send THR_MIN PWM when disarmed

But actually it sends the servo minimum not THR_MIN. Which cause petrol engine to turn off.

[IamPete1]

this would be a change of behaviour rather than a bug fix?

Not done any ICE stuff, but you would definitely not want this on a electric model

[IamPete1]

yeah, looks ICE stuff is dealt with in a different way, http://ardupilot.org/plane/docs/common-ice.html , have you tested that stuff?

[Jaaaky]

I understand your point, but why would some one set THR_MIN > 0 on electric model?

Also, why it was documented this way before? And yes, the ICE documentation says that this should be the behavior; I've to set THR_MIN and ardupilot should enforce it.

[IamPete1]

if you what that you should just set the servo min for that channel. ICE stuff overrides here https://github.com/ArduPilot/ardupilot/blob/6dd05db3b7a20228101a06543fcdee5457a14318/ArduPlane/servos.cpp#L791

[Jaaaky]

Actually this is only for using a starter, and I don't have one. I start it manually with a drill. It has nothing to do with THR_MIN. I have tried to change ICE_START_PCT but it has no effect on minimum throttle.

Again what is the whole point of setting THR_MIN > 0 , if not intentionally need to enforce?

If there is some other way, or no way at all, tell me. But, then, parameter description should be fixed.

[IamPete1]

ICE_PWM_IGN_ON is the one that should effect minimum throttle, maybe you could do a PR with the changes you outlined here https://github.com/ArduPilot/ardupilot/pull/11059#issuecomment-482501783 then we can discus on the dev call, as i say i'm not familiar with how the ICE stuff is supposed to work. Personally i think we should not output anything when disarmed, unless over ridden by the ICE stuff. Using ICE engine is the only reason i can think of to want to output some throttle when disarmed. And if you have ICE you would be better using the ICE params because you get additional functionality. But as you say that contradicts the ARMING_REQUIRE description. I suspect that functionality is left over from before the ICE stuff was added.

[IamPete1]

I was wrong about ICE_PWM_IGN_ON, as you say it should be THR_MIN. I still think the functionality would be better to be moved to the ICE library. https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_ICEngine/AP_ICEngine.cpp

[Jaaaky]

I've sent a pull request

[IamPete1]

https://github.com/ArduPilot/ardupilot/pull/11109 has been merged so you can now use the ICE functionally to idle. I suspect the THR_MIN when disarmed functionality will be removed soon, or rather not fixed and ARMING_REQUIRE description changed.

[Jaaaky]

OK, Thanks. But THR_MIN enforcing on FBWA/Stabilize is not fixed yet?

11109 has been merged so you can now use the ICE functionally to idle. I suspect the THR_MIN when disarmed functionality will be removed soon, or rather not fixed and ARMING_REQUIRE description changed.

[IamPete1]

this new stuff ICE enforces the idle throtttle all the time, not fixed the THR_MIN stuff yet

[IamPete1]

https://github.com/ArduPilot/ardupilot/pull/11059 has been merged, so this should be fixed