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

EKF3: allow earth-frame fields to be estimated with an origin but no GPS #25666

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

Williangalvani
Copy link
Contributor

@Williangalvani Williangalvani commented Nov 29, 2023

This is my attempt at fixing #18229

Here's a log using it:
00000228.zip
I am using a script to set the origin so we can use the earth field database.

I can see that XKF2 fields are all populated for magnetic field in body frame and earth frame.

image

I am not sure of what else to look at, though. magnetic field innovations seem bad, but at least they exist?
image

@rmackay9
Copy link
Contributor

rmackay9 commented Nov 30, 2023

Hi, if the issue is the earth magnetic field (aka WMM) is not being estimated then the EKF also shouldn't be trying to use the WWM to reject the compass, instead it should accept it I think.

As a side note, it's naughty that the EKF is using the GPS status at all when estimating the WWM. This presumably means the same problem could happen when using a non-GPS navigation source while also using a compass which is very possible (e.g. Beacons).

@lixiaowei123
Copy link

af113c5725f37d8b81e856b1ebd5435
I have been saying that there is a bug in the estimation of EKF3 acceleration velocity increment offset

@lixiaowei123
Copy link

@rmackay9 You can view all the latest version logs to see if this is sometimes very large, and normally it should not exceed 0.3

libraries/AP_NavEKF3/AP_NavEKF3_Measurements.cpp Outdated Show resolved Hide resolved
@rmackay9
Copy link
Contributor

rmackay9 commented Dec 5, 2023

As discussed on the dev call we should investigate why the EKF3 is unable to fuse the compass when it doesn't have a position estimate. It really should be able to although it won't be as accurate.

@Williangalvani Williangalvani added this to the Sub-4.5 milestone Feb 26, 2024
@Williangalvani Williangalvani force-pushed the allow_mag_estimation branch 2 times, most recently from f0b04d2 to 19a9ac5 Compare March 5, 2024 15:28
@Williangalvani Williangalvani force-pushed the allow_mag_estimation branch 3 times, most recently from 049a447 to 6418137 Compare June 27, 2024 03:32
@Williangalvani Williangalvani marked this pull request as ready for review June 27, 2024 03:35
if (!stateStruct.quat.is_zero()) {
alignMagStateDeclination();
const auto &compass = dal.compass();
if (compass.have_scale_factor(magSelectIndex) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

could be a common function with the code in readGpsData()

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe setEarthFieldFromLocation()

@rmackay9
Copy link
Contributor

rmackay9 commented Jul 1, 2024

We discussed on the July 2nd dev call and decided that we should move the shared code to a new function and then call it from within setOrigin and readGPSData.

@tridge tridge merged commit cb74ebb into ArduPilot:master Jul 3, 2024
92 checks passed
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.

6 participants