FlightTasks: add Descend task to land without GPS #13143

julianoes · 2019-10-09T13:09:53Z

This adds a flight task to catch the case where we want to do an emergency descent without GPS but only a baro.

Previously, this would lead to the navigator land class being called without position estimates which then made the flight tasks fail and react with a flight task failsafe. This however meant that landed was never detected and a couple of confusing error messages.

This applies if NAV_RCL_ACT is set to 3 "land".

Fixes #11468.

Tested in SITL with iris, plane, and standard_vtol.

julianoes · 2019-10-09T14:25:03Z

I tried this with and without GPS withvtol_standard and in both cases it transitioned to multicopter flight and then descended. I think that's correct but I don't exactly understand why/how the backtransition is triggered.

ThomasRigi · 2019-10-10T07:35:02Z

I tried this with and without GPS withvtol_standard and in both cases it transitioned to multicopter flight and then descended. I think that's correct but I don't exactly understand why/how the backtransition is triggered.

@julianoes I think this is due to NAV_FORCE_VT parameter. Not sure about the exact implementation though. If NAV_FORCE_VT is true then the vehicle will / should always land (and takeoff) in MC mode. Meaning that if you deactivate it it should stay in FW for landing

mhkabir · 2019-10-10T14:09:43Z

How is this different from the Failsafe flight task?

bresch · 2019-10-11T08:36:54Z

I also don't really see the advantage over the failsafe flight task. Also, no GPS doesn't mean no position/velocity estimate, the drone can still have an optical flow sensor and brake horizontal speed.

julianoes · 2019-10-11T11:49:40Z

The failsafe task will actually stay at the same position because we have a valid z-position, however, we should descend.

julianoes · 2019-10-14T12:02:51Z

@MaEtUgR please review.

julianoes · 2019-10-14T12:10:03Z

@Tony3dr I would appreciate if you could test this pull request carefully on the cheapest multicopter 😄:

Test instructions
Before the tests:

Set RC failsafe to Land (NAV_RCL_ACT = 3)
Make sure RC failsafe is correctly detected (on ground)
Test:
Hover in POSCTL, then switch off RC -> it should land nicely
Test:
Fly without GPS, so after powering on PX4, open mavlink shell and do gps stop.
Hover in ALTCTL (maybe around 1-2 meters), then switch off RC -> it should descend/land but it will drift with wind, so you need to make sure you have enough space and no wind!

Nevermind please wait with this test.

MaEtUgR · 2019-10-14T13:31:27Z

src/lib/FlightTasks/tasks/Descend/FlightTaskDescend.cpp

+	bool ret = FlightTask::activate(last_setpoint);
+	_position_setpoint = {NAN, NAN, NAN};
+	_velocity_setpoint = {NAN, NAN, NAN};
+	_thrust_setpoint = matrix::Vector3f(0.0f, 0.0f, -_param_mpc_thr_hover.get() * 0.6f);


Note to myself: This conflicts with #12072 and will only show up as a compile error.

MaEtUgR · 2019-10-14T13:35:52Z

src/modules/navigator/navigator_main.cpp

@@ -620,6 +615,7 @@ Navigator::run()
 		case vehicle_status_s::NAVIGATION_STATE_ACRO:
 		case vehicle_status_s::NAVIGATION_STATE_ALTCTL:
 		case vehicle_status_s::NAVIGATION_STATE_POSCTL:
+		case vehicle_status_s::NAVIGATION_STATE_DESCEND:


It would anyways run into default.

src/lib/FlightTasks/tasks/Descend/FlightTaskDescend.cpp

julianoes · 2019-10-14T13:55:21Z

@Tony3dr sorry, don't test just yet. We need to change something first.

Tony3dr · 2019-10-14T16:24:49Z

@Tony3dr sorry, don't test just yet. We need to change something first.

Sounds good let us know when it's ready.

MaEtUgR · 2019-10-15T12:12:58Z

@julianoes I pushed on your pr, I hope that's ok.

The first commit I squeezed in is fixing the failsafe handling if erroneous non-executable setpoints get into the position controller which was the case before. It then still does an emergency descend but spitting warnings this time.
The second commit is yours unchanged
The third commit contains my suggestions to use the flight task interface for that case.

I tested before and after the descend introduction and it does the right thing in the case you described (default configuration NAV_RCL_ACT = 3 suffered the same problem). The case where you lose the altitude estimate is hard to test currently... not sure if that's supported at the moment.

julianoes · 2019-10-15T12:57:58Z

@MaEtUgR I've tested these cases:

NAV_RCL_ACT=3 -> GPS ok and RC lost -> switches from POSCTL to LAND and lands ✔️
NAV_RCL_ACT=3 -> no GPS and RC lost -> switches from POSCTL to LAND and descends ✔️
NAV_RCL_ACT=0 -> GPS ok and RC lost -> stays still in POSCTL and stays still ✔️
NAV_RCL_ACT=0 -> no GPS and RC lost -> stays in ALTCTL but descends ✖️

For 4. the output is:

WARN  [commander] Manual control lost
WARN  [mc_pos_control] Position-Control Setpoint-Update failed
WARN  [mc_pos_control] Failsafe: blind hover

MaEtUgR · 2019-10-15T16:07:11Z

I wonder why it would use so much more flash...

@julianoes

"stays in ALTCTL but descends" that's what I implemented because:
- you are in altitude mode relying on RC
- you loose RC but have no reaction so you stay relying on it
- the consumer trying to use RC is failing so you go into last resort handling
- there's no GPS to just stop and wait so it stays level and will start drifting
- because drifting without RC is not a sustainable state it's descending
  I consider this much safer than any other solution like staying on altitude drifting away which is a flyaway or terminating which is probably unexpected and is likely causing more damage.
You must be running a different/old branch since I replaced the posted message

Failsafe: blind hover

Can you check again?

julianoes · 2019-10-16T11:18:41Z

@MaEtUgR I checked 4. again and you're right. It's now correct. And I agree with your reasoning of switching to descend for this case.

WARN  [commander] Manual control lost
WARN  [mc_pos_control] Position-Control Setpoint-Update failed
WARN  [mc_pos_control] Failsafe: blind land

julianoes · 2019-10-16T11:19:43Z

So now the only thing is to figure out how this fits into fmu-v2.

julianoes · 2019-10-16T12:23:01Z

@Tony3dr ok I think you can go ahead and test this now:

Test instructions
Before the tests:

Set RC failsafe to Land (NAV_RCL_ACT = 3)
Make sure RC failsafe is correctly detected (on ground)
Test:
Hover in POSCTL, then switch off RC -> it should land (not RTL) nicely
Test:
Fly without GPS, so after powering on PX4, open mavlink shell and do gps stop. Don't disconnect the GPS, otherwise you don't have the external mag.
Hover in ALTCTL (maybe around 1-2 meters), then switch off RC -> it should descend/land but it will drift with wind, so you need to make sure you have enough space and no wind!

dannyfpv · 2019-10-17T21:29:00Z

Tested on Pixhawk 4 v5 f-450

NAV_RCL_ACT = land mode

1 log switch off RC with GPS: no issues
https://review.px4.io/plot_app?log=2ea6423f-b232-45c9-8c40-32e775118359

2 log switch off RC without GPS: no issues drift with wind 30 meters
https://review.px4.io/plot_app?log=3104d449-dd29-473f-9055-354984ff8afa

LorenzMeier · 2019-10-18T09:55:26Z

I've updated the branch - let's see if it passes now.

This adds a flight task to catch the case where we want to do an emergency descent without GPS but only a baro. Previously, this would lead to the navigator land class being called without position estimates which then made the flight tasks fail and react with a flight task failsafe. This however meant that landed was never detected and a couple of confusing error messages. This applies if NAV_RCL_ACT is set to 3 "land".

julianoes · 2019-10-18T11:18:22Z

@dannyfpv thanks a lot for testing and adding the logs, that's very helpful!

@MaEtUgR we need to check the descend case, something is odd with thrust:

julianoes · 2019-10-18T11:46:25Z

@MaEtUgR it looks like ground_contact triggers but always after throttle is set to 0, somehow.

And I can't reproduce it in SITL.

julianoes · 2019-10-18T12:26:58Z

Looking at the vertical position z estimate it looks like not enough vertical movement which triggered the ground contact.

julianoes · 2019-10-18T14:14:00Z

@bresch and I found that the land detector determined:

vertical_movement to be false: https://github.com/PX4/Firmware/blob/95dc522b99688a08402ac6462ad28f5dfd338eda/src/modules/land_detector/MulticopterLandDetector.cpp#L154
hit_ground to be true: https://github.com/PX4/Firmware/blob/95dc522b99688a08402ac6462ad28f5dfd338eda/src/modules/land_detector/MulticopterLandDetector.cpp#L165

This happens because the z controller only reacts slowly to the new setpoint so the z velocity down is much less than the z velocity setpoint here:
https://github.com/PX4/Firmware/blob/95dc522b99688a08402ac6462ad28f5dfd338eda/src/modules/land_detector/MulticopterLandDetector.cpp#L151-L154

It looks to us that we should check the land detector for this edge cases in a future pull request. The issue is general and not specific to this change. Therefore we can merge this PR already.

bresch

Let's merge that and address the land detector in a separate PR.

julianoes requested review from MaEtUgR, Stifael and sfuhrer October 9, 2019 13:09

weekly-digest bot mentioned this pull request Oct 13, 2019

Weekly Digest (6 October, 2019 - 13 October, 2019) #13171

Closed

MaEtUgR reviewed Oct 14, 2019

View reviewed changes

Tony3dr added this to Open Pull Requests in Test Flights Oct 14, 2019

MaEtUgR force-pushed the pr-fix-emergency-descend branch from b1b1e83 to e67b5db Compare October 15, 2019 11:59

julianoes added this to To Do in Release 1.11 Blockers via automation Oct 16, 2019

julianoes moved this from To Do to Testing in Release 1.11 Blockers Oct 16, 2019

dagar self-requested a review October 16, 2019 16:27

MaEtUgR and others added 3 commits October 18, 2019 11:56

mc_pos_control: execute failsafe with invalid setpoints

624473f

FlightTaskDescend: set no vertical thrust when commanding velocity

935897a

julianoes force-pushed the pr-fix-emergency-descend branch from bb2a0ea to 935897a Compare October 18, 2019 09:57

julianoes requested a review from bresch October 18, 2019 14:15

bresch approved these changes Oct 18, 2019

View reviewed changes

julianoes merged commit 22c4bb4 into master Oct 18, 2019

Release 1.11 Blockers automation moved this from Testing to Done Oct 18, 2019

Test Flights automation moved this from Open Pull Requests to Done Oct 18, 2019

julianoes deleted the pr-fix-emergency-descend branch October 18, 2019 14:18

julianoes mentioned this pull request Oct 18, 2019

Land detector edge case in descend mode #13231

Closed

weekly-digest bot mentioned this pull request Oct 20, 2019

Weekly Digest (13 October, 2019 - 20 October, 2019) #13236

Closed

mrpollo mentioned this pull request Nov 7, 2019

v1.10.0 Release Notes + Changelog #13366

Closed

julianoes mentioned this pull request Dec 17, 2019

LAND commanded but not land. #13754

Closed

RomanBapst restored the pr-fix-emergency-descend branch May 15, 2020 12:07

MaEtUgR deleted the pr-fix-emergency-descend branch July 26, 2020 15:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FlightTasks: add Descend task to land without GPS #13143

FlightTasks: add Descend task to land without GPS #13143

julianoes commented Oct 9, 2019 •

edited

Loading

julianoes commented Oct 9, 2019

ThomasRigi commented Oct 10, 2019

mhkabir commented Oct 10, 2019

bresch commented Oct 11, 2019

julianoes commented Oct 11, 2019

julianoes commented Oct 14, 2019

julianoes commented Oct 14, 2019 •

edited

Loading

MaEtUgR Oct 14, 2019 •

edited

Loading

MaEtUgR Oct 14, 2019

julianoes commented Oct 14, 2019

Tony3dr commented Oct 14, 2019 •

edited by MaEtUgR

Loading

MaEtUgR commented Oct 15, 2019

julianoes commented Oct 15, 2019

MaEtUgR commented Oct 15, 2019 •

edited

Loading

julianoes commented Oct 16, 2019 •

edited

Loading

julianoes commented Oct 16, 2019

julianoes commented Oct 16, 2019 •

edited

Loading

dannyfpv commented Oct 17, 2019 •

edited by Tony3dr

Loading

LorenzMeier commented Oct 18, 2019

julianoes commented Oct 18, 2019 •

edited

Loading

julianoes commented Oct 18, 2019 •

edited

Loading

julianoes commented Oct 18, 2019

julianoes commented Oct 18, 2019

bresch left a comment

FlightTasks: add Descend task to land without GPS #13143

FlightTasks: add Descend task to land without GPS #13143

Conversation

julianoes commented Oct 9, 2019 • edited Loading

julianoes commented Oct 9, 2019

ThomasRigi commented Oct 10, 2019

mhkabir commented Oct 10, 2019

bresch commented Oct 11, 2019

julianoes commented Oct 11, 2019

julianoes commented Oct 14, 2019

julianoes commented Oct 14, 2019 • edited Loading

MaEtUgR Oct 14, 2019 • edited Loading

Choose a reason for hiding this comment

MaEtUgR Oct 14, 2019

Choose a reason for hiding this comment

julianoes commented Oct 14, 2019

Tony3dr commented Oct 14, 2019 • edited by MaEtUgR Loading

MaEtUgR commented Oct 15, 2019

julianoes commented Oct 15, 2019

MaEtUgR commented Oct 15, 2019 • edited Loading

julianoes commented Oct 16, 2019 • edited Loading

julianoes commented Oct 16, 2019

julianoes commented Oct 16, 2019 • edited Loading

dannyfpv commented Oct 17, 2019 • edited by Tony3dr Loading

LorenzMeier commented Oct 18, 2019

julianoes commented Oct 18, 2019 • edited Loading

julianoes commented Oct 18, 2019 • edited Loading

julianoes commented Oct 18, 2019

julianoes commented Oct 18, 2019

bresch left a comment

Choose a reason for hiding this comment

julianoes commented Oct 9, 2019 •

edited

Loading

julianoes commented Oct 14, 2019 •

edited

Loading

MaEtUgR Oct 14, 2019 •

edited

Loading

Tony3dr commented Oct 14, 2019 •

edited by MaEtUgR

Loading

MaEtUgR commented Oct 15, 2019 •

edited

Loading

julianoes commented Oct 16, 2019 •

edited

Loading

julianoes commented Oct 16, 2019 •

edited

Loading

dannyfpv commented Oct 17, 2019 •

edited by Tony3dr

Loading

julianoes commented Oct 18, 2019 •

edited

Loading

julianoes commented Oct 18, 2019 •

edited

Loading