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: Reinstate saving of mag declination for use next start #10024

Merged
merged 2 commits into from
Jul 26, 2018

Conversation

priseborough
Copy link
Contributor

@priseborough priseborough commented Jul 25, 2018

Fixes #10023

The capability and parameter selected default behaviour to save the magnetic declination for the last location had been removed unintentionally by previous changes. This PR reinstates the behaviour.

Requires PX4/PX4-ECL#486 to be merged first.

Reported here

Test data / coverage

  1. Start QGC and connect autopilot via USB
  2. Load parameters and set EKF2_MAG_DECL to 0
  3. Repower board and wait for GPS checks to pass.
  4. Reload parameters in QGC and verify EKF2_MAG_DECL has been updated.
  5. Repower board with GPS disconnected and and check EKF2_MAG_DECL value has been saved. See screen shot below

screen shot 2018-07-25 at 11 23 35 am

This capability was removed unintentionally by previous changes.
Requires ecl version with updated declination accessor function
@dagar
Copy link
Member

dagar commented Jul 25, 2018

Any idea when (or why) this disappeared?

@dagar
Copy link
Member

dagar commented Jul 25, 2018

You can actually still use _params.mag_declination_deg directly. All you need to do is decide when to commit the parameter ekf2 side. The internal value of _mag_declination_deg is a reference to _params.mag_declination_deg.

@priseborough priseborough requested a review from dagar July 25, 2018 07:31
@priseborough
Copy link
Contributor Author

No idea when or why the functionality was lost.

I don't understand your second comment about using _params.mag_declination_deg directly.

@dagar
Copy link
Member

dagar commented Jul 25, 2018

It's not critical, but you wouldn't have to actually copy the declination back to ekf2_main, and ecl/EKF side you wouldn't need _mag_declination_to_save_deg.

In ekf2_main _mag_declination_deg is a ParamExtFloat for EKF2_MAG_DECL. The internal value of _mag_declination_deg is _params->mag_declination_deg.

@dagar
Copy link
Member

dagar commented Jul 25, 2018

We should also set EKF2_MAG_DECL volatile (parameters automatically changed vehicle side).

@priseborough
Copy link
Contributor Author

So you are saying we should modify ecl to write the value retrieved from the geo library directly to the parameter value rather than provide it via an accessor function?

I will give you a call to clarify the use of the volatile attribute.

@LorenzMeier
Copy link
Member

Let's merge as-is and follow up to this PR if needed.

@LorenzMeier
Copy link
Member

@priseborough ECL updated.

@LorenzMeier LorenzMeier merged commit 77751d4 into PX4:master Jul 26, 2018
@dagar dagar added this to the Release v1.8.1 milestone Jul 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants