Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ekf2 preflight: only check innovation of active height sources #19774

Merged
merged 1 commit into from Jun 7, 2022

Conversation

bresch
Copy link
Member

@bresch bresch commented Jun 7, 2022

fixes #19770

Describe problem solved by this pull request

In 639222d#diff-b98e0cba6406890a752c8c8deb36086c8ed27442210a00a7ac02e2692245460c , the innovation for all the potential aiding sources is published. The problem is that the preflight checker in EKF2 module assumed that only the active source was non-zero (which was correct before this commit).

Describe your solution

Only run the check on the active height source(s), similar to what we were already doing for the horizontal sources.

@bresch bresch added the EKF2 label Jun 7, 2022
@bresch bresch requested review from dagar and MaEtUgR June 7, 2022 14:51
@bresch bresch self-assigned this Jun 7, 2022
Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

I don't see any error. This follows the same pattern as other innovation checks. I just had the question in the comments about the filter updates.

I quickly tried on my setup and the "height estimate unstable" blocking error is resolved. It keeps warning me that I don't have a GPS fix but as discussed this is actually useful and not blocking flight.
image

_preflt_checker.setUsingBaroHgtAiding(_ekf.control_status_flags().baro_hgt);
_preflt_checker.setUsingGpsHgtAiding(_ekf.control_status_flags().gps_hgt);
_preflt_checker.setUsingRngHgtAiding(_ekf.control_status_flags().rng_hgt);
_preflt_checker.setUsingEvHgtAiding(_ekf.control_status_flags().ev_hgt);
Copy link
Member

Choose a reason for hiding this comment

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

Note: I had to look up what Ev means again. Maybe the naming could be made more readable 😅

bool has_failed = false;

if (_is_using_baro_hgt_aiding) {
const float baro_hgt_innov_lpf = _filter_baro_hgt_innov.update(innov.baro_vpos, alpha, _hgt_innov_spike_lim);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the innovation low-pass filter always be updated and reset if it's not updated?
As far as I can see it's only reset up on landing but as we found out with this bug accepting the altitude estimate (and allowing Altitude mode) is dependent on the result of this check. I assume also in air.

@dagar
Copy link
Member

dagar commented Jun 7, 2022

We should come back to this later and find a way to do it without manually mirroring so much state.

@dagar dagar merged commit 4f10917 into master Jun 7, 2022
@dagar dagar deleted the pr-preflt-check-active-aiding-only branch June 7, 2022 15:44
@ryanjAA
Copy link
Contributor

ryanjAA commented Jun 8, 2022

Im able to arm as well but I do get GPS Disabled in QGC Daily.

No other changes to the code (custom airframes only on this test).

EDIT: I should note that when it goes back to not having a stable enough GPS (preflight fail, etc) it says AHRS disabled.

Screen Shot 2022-06-08 at 2 22 02 PM

@ryanjAA
Copy link
Contributor

ryanjAA commented Jun 9, 2022

Did a couple of flights. Still had the GPS disabled message but after GPS had enough sats and what not, it didn't give the fail message (but still the gps disabled stayed).

Strangely no logging was recorded even though it says the log was started (see attached). Have the tlog but that's all. Not sure if a one off but did several flights. All the same
Screen Shot 2022-06-08 at 8 09 03 PM
.

@vincentpoont2
Copy link
Contributor

Flight was fine in position mode, but just like Ryan. GPS shows "disabled" in QGC sensor status.
@dagar @bresch is this just a display error on QGC?
image

https://review.px4.io/plot_app?log=5cc286d1-91b7-4662-abcd-f623b50037d8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preflight Fail: height estimate not stable
5 participants