[j-w9]

The Step Response tooltip says "Simulated PID response to a step input", but the plot is estimated from the logged target and actual rates, and windows where the target stays below 20 deg/s are skipped (PIDReview.js, TarMax < 20). A hover-only section gives an empty plot with no hint as to why.

• Tooltip now describes the estimate, the 20 deg/s requirement and the grey/mean lines (as in the Readme).
• A one-line note under the plot is shown when no set produced an estimate, hidden otherwise and on reset.

No change to the calculation. Checked on three logs: a ground run shows the note, flights show the step response and no note.

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

The new empty step-response note works for the normal case, but in a few situations it disagrees with the plot or gives the wrong guidance.

Should fix

• PIDReview/PIDReview.js:1211: after recalculating a calm range on the same axis, the note shows while the previous range's mean curve is still drawn. Also clear step_plot.data[plot_index+1].x/.y in the clear loop at line 1049.
• PIDReview/index.html:262: on the Rover PIDA speed axis (m/s) the threshold is effectively 20 m/s, so the 'below 20 deg/s, add sharp stick inputs' note and the tooltip are wrong there. Word them per axis units, or leave the stick advice out for PIDA.

Minor (3)

• PIDReview/PIDReview.js:1052: a note shown on one axis stays visible after switching to an axis with no FFT data (all batches shorter than the window, or after a window-size change), because redraw() returns before redraw_step(). Hide it in setup_FFT_data().
• PIDReview/index.html:262: 'stayed below 20 deg/s' is too strong when inputs sit at a batch edge (Hann-attenuated) or only in batches shorter than the window. Consider wording like 'not enough target rate above 20 deg/s'.
• PIDReview/PIDReview.js:1197: sets_with_steps counts batches, not sets as the comment says. Only > 0 is used, so a boolean or a reworded comment would do.

[AP-Review]

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

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

The step-response note now behaves correctly in the normal cases, and only one minor stale-note path is left.

Since the last review (at aed72ab), the stale mean curve, the Rover PIDA units and stick advice, the axis-switch hide, the over-strong wording and the counter are all fixed.

Minor (1)

• PIDReview/PIDReview.js:1055: stale note after a window-size change (still open). If the new window is longer than every batch, Calculate leaves no FFT data and redraw() returns before redraw_step(), so the old 'above 20 deg /s, sharp stick inputs' note stays visible. Hide it in clear_calculation() (PIDReview/PIDReview.js:507).