[j-w9]

"Settling time read out. also other stats overshoot ect." from #92.

A table under the Step Response title gives, for the mean response of each test: rise time (10 to 90%), peak and peak time, overshoot, settling time (5% band) and final value.

• The last 0.1 s of the estimate is ignored, because it turns up at the end (visible in the screenshot). Stats use 0 to 400 ms, and the final value is the mean from 200 to 400 ms.
• If the final value is outside 0.5 to 1.5, only the final value is shown, as the other numbers would be misleading. On one of my logs the mean settles at 0.21.
• Overshoot is left blank if the peak is inside the final value window, i.e. the response is still rising. Settling shows "> 400 ms" if it hasn't settled by then.
• Stats are taken once per test after the batch loop. The mean plot is updated on each batch, so only the final mean is used.

The calculation matches an independent Python version on three analytic second order responses (damping 0.3/0.5/0.8). For damping 0.5 it gives 16.4% overshoot with the peak at 90 ms, against theory's 16.3% at 90.7 ms. The thresholds (step_stats_*) are easy to change if you'd prefer other definitions. No change to the step estimate itself.

[AP-Review]

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

The step-response stats table works and gives correct values for the default 400 Hz / 512-sample case, but there are two edge cases where it shows misleading numbers.

Should fix

• PIDReview/PIDReview.js:1039: when the estimate is shorter than 0.4 s (FFT window 128 at 400 Hz, 256 at 1 kHz, or the default 512 at 2 kHz fast-rate logging), len falls back to response.length. The final value then uses the end-of-estimate region the comment above says to avoid, an unsettled response shows as > 400 ms, and the note still says 200-400 ms. Return early (or mark the row unreliable) when end_index < 0.
• PIDReview/PIDReview.js:1336: switching to an axis that has data but no complete FFT window leaves the previous axis's table visible, because redraw() returns on PID.sets.FFT == null before redraw_step(). Call show_step_stats([]) from setup_FFT_data()/setup_axis(), or before that early return.

Minor (1)

• PIDReview/PIDReview.js:1325: rows for tests unchecked with the per-test set_selection_N boxes stay in the table, because update_hidden() only redraws the plot. Skip unchecked sets in redraw_step() and refresh the table from update_hidden().

[AP-Review]

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

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

The step-response stats table now looks good to merge at head 9eae3fe, with no open findings.

All three earlier findings are fixed. Short estimates (under 0.4 s) no longer produce a row, switching to an axis without an FFT clears the table, and unticked tests drop out of the table.

Two new concerns were checked and dropped as pre-existing behaviour rather than this PR. After a Scale or Calculate redraw, unticked mean lines come back on the plot, because redraw_step() forces them visible at PIDReview/PIDReview.js:1323. A failed recalculation leaves the step plot stale, and the table matches that stale plot.