Skip to content

MAGFit: add GSF and GPS yaw attitude sources - #372

Open
j-w9 wants to merge 3 commits into
ArduPilot:mainfrom
j-w9:magfit-gsf-gps-yaw
Open

j-w9 wants to merge 3 commits into
ArduPilot:mainfrom
j-w9:magfit-gsf-gps-yaw

Conversation

@j-w9

@j-w9 j-w9 commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

"Option to use GSF and GPS yaw as truth" from #81.

New attitude sources: EKF3 roll and pitch with yaw from the GSF yaw estimator (XKY0.YC), or from any GPS instance that logs yaw (GPS.Yaw, which is logged as 0 when there is none). Neither uses the compass.

  • GSF samples are used when YCS is under the EKF's own reset threshold, GSF_YAW_ACCURACY_THRESHOLD_DEG (15 deg). YCS is logged as sqrt(degrees(variance)), so the threshold is converted to match (about 1.98).
  • The new attitude is built at the EKF quaternion times with interpolated yaw. Samples with more than 1 s between valid yaw samples are skipped.
  • These sources only cover part of the flight, so selecting one limits the analysis time to that range rather than changing the fit code.
  • The sources only appear when the log has valid data, and the default selection is unchanged. Default fits are identical to main on four logs.

Tested on a SITL flight with an NMEA GPS emitting HDT (GPS1_TYPE 5, SIM_GPS1_TYPE 5, SIM_GPS1_HDG 1), flying a square and full yaw turns. Mean yaw difference from ATT: GSF 5.6 deg, GPS 2.2 deg. Offsets with EKF / GSF / GPS yaw: (3.0, 16.9, -17.7) / (4.2, 14.9, -17.6) / (3.1, 14.3, -17.5).

Attitude sources

@tridge tridge added the AIReview label Nov 9, 2026
@AP-Review

AP-Review commented Nov 9, 2026 •

Copy link
Copy Markdown

Deprecated — see below for the updated review.

Previous review (2026-11-09)

Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting.
Verdict: REQUEST CHANGES

Reviewed at head 9c7066da2c.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/webtools/372/1.html#prWebTools-372

The new GSF and GPS-yaw attitude sources look right in themselves, but samples inside yaw gaps, or outside the source's time range, still reach the compass fit and can produce wrong offsets.

Blocking

  1. MAGFit/magfit.js:1747: yaw gaps are not excluded from the fit. replace_yaw drops these samples, but select_body_frame_attitude slerps across the hole onto every MAG sample and fit() uses the whole TimeStart..TimeEnd slice, so samples in a gap get a made-up roll, pitch and yaw. This happens with multi-flight logs, GSF threshold dropouts and GPS-yaw outages. Carry a validity mask or gap intervals with the source and exclude invalid MAG samples from the bins, errors and fit.

Should fix

  • MAGFit/magfit.js:1789: the source time range is only applied when the radio button changes. Editing TimeStart/TimeEnd, zooming the flight plot or double-click autorange widens it again, and the fit then uses held endpoint attitudes. The mask above fixes this too; otherwise clamp in calculate().

The duplicate moving-baseline GPS yaw source concern was dropped: the instances have separate timestamps and the extra option is harmless.

GSF and GPS yaw sources can have gaps and only cover part of the log.
Compass samples in a gap or outside the source's time are now given zero
weight in the fit, error and orientation check, rather than using an
attitude interpolated across the gap. Other attitude sources have no
gaps and are unchanged.
@AP-Review

Copy link
Copy Markdown

Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting.
Verdict: COMMENT

Reviewed at head e19a75669a.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/webtools/372/2.html#prWebTools-372

The new commit fixes the fit itself; only a display issue and a comment wording nit are left.

Since the last review at 9c7066d, compass samples in yaw gaps or outside the attitude source's time are left out of the fit, and a time range with no valid samples is refused. That fixes both earlier findings.

Low

  • MAGFit/magfit.js:1320: the new validity mask only feeds the fit weights. redraw() still plots error and yaw change for invalid samples from slerped or held attitude, so the error plot shows large made-up errors across yaw gaps. Set the plotted values to null where MAG_Data[i].valid is false, so Plotly leaves a gap.
Minor (1)
  • MAGFit/magfit.js:1225: the comment says "any compass" but have_valid_samples needs every masked compass to have valid samples. The code is right; reword the comment to "every".

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.

3 participants