Plane: Fix inability to climb the last few meters of takeoff #28707

Georacer · 2024-11-22T14:46:35Z

Summary

This PR addresses #28685.

Details

This affects takeoffs without an airspeed sensor.

Cause

Some airframes will not climb at 0 pitch, regardless of throttle setting.
Thus, the current takeoff level-off strategy of asymptotically forcing pitch to 0, results in the aircraft to never climb the final few meters.

Reproduction

The behaviour was reproduced in SITL, by setting:

PTCH_TRIM_DEG: -10.0
TKOFF_THR_MAX: 75.0

Solution

After the expected time to complete the takeoff has elapsed, drive the desired pitch gradually back to takeoff pitch, over the next 5 seconds.

Testing

Before:

After:

Alternatives

The proposed solution of widening the TECS pitch limits and letting TECS take over and regulate the last few meters of the climb was not feasible:
During an airspeedless takeoff, TECS uses throttle to correct for altitude errors.
It would use pitch (if it was allowed) to correct for airspeed errors).
Thus letting TECS command the pitch angle would not help.

When an airspeed sensor is not used, during a takeoff, the pitch angle is asymptotically driven to 0 as the takeoff altitude is approached. Some airplanes will then stop climbing and fail to reach altitude. If that happens, then minimum pitch is driven back up to taekoff pitch gradually within 5 seconds.

menschel · 2024-11-22T15:29:21Z

Weather is currently not favorable for a test flight but I'll try to test in case it gets better.

Edit: Please also address the forced 100% throttle.

menschel · 2024-11-23T08:15:54Z

Two short flights.
24-11-23_08-37-52.bin
24-11-23_08-42-26.bin
Fun fact, the overshoot now happens based on the workaround.
EDIT: Scratch that, apparently turning into the wind and therefore trading speed for altitude also adds to the overshoot.

IMO the whole thing is connected to a false climb-rate estimation. I have logs from a landing test session with 4.5.7 and I see level-off happening at 3m below target altitude, not 8m.
It may be a good idea to stop limiting the pitch at level-off and see what TECS is doing out of it. If I'm not mistaken, the waypoint is 1km away and TECS should linearly climb.

EDIT:
The 4.5.7 test session. 00000027.BIN
I've also noticed an increasing rate of log corruption with 4.6.0beta. APM Planner complains, MissionPlanner ignores it.

peterbarker · 2024-11-25T23:12:32Z

ArduPlane/takeoff.cpp

@@ -304,6 +304,14 @@ int16_t Plane::get_takeoff_pitch_min_cd(void)
        // if height-below-target has been initialized then use it to create and apply a scaler to the pitch_min
        if (auto_state.height_below_takeoff_to_level_off_cm != 0) {
            float scalar = remaining_height_to_target_cm / (float)auto_state.height_below_takeoff_to_level_off_cm;
+
+            const int32_t takeoff_end_delay_ms = AP_HAL::millis() - takeoff_state.level_off_start_time_ms - g.takeoff_pitch_limit_reduction_sec*1000;


Suggested change

const int32_t takeoff_end_delay_ms = AP_HAL::millis() - takeoff_state.level_off_start_time_ms - g.takeoff_pitch_limit_reduction_sec*1000;

const uint32_t takeoff_end_delay_ms = AP_HAL::millis() - takeoff_state.level_off_start_time_ms - g.takeoff_pitch_limit_reduction_sec*1000;

.... or at least something needs to change here; we rely on integer wrap semantics. Possibly break this into two separate statements.

tridge · 2024-11-25T23:13:46Z

@Georacer I'm surprised you didn't implement the expanding pitch window using TECS like we discussed, did it not work out?

Georacer · 2024-11-26T07:04:26Z

@Georacer I'm surprised you didn't implement the expanding pitch window using TECS like we discussed, did it not work out?

As I wrote in the description, the proposed solution of widening the TECS pitch limits and letting TECS take over and regulate the last few meters of the climb was not feasible:
During an airspeedless takeoff, TECS uses throttle to correct for altitude errors.
It would use pitch (if it was allowed) to correct for airspeed errors; this is not reliable in an airspeedless case, which we are in.
Thus letting TECS command the pitch angle would not help.

I did try it and it didn't give effective results.
If you're really curious I can repeat the experiment.

peterbarker · 2024-11-27T07:59:52Z

Tools/autotest/arduplane.py

+
+        self.delay_sim_time(5)
+
+        self.reboot_sitl(force=True)


Suggested change

self.reboot_sitl(force=True)

self.disarm_vehicle(force=True)

.... we're going to restart SITL anyway, this is a little faster than rebooting.

peterbarker · 2024-11-27T08:02:15Z

ArduPlane/takeoff.cpp

@@ -304,6 +304,14 @@ int16_t Plane::get_takeoff_pitch_min_cd(void)
        // if height-below-target has been initialized then use it to create and apply a scaler to the pitch_min
        if (auto_state.height_below_takeoff_to_level_off_cm != 0) {
            float scalar = remaining_height_to_target_cm / (float)auto_state.height_below_takeoff_to_level_off_cm;
+
+            const int32_t takeoff_end_delay_ms = AP_HAL::millis() - takeoff_state.level_off_start_time_ms - g.takeoff_pitch_limit_reduction_sec*1000;
+            if (takeoff_end_delay_ms > 0) {


This might be a linear_interpolate?

tridge · 2024-11-27T08:15:38Z

discussed on EU dev call, decided just to declare the takeoff complete if we are in the taper off region and we timeout

menschel · 2024-11-27T10:58:54Z

Can we also do something about the now forced 100% throttle?

I argue that it's a wrong design decision not to fall back to TECS controlled throttle, means the airspeed sensor presence should not be evaluated here.
https://github.com/ArduPilot/ardupilot/blob/master/ArduPlane/takeoff.cpp#L282

It also adds to the issue here because it influences the climb-rate estimation.

Georacer · 2024-11-27T16:49:17Z

Can we also do something about the now forced 100% throttle?

I argue that it's a wrong design decision not to fall back to TECS controlled throttle, means the airspeed sensor presence should not be evaluated here. https://github.com/ArduPilot/ardupilot/blob/master/ArduPlane/takeoff.cpp#L282

It also adds to the issue here because it influences the climb-rate estimation.

For this PR we definitely won't change the throttle strategy.

Having a constant THR_MAX was a request from other developers, so you'd have to make your case on why this is a good idea.
Preferably by opening a new ticket.

menschel · 2024-11-30T19:04:53Z

I believe I have the issue #28685 solved in #28776

If I may do so, I'll include your commit d698139 .

Georacer · 2024-12-01T19:02:18Z

Superseded by #28792

Georacer added 2 commits November 22, 2024 15:36

autotest: Created test for unsuccessful takeoff level off

d698139

Georacer mentioned this pull request Nov 22, 2024

AP_TECS: Fix Regression of TKOFF_IGAIN #28352

Closed

menschel mentioned this pull request Nov 23, 2024

ArduPilot 4.6 issues list #28612

Open

23 tasks

tridge added the DevCallEU label Nov 25, 2024

peterbarker requested changes Nov 25, 2024

View reviewed changes

peterbarker requested changes Nov 27, 2024

View reviewed changes

peterbarker removed the DevCallEU label Nov 27, 2024

Georacer mentioned this pull request Dec 1, 2024

Plane: Fix inability to climb the last few meters of takeoff #28792

Merged

Georacer closed this Dec 1, 2024

Georacer deleted the pr/takeoff_level_off_bugfix branch December 1, 2024 19:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plane: Fix inability to climb the last few meters of takeoff #28707

Plane: Fix inability to climb the last few meters of takeoff #28707

Georacer commented Nov 22, 2024

menschel commented Nov 22, 2024 •

edited

Loading

menschel commented Nov 23, 2024 •

edited

Loading

peterbarker Nov 25, 2024

peterbarker Nov 25, 2024

tridge commented Nov 25, 2024

Georacer commented Nov 26, 2024

peterbarker Nov 27, 2024

peterbarker Nov 27, 2024

tridge commented Nov 27, 2024

menschel commented Nov 27, 2024 •

edited

Loading

Georacer commented Nov 27, 2024

menschel commented Nov 30, 2024 •

edited

Loading

Georacer commented Dec 1, 2024

	const int32_t takeoff_end_delay_ms = AP_HAL::millis() - takeoff_state.level_off_start_time_ms - g.takeoff_pitch_limit_reduction_sec*1000;
	const uint32_t takeoff_end_delay_ms = AP_HAL::millis() - takeoff_state.level_off_start_time_ms - g.takeoff_pitch_limit_reduction_sec*1000;

Plane: Fix inability to climb the last few meters of takeoff #28707

Plane: Fix inability to climb the last few meters of takeoff #28707

Conversation

Georacer commented Nov 22, 2024

Summary

Details

Cause

Reproduction

Solution

Testing

Alternatives

menschel commented Nov 22, 2024 • edited Loading

menschel commented Nov 23, 2024 • edited Loading

peterbarker Nov 25, 2024

Choose a reason for hiding this comment

peterbarker Nov 25, 2024

Choose a reason for hiding this comment

tridge commented Nov 25, 2024

Georacer commented Nov 26, 2024

peterbarker Nov 27, 2024

Choose a reason for hiding this comment

peterbarker Nov 27, 2024

Choose a reason for hiding this comment

tridge commented Nov 27, 2024

menschel commented Nov 27, 2024 • edited Loading

Georacer commented Nov 27, 2024

menschel commented Nov 30, 2024 • edited Loading

Georacer commented Dec 1, 2024

menschel commented Nov 22, 2024 •

edited

Loading

menschel commented Nov 23, 2024 •

edited

Loading

menschel commented Nov 27, 2024 •

edited

Loading

menschel commented Nov 30, 2024 •

edited

Loading