Plane: TKOFF_LVL_PITCH and TKOFF_THR_SLEW not respected in auto takeoff

[vierfuffzig]

upon user report on discuss https://discuss.ardupilot.org/t/takeoff-flight-mode-vs-takeoff-in-a-mission-plane-4-0-6-clearly-different/63208/12 i did some SITL testing and found that neither TAKEOFF as a flightmode, nor as a mission command actually respects pitch or throttle slew limitations beyond what is set in THR_SLEWRATE or LIM_PITCH_MAX.

this is auto takeoff as flightmode and mission item with TKOFF_LVL_PITCH = 10 TKOFF_THR_SLEW = 30

there's no throttle slew applied and an increase of des.pitch, though actual pitch is reaching almost twice the set pitch limit during climbout.

Version ArduPlane V4.1.0dev (379506b7)

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

Airframe type SITL

Logs https://github.com/vierfuffzig/Chibios-miscellaneous/raw/master/00000044.BIN

[magicrub]

Thanks for looking into this for forum user Dale2020.

First, some context: The TAKEOFF mode was added relatively recently by @tridge . It was the first flight mode added after the big plane refactor I did 1.5 years ago that allowed for much easier mode additions (like Copter). However, I think it needs better integration in general because it actually operates totally separately from the AUTO mode so the behaviors have diverged. The flight mode and the auto mission takeoff should really get unified into a single takeoff operation that both use the same params and behavior.

[vierfuffzig]

thanks @magicrub for looking into this. iirc with TAKEOFF flightmode a TKOFF parameter group was added, while there‘s older TKOFF parameters living in plane that originally referred to TAKEOFF mission command. while room for optimization of newer TAKEOFF flightmode is likely to be expexted, i wonder why the original TKOFF_THR_SLEW has no effect on TAKEOFF mission item thr handling, ie has it never worked or did something break it? https://github.com/ArduPilot/ardupilot/blob/df90e28b89141b594b0f66c9dbdd78bc7d4ce916/ArduPlane/servos.cpp#L29

[magicrub]

@vierfuffzig param "TKOFF_THRSLEW" already existed and was not actually part of the new "TKOFF...." params added for the mode. Check this list for the new Takeoff mode specific params. https://github.com/ArduPilot/ardupilot/blob/master/ArduPlane/mode_takeoff.cpp

[vierfuffzig]

@magicrub yep, that was what i was trying to say, wondering why parts of the "old" takeoff mission item handling don't work too.

[kniuk]

I just wanted to confirm that I have exactly the same observations as @vierfuffzig. In AUTO TAKEOFF mode looks like THR_SLEWRATE and LIM_PITCH_MAX are used instead of TKOFF_THR_SLEW and TKOFF_LVL_PITCH. I was flying on 4.10 dev built on 23 Nov 2020

[magicrub]

We need to rename this issue to "Mode AUTO takeoff and mode TAKEOFF need to unify behavior". They are currently independent "features" with some copy/paste commonality but they really should be acting the same using shared params and shared logic. Pinging @tridge for awareness.

[vierfuffzig]

@magicrub unifying behaviour of mode-type and mission-type takeoff sure is part of it, but what i was trying to point out too is that in SITL TKOFF_THR_SLEW and TKOFF_LVL_PITCH don't take effect at all, neither in mode-type, nor in mission-type takeoff. see log plots above. i assumed this applied to actual hardware too but didn't actually test yet. will do that and report back.

[vierfuffzig]

testing on actual hardware running current master shows this likely is two seperate issues really:

1. diverging mode-type and mission-type TAKEOFF behaviour

2. TKOFF_THR_SLEW and _LVL_PITCH don't seem to work in SITL mission-type TAKEOFF

[kniuk]

I put some effort to analyze code related to setting pitch for auto take off. Please bare in mind I am new to this, so some of my analysis may be pure bollocks. However for me it looks like LVL_PITCH is more strictly obeyed when no air speed is used. With air speed present AP just makes sure pitch is not less than LVL_PITCH

if (nav_pitch_cd < takeoff_pitch_min_cd))

presented fragment of code is from: void Plane::takeoff_calc_pitch(void) @ takeoff.cpp

if (ahrs.airspeed_sensor_enabled()) {
    int16_t takeoff_pitch_min_cd = get_takeoff_pitch_min_cd();
    calc_nav_pitch();
    if (nav_pitch_cd < takeoff_pitch_min_cd) {
        nav_pitch_cd = takeoff_pitch_min_cd;
    }
} else {
    nav_pitch_cd = ((gps.ground_speed()*100) / (float)aparm.airspeed_cruise_cm) * auto_state.takeoff_pitch_cd;
    nav_pitch_cd = constrain_int32(nav_pitch_cd, 500, auto_state.takeoff_pitch_cd);
}

It is even called "pitch min" So we got maximum climb, with just extra safety of keeping it no less than LVL_PITCH Maximum climb is forced by this: void ModeTakeoff::update() @ mode_takeoff.cpp

    plane.prev_WP_loc = plane.current_loc;
    plane.next_WP_loc = plane.current_loc;
    plane.next_WP_loc.alt += alt*100.0;
    plane.next_WP_loc.offset_bearing(direction, dist);

For no airspeed we have value between 5 degree and LVL_PITCH

What do you think?

[kniuk]

I was thinking about these lines:

if (nav_pitch_cd < takeoff_pitch_min_cd) {
    nav_pitch_cd = takeoff_pitch_min_cd;
}

And I am leaning towards an idea that it is a typo and should read:

if (nav_pitch_cd > takeoff_pitch_min_cd) {
    nav_pitch_cd = takeoff_pitch_min_cd;
}

I think now I am starting to understand @magicrub opinion that unifying mission takeoff and mode takeoff is the only way to go for clear code here.

[magicrub]

Any volunteers?

[stevegut78]

This issue is still open. Any word on a fix?

[skorokithakis]

I'd just like to chime in here to say that a TKOFF_THR_SLEW value of -1 should still ramp the throttle up instantly. For hand launches, all I care about is getting the maximum speed possible in the minimum time possible, so any change should preserve that behaviour for a slew rate value of -1.

I say this because I think that the takeoff WP changed the behaviour and now hand launches are very hit-and-miss, with the plane crashing much more frequently during launch.

[julled]

we also stumbled upon this problem, a fix would be very nice!