altitude mode not failing, when all height sources are missing

[jbeyerstedt]

When flying in Altitude mode and all height sources failed (GPS and Baro), PX4 does not switch to stabilized mode but stays in altitude mode, which is obviously not possible to achieve any more. (Only tested in jmavsim STIL)

To Reproduce
Steps to reproduce the behavior:

1. Start jmavsim SITL simulation with QGroundControl (with on screen RC inputs enabled or using a RC via USB/ Serial)
2. type commander takeoff in the PX4 shell
3. the vehicle takes off and holds position
4. change to position mode in QGC
5. type gpssim stop in the PX4 shell
6. PX4 switches from position mode to altitude
7. stop/ disable the barometer (barosim stop in the PX4 shell)
8. PX4 stays in altitude mode
9. (I switched to manual to be able to land the vehicle and then disarmed/ emergency stopped in QGC)

The issue is the same, if the barometer fails first (then PX4 will do a failover to GPS height) and GPS is turned off second.

Expected behavior
PX4 should switch from altitude to stabilized or at least print a warning in QGC.

Log Files and Screenshots
https://logs.px4.io/plot_app?log=0d53ff6b-2cb8-4955-8940-05f1bb611fff
(I still had a patch running, that disables the magnetometer, so there is no mag data. But this does not affect the issue.)

Additional context
Here is my patch which adds a stop command to the barosim:

diff --git a/src/modules/simulator/barosim/baro.cpp b/src/modules/simulator/barosim/baro.cpp
index 86a370fbac..6d263a39b9 100644
--- a/src/modules/simulator/barosim/baro.cpp
+++ b/src/modules/simulator/barosim/baro.cpp
@@ -308,7 +308,7 @@ info()
static void
usage()
{
- PX4_WARN("missing command: try 'start', 'info'");
+ PX4_WARN("missing command: try 'start', 'stop', 'info'");
}

}; // namespace barosim
@@ -332,6 +332,16 @@ barosim_main(int argc, char *argv[])
ret = barosim::start();
}

+ else if (!strcmp(verb, "stop")) {
+ if (g_barosim != nullptr) {
+ delete g_barosim;
+ g_barosim = nullptr;
+ } else {
+ PX4_WARN("already stopped.");
+ }
+ ret = 0;
+ }
+
/*
* Print driver information.
*/

[mhkabir]

Looks like we need to handle altitude loss failsafes (currently not implemented iirc.)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[julianoes]

Thanks, this is something to consider when we add testing for these failsafes.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

[jbeyerstedt]

I think, this is still an issue. (Looking at you, stale bot.)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

[jbeyerstedt]

Should this still be left open? Unfortunately I don't have time to fix this myself currently. And I don't really know how the whole failsafe system is supposed to work anyway. (But if you want to write tests for this, there should be a specification at some time 😉)

[julianoes]

It sounds like it's a bug, so leave it open.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

[bresch]

It stays in altitude mode because the z_valid and vz_valid flags are always true when flying. I'll reworks those flags this year