Skip to content

FilterReview: warn if raw IMU data is missing for much of the analysis time - #376

Open
j-w9 wants to merge 3 commits into
ArduPilot:mainfrom
j-w9:filter-review-missing-data
Open

j-w9 wants to merge 3 commits into
ArduPilot:mainfrom
j-w9:filter-review-missing-data

Conversation

@j-w9

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

Copy link
Copy Markdown
Contributor

"Warn if lots of missing data" from #10.

For raw IMU logs, each gyro's info now shows "Missing data: N% of analysis time" when more than 10% of the analysis time has no usable data. Coverage comes from the batches the raw loader already splits at gaps (so dropped chunks of fewer than 64 samples also count as missing), clipped to the analysis time and to that gyro's data range. It updates on load and on every Calculate.

Batch logs are not checked. The batch sampler waits and retries when the log buffer is full rather than dropping data (BatchSampler.cpp, "maybe later"), so gaps there aren't lost data.

The 10% threshold is a guess (missing_data_warn_fraction), happy to change it.

Tested with a SITL raw IMU log, and a copy of it with 20 s of GYR removed mid hover to stand in for dropped messages. The original shows no warning. The copy shows 26% for the default analysis time and 37% for the 56 to 110 s hover (20 s of 54). No warning on batch logs.

Missing data warning

@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: COMMENT

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

This adds a useful missing-data warning, and interior gaps are reported correctly, but gaps at the edges of the analysis window are never counted.

Should fix

  • FilterReview/FilterReview.js:300: get_missing_fraction clips the window to this IMU's own first/last batch. An IMU that stops or starts partway through the window, or has no data in it at all, shows 0% and the warning stays hidden (multi-IMU or pre+post raw logs). Clip to Gyro_batch.start_time / Gyro_batch.end_time instead (the union over all instances, already set by load_from_raw_log) and drop first/last. In testing this gives 50%/90%/100% for those cases and still 0% for continuous data.

Short batches that the FFT skips are counted as covered, but that is existing FFT behaviour, not missing data, so it is not raised as an issue.

Clip to the time covered by all IMUs rather than this IMU's own data, so
an IMU that starts late, stops early or has no data in the window is
counted as missing.
@AP-Review

Copy link
Copy Markdown

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

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

This PR now looks good to merge: the missing-data warning is correct for interior gaps and for gaps at the edges of the analysis window. Since the last review, 34bde32 clips to the shared Gyro_batch.start_time/end_time, which fixes the edge-of-window false negative (FilterReview/FilterReview.js:299). An IMU that starts late, stops early or has no data in the window now reports 50%/100% instead of 0%, and continuous data still shows 0%.

The earlier note about short batches the FFT skips still stands as a non-issue.

Not fully reviewed: tested only with synthetic gyro data, not with a real raw-IMU .bin log loaded in a browser.

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