Skip to content

FilterReview: prompt to show estimated post-filter after recalculating filters - #375

Open
j-w9 wants to merge 2 commits into
ArduPilot:mainfrom
j-w9:filter-review-estimate-prompt
Open

j-w9 wants to merge 2 commits into
ArduPilot:mainfrom
j-w9:filter-review-estimate-prompt

Conversation

@j-w9

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

Copy link
Copy Markdown
Contributor

"Prompt to select estimated post filter if not shown and filter param change is done" from #10.

After "Recalculate filters", if estimates are available but no Estimated post-filter checkbox is ticked, a line under the buttons says so and offers "Show estimated". That ticks the estimate for each gyro/axis whose pre or post line is shown (or all, if none are). The line hides again once any estimate is visible, on reset, and when the estimates are already shown.

Only the Recalculate filters button triggers it (via a small recalculate_filters() wrapper), so loading a log or a param file doesn't show it. The Setup fieldset uses min-height so the line fits.

Checked on a batch log: hint after a notch change and recalculate, then all three axes shown after "Show estimated", and no hint on the next recalculate.

Estimate hint

@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 e50cb8fb16.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/webtools/375/1.html#prWebTools-375

The Show estimated hint works as intended for the Recalculate filters button; three small nonblocking follow-ups remain.

Suggestions

  • FilterReview/FilterReview.js:1883: only the Recalculate filters button updates the hint. Load Parameters (load_parameters() calls re_calc() directly and leaves Recalculate disabled) and the filter-version radios recompute the estimate without ever showing it. Route them through recalculate_filters() too.
Minor (2)
  • FilterReview/FilterReview.js:1850: load() enables PostEst for every sensor once any sensor has pre-filter data. With a pre-only gyro and a post-only primary gyro, Show estimated ticks only the empty post-only estimates and the hint disappears with nothing drawn. Skipping checkboxes whose batch has no FFT.H would fix it.
  • FilterReview/FilterReview.js:1878: show_estimated() calls update_hidden() per checkbox, so the fallback redraws the FFT plot up to 9 times. Set each trace's visibility, then redraw once, as the legend path does.

Show the hint after loading parameters or changing filter version too,
only offer estimates for gyros with pre-filter data, and redraw once
when showing them.
@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 6aa6b1d026.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/webtools/375/2.html#prWebTools-375

No remaining issues found at head 6aa6b1d: the Show estimated hint works for Recalculate filters, Load Parameters and the filter-version radios.

Since the last review (at e50cb8f), all three earlier suggestions are fixed. Load Parameters and the version radios now go through recalculate_filters(). Show estimated skips gyros without pre-filter data. The plot redraws once instead of once per checkbox.

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