Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Fix baro fail-over in GPS hgt mode #799

Merged
merged 3 commits into from
Apr 19, 2020
Merged

Fix baro fail-over in GPS hgt mode #799

merged 3 commits into from
Apr 19, 2020

Conversation

bresch
Copy link
Member

@bresch bresch commented Apr 16, 2020

When the primary height source is GPS but the GPS data times out, EKF2 switches to baro and makes a reset. However, the _hgt_sensor_offset was not set to 0 and the height reset creates an incorrect jump in the state.

partially fixing PX4/PX4-Autopilot#14677

@bresch bresch self-assigned this Apr 16, 2020
priseborough
priseborough previously approved these changes Apr 16, 2020
Copy link
Collaborator

@priseborough priseborough left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look like they should address the issue caused by unnecessary and incorrect application of _hgt_sensor_offset to baro height. We do need to get some test data for the GPS -> baro -> GPS height case. This should be possible in SITL by setting EKF2_HGT_MODE = 1 stopping GPS and then restarting it .

kamilritz
kamilritz previously approved these changes Apr 16, 2020
Copy link
Contributor

@kamilritz kamilritz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could try to add a test for this. Reproduce the issue in sitl. Convert the ulog to the sensor data format. Replay the data during the test just before the switch to baro. Switch to baro, continue replaying the data for some small amount. And check that the height is closed to the one before the switch.
The same data could then also be used for more extensive change_indication.

@bresch bresch dismissed stale reviews from kamilritz and priseborough via d9fb281 April 16, 2020 16:42
@bresch
Copy link
Member Author

bresch commented Apr 17, 2020

Reproduced in SITL:
2020-04-16_18-06-23_01_plot
Shows a jump of 7m

Replayed with this fix:
2020-04-17_10-20-56_01_plot

Jump of 3m only. We still have a small jump because it's a reset and the baro offset doesn't exactly compensate (it's a filtered value). But this should be all right, the reset counter increments, the delta_z is published and should be handled by the setpoint generator.
However, I found that we have a race condition in the handling logic and I'm trying to fix it.

For reference, this is the replay without the fix:
2020-04-17_10-23-21_01_plot

@bresch
Copy link
Member Author

bresch commented Apr 17, 2020

I even got a better one here after going in and out of range aid a few times:
(Original log: https://logs.px4.io/plot_app?log=12da16ef-b98c-47ca-85b5-fdd87aabd09f, note that I fixed the reset on the firmware side, this is why there is no thrust spike despite the height change of 400m)
2020-04-17_14-06-43_01_plot

Replayed with this PR:
2020-04-17_14-07-17_01_plot

@bresch bresch merged commit 30d69aa into master Apr 19, 2020
@bresch bresch deleted the pr-baro-switch branch April 19, 2020 10:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants