commander: fix battery failsafe without GPS #13294

julianoes · 2019-10-28T10:04:32Z

This fixes the battery failsafe for the following corner cases:

Battery failsafe set to Return but we can't do RTL because we don't have a global position or home position. In this case we now switch to Land. Land might end up in Descend in the failsafe state machine later.
Battery failsafe set to Land but we can't land because we don't have a local position. In this case we switch to land anyway and then fall back to descend in the failsafe state machine later.

The "fix" involves ignoring using the main_state_transition and implementing the guards in place. This is a hack for now but should cover the corner case until a more thorough refactor.

The different failsafe state machines have involved over time from requirements and learnings based on developed solutions and products. The implementations in various places will need to get consolidated in the future.

Tested in SITL for Return and Land with and without GPS.

Fixes #13291.

julianoes · 2019-10-28T10:21:48Z

@PX4/testflights please test this on the cheapest frame as follows:

For these tests I suggest to set the battery failsafe thresholds higher in QGC -> Settings -> Safety. (e.g. warning at 60%, failsafe at 50%, emergency at 20%. That way you don't need to risk damaging batteries.

Test 1: Land with GPS

Set the battery failsafe action to "Land" Mode.
Fly in POSCTL 2m above a possible landing spot.
Wait for the critical battery warning to appear. It should then nicely land.

Test 2: Land without GPS -> only when no wind!

Set the battery failsafe action to "Land" Mode.
Disable GPS by doing gps stop in mavlink_shell.
Fly in ALTCL 2m above a possible landing spot (and some space for drift with wind).
Wait for the critical battery warning to appear. It should then descend (but drift with the wind).

Test 3: RTL with GPS

Set the battery failsafe action to "Return mode at critically low level, Land mode at ..." Mode.
Fly in POSCTL maybe at 5m maybe about 10m from the takeoff position.
Wait for the critical battery warning to appear. It should then do RTL as usual. If the emergency battery level is also triggered it will switch to Land which would happen if the battery estimate goes down quickly.

Test 4: RTL without GPS -> only when no wind!

Set the battery failsafe action to "Return mode at critically low level, Land mode at ..." Mode.
Disable GPS by doing gps stop in mavlink_shell.
Fly in ALTCL 2m above a possible landing spot (and some space for drift with wind).
Wait for the critical battery warning to appear. It should then descend (but drift with the wind).

Thanks and let me know if you have any questions!

LorenzMeier · 2019-10-28T10:30:02Z

@julianoes Thanks for providing a really good test plan. You're setting a very good example with that and a lot of PRs are lacking that.

This fixes the battery failsafe for the following corner cases: - Battery failsafe set to Return but we can't do RTL because we don't have a global position or home position. In this case we now switch to Land. Land might end up in Descend in the failsafe state machine later. - Battery failsafe set to Land but we can't land because we don't have a local position. In this case we switch to land anyway and then fall back to descend in the failsafe state machine later. The "fix" involves ignoring using the main_state_transition and implementing the guards in place. This is a hack for now but should cover the corner case until a more thorough refactor. The different failsafe state machines have involved over time from requirements and learnings based on developed solutions and products. The implementations in various places will need to get consolidated in the future. Tested in SITL for Return and Land with and without GPS.

src/modules/commander/state_machine_helper.cpp

potaito

To me it is now clearer what happens when RTL or similar are executing without GPS.

dannyfpv · 2019-10-29T22:41:42Z

Tested on Pixhawk v4pro f-450

Test 1: Land with GPS
Note:
Waited for the critical battery to appear the vehicle landed smoothly only issue is that the vehicle did not disarm after landing.
Log:
https://review.px4.io/plot_app?log=4633646c-e55e-4c54-8c6f-a8df6c1502bf

Test 2: Land without GPS
Note:
Waited for the critical battery warning to appear. The vehicle descends, drifted with the wind. Only issue is that the vehicle did not disarm after landing.
Log:
https://review.px4.io/plot_app?log=056f0e27-9da1-4c55-8c99-196ad6f7620b

Test 3: RTL with GPS
Notes:
Set the battery failsafe action to "Return mode at critically low level, Land mode at ..." Mode.
Waited for the critical battery warning to appear. The vehicle triggered RTL. The only issue noted is that the vehicle did not disarm after landing.
Logs:
https://review.px4.io/plot_app?log=399723f5-89dc-4a6b-8b3e-957c3d5117f3

Test 4: RTL without GPS
Notes:
Set the battery failsafe action to "Return mode at critically low level, Land mode at ..." Mode.
Waited for the critical battery warning to appear. The vehicle descends, drifted with the wind. The only issue noted is that the vehicle did not disarm after landing.
Logs:
https://review.px4.io/plot_app?log=0051d897-b88e-44ae-9c0c-376e6eacc30c
@julianoes the test was completed, everything seemed to work as expected, except in all the tests the vehicle did not disarm upon touching the ground.

julianoes · 2019-10-30T10:01:19Z

@dannyfpv Thanks a lot for testing and the logs. I'm investigating the problem where it does not auto-disarm on landing. From the log I can see that landing is detected but then disarm does not happen. I can't reproduce this behaviour in SITL, so I'm wondering what might be different.

One thing that caught my eye is the weird noise/spikes in the mag values. It's something we need to investigate. I don't know if this has something to do with the fact that auto-disarm does not work.

Tony3dr · 2019-10-30T17:46:45Z

FC: PixhawkPro
Frame: DJI F450
GPS: CUAV U-Blox NEO-M8N High Precision GPS
ESC: 20A
Motors: Tmotors 2212-920kv
log A log with logging set from boot
https://review.px4.io/plot_app?log=49829594-e88e-4f85-8740-6b2f7bf87c47&fbclid=IwAR1vs4sYoGntIlDX6kiA_58IYzI2y26o6n-Zo_7NfdaaunZl9lXonkU9zls

@julianoes

julianoes · 2019-10-31T16:13:19Z

@dannyfpv @Tony3dr I'm trying to figure out if auto disarm did not work or if landing was not detected (and therefore auto disarm did not work). Do you remember?

From the logfiles, I'm not entirely sure. In some logfiles you switch to manual, in others you seem to use the kill switch.

Tony3dr · 2019-10-31T19:47:05Z

@dannyfpv @Tony3dr I'm trying to figure out if auto disarm did not work or if landing was not detected (and therefore auto disarm did not work). Do you remember?

From the logfiles, I'm not entirely sure. In some logfiles you switch to manual, in others you seem to use the kill switch.

@julianoes the landing was detected, once the vehicle triggered the failsafe landing was detected. The reason why we switched to manual is to disarm the vehicle by using the kill switch. We can give it another go on Monday. Get more data to see if we can figure out what could be the issue.

RomanBapst · 2019-11-01T06:52:37Z

src/modules/commander/state_machine_helper.cpp

@@ -1194,13 +1195,9 @@ void battery_failsafe(orb_advert_t *mavlink_log_pub, const vehicle_status_s &sta

 		// FALLTHROUGH
 		case LOW_BAT_ACTION::LAND:
-			if (TRANSITION_DENIED != main_state_transition(status, commander_state_s::MAIN_STATE_AUTO_LAND, status_flags,


@julianoes Sorry, I can't comment on a line which you did not change but don't you need to put the LOW_BAT_ACTION::RETURN_OR_LAND case before the LOW_BAT_ACTION::RETURN case, as you have done for the critical battery level section? As I understand it will otherwise fall through to landing and never RTL.

I think this is correct because in the emergency case we want to land, so we fallthrough to land.

In the critical (non-emergency) case we still do RTL.

dannyfpv · 2019-11-02T00:16:19Z

Tested on Pixhawk4 v5 f-450

Test 1: Land with GPS
Note:
Waited for the critical battery to appear the vehicle landed smoothly only issue is that the vehicle did not disarm after landing.
Log:
https://review.px4.io/plot_app?log=242b4d47-50f1-4975-9000-a5df63b92175

Test 2: Land without GPS
Note:
Waited for the critical battery warning to appear. The vehicle descends, drifted with the wind. The only issue is that the vehicle did not disarm after landing.
Log:
https://review.px4.io/plot_app?log=f34aeffd-50e4-48a3-a23d-69cadc351a12

Test 3: RTL with GPS
Notes:
Set the battery failsafe action to "Return mode at a critically low level, Land mode at ..." Mode.
Waited for the critical battery warning to appear. The vehicle triggered RTL. The only issue noted is that the vehicle did not disarm after landing.
Logs:
https://review.px4.io/plot_app?log=c6e55aaa-f026-407a-bcad-29b7a73ee04c

Test 4: RTL without GPS
Notes:
Set the battery failsafe action to "Return mode at a critically low level, Land mode at ..." Mode.
Waited for the critical battery warning to appear. The vehicle descends, drifted with the wind. The only issue noted is that the vehicle did not disarm after landing.
Logs:
https://review.px4.io/plot_app?log=e5602465-fc8e-4c44-a351-2a39ea96e114

@julianoes the same behavior in all the tests the vehicle did not disarm upon touching the ground.

julianoes · 2019-11-04T14:10:44Z

@dannyfpv when you do a normal RTL with the same vehicle on master: does auto-disarm work?

julianoes · 2019-11-04T14:57:54Z

@dannyfpv I think your logs got a bit messed up above. The first two both seem to have GPS.

julianoes · 2019-11-04T14:59:12Z

@Tony3dr:

@julianoes the landing was detected, once the vehicle triggered the failsafe landing was detected.

Does that mean that the failsafe was done or that landing was actually detected and the props were spinning slower (but not disarmed)?

julianoes · 2019-11-04T15:53:18Z

From the logs I can tell that the problem seems to be the land estimator not triggering, rather than auto-disarm not working.

@bresch I'm looking at this log and I can't figure out why the land detector doesn't trigger:
https://review.px4.io/plot_app?log=f34aeffd-50e4-48a3-a23d-69cadc351a12

Tony3dr · 2019-11-04T17:18:21Z

@dannyfpv I think your logs got a bit messed up above. The first two both seem to have GPS.
@julianoes sorry about that it looks like the order of the logs is off, see below.
Test 1: Land with GPS
https://review.px4.io/plot_app?log=242b4d47-50f1-4975-9000-a5df63b92175
https://review.px4.io/plot_app?log=f34aeffd-50e4-48a3-a23d-69cadc351a12

Test 2: Land without GPS
https://review.px4.io/plot_app?log=c6e55aaa-f026-407a-bcad-29b7a73ee04c

Test 3: RTL with GPS
https://review.px4.io/plot_app?log=f34aeffd-50e4-48a3-a23d-69cadc351a12

Tony3dr · 2019-11-04T17:21:54Z

@dannyfpv when you do a normal RTL with the same vehicle on master: does auto-disarm work?
@julianoes we will post a couple of logs of the Master branch, later on, today we posted a couple in #13321, The landing problem is apparently an issue on current Master that is been worked on.

dannyfpv · 2019-11-04T23:29:23Z

tested on pixhawk4 v4 f-450
master firmware
mission mode: no issue
rtl: not disarm on land mode
log:
https://review.px4.io/plot_app?log=2f5469ab-ab04-4d7e-896e-11808f94a2df

tested on pixhawk v4pro f-450
master firmware
mission mode: no issue
rtl: not disarm on land mode
log:
https://review.px4.io/plot_app?log=2f440dd7-657f-4d11-aa3c-54474af3e352

julianoes · 2019-11-05T09:39:44Z

Ok, I'll merge this given the land detection issues are unrelated.

julianoes requested review from dagar, potaito and RomanBapst October 28, 2019 10:04

julianoes added the bug label Oct 28, 2019

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

julianoes added this to In progress in Commander / Safety via automation Oct 28, 2019

julianoes added the [ARCHIVED] flight-testing (Use "Testing needed") label Oct 28, 2019

julianoes force-pushed the fix-battery-failsafe-nogps branch from f3505fe to e3783a3 Compare October 28, 2019 12:27

potaito reviewed Oct 28, 2019

View reviewed changes

src/modules/commander/state_machine_helper.cpp Show resolved Hide resolved

src/modules/commander/state_machine_helper.cpp Show resolved Hide resolved

commander: fix capitalization of mavlink messages

0b86374

potaito approved these changes Oct 28, 2019

View reviewed changes

Commander / Safety automation moved this from In progress to Reviewer approved Oct 28, 2019

Tony3dr added this to Ready for testing in Test Flights Oct 28, 2019

dagar added this to the Release v1.10.0 milestone Oct 28, 2019

dagar moved this from To Do to In progress in Release 1.11 Blockers Oct 30, 2019

RomanBapst requested changes Nov 1, 2019

View reviewed changes

Commander / Safety automation moved this from Reviewer approved to Review in progress Nov 1, 2019

julianoes mentioned this pull request Nov 1, 2019

mpu9250: fix mag spikes on fmu-v4pro #13353

Merged

julianoes merged commit 4ff4f5c into master Nov 5, 2019

Release 1.11 Blockers automation moved this from In progress to Done Nov 5, 2019

Test Flights automation moved this from Ready for testing to Done Nov 5, 2019

Commander / Safety automation moved this from Review in progress to Done Nov 5, 2019

julianoes deleted the fix-battery-failsafe-nogps branch November 5, 2019 09:40

julianoes mentioned this pull request Nov 5, 2019

Land detection not working in descend/land mode #13388

Closed

mrpollo mentioned this pull request Nov 7, 2019

v1.10.0 Release Notes + Changelog #13366

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

commander: fix battery failsafe without GPS #13294

commander: fix battery failsafe without GPS #13294

julianoes commented Oct 28, 2019 •

edited

Loading

julianoes commented Oct 28, 2019

LorenzMeier commented Oct 28, 2019

potaito left a comment

dannyfpv commented Oct 29, 2019 •

edited by Tony3dr

Loading

julianoes commented Oct 30, 2019

Tony3dr commented Oct 30, 2019

julianoes commented Oct 31, 2019

Tony3dr commented Oct 31, 2019

RomanBapst Nov 1, 2019

julianoes Nov 1, 2019 •

edited

Loading

dannyfpv commented Nov 2, 2019 •

edited by Tony3dr

Loading

julianoes commented Nov 4, 2019

julianoes commented Nov 4, 2019

julianoes commented Nov 4, 2019

julianoes commented Nov 4, 2019

Tony3dr commented Nov 4, 2019

Tony3dr commented Nov 4, 2019

dannyfpv commented Nov 4, 2019 •

edited

Loading

julianoes commented Nov 5, 2019

commander: fix battery failsafe without GPS #13294

commander: fix battery failsafe without GPS #13294

Conversation

julianoes commented Oct 28, 2019 • edited Loading

julianoes commented Oct 28, 2019

LorenzMeier commented Oct 28, 2019

potaito left a comment

Choose a reason for hiding this comment

dannyfpv commented Oct 29, 2019 • edited by Tony3dr Loading

julianoes commented Oct 30, 2019

Tony3dr commented Oct 30, 2019

julianoes commented Oct 31, 2019

Tony3dr commented Oct 31, 2019

RomanBapst Nov 1, 2019

Choose a reason for hiding this comment

julianoes Nov 1, 2019 • edited Loading

Choose a reason for hiding this comment

dannyfpv commented Nov 2, 2019 • edited by Tony3dr Loading

julianoes commented Nov 4, 2019

julianoes commented Nov 4, 2019

julianoes commented Nov 4, 2019

julianoes commented Nov 4, 2019

Tony3dr commented Nov 4, 2019

Tony3dr commented Nov 4, 2019

dannyfpv commented Nov 4, 2019 • edited Loading

julianoes commented Nov 5, 2019

julianoes commented Oct 28, 2019 •

edited

Loading

dannyfpv commented Oct 29, 2019 •

edited by Tony3dr

Loading

julianoes Nov 1, 2019 •

edited

Loading

dannyfpv commented Nov 2, 2019 •

edited by Tony3dr

Loading

dannyfpv commented Nov 4, 2019 •

edited

Loading