Skip to content

AnalyticTune: fix ESLint problems - #346

Open
j-w9 wants to merge 6 commits into
ArduPilot:mainfrom
j-w9:lint-AnalyticTune
Open

j-w9 wants to merge 6 commits into
ArduPilot:mainfrom
j-w9:lint-AnalyticTune

Conversation

@j-w9

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

Copy link
Copy Markdown
Contributor

Fixes ESLint problems in AnalyticTune and removes the fixed entries from eslint-suppressions.json. None of the changes alter behaviour.

  • Drop repeated var declarations. The variable is already declared with var in the same function, so the repeat only assigns.
  • Declare variables that were implicitly global. Each was assigned without let/var, creating a global, but is only used within the function (or loop) that assigns it, and is not used by any other script on the page, so let does not change behaviour.
  • Declare the page script's loop variable. It was an implicit global, used only inside its for...of loop.
  • Declare variables assigned in if/else branches. PID_H_TOT and TGT_FILT_H are set in both branches and read after them; H_att_nff_pred was the one destructured name missing from the var list above it. All were implicit globals used only in their function.
  • Remove variables that are assigned but never read. dbl and triple in the notch filters were implicit globals that nothing read; declaring them would only make them unused locals, so they are removed.

One commit per kind of fix; npm run lint passes.

j-w9 added 4 commits October 11, 2026 23:54
The variable is already declared with var in the same function, so the
repeat only assigns.
Each was assigned without let/var, creating a global, but is only used
within the function (or loop) that assigns it, and is not used by any
other script on the page, so let does not change behaviour.
It was an implicit global, used only inside its for...of loop.
PID_H_TOT and TGT_FILT_H are set in both branches and read after them;
H_att_nff_pred was the one destructured name missing from the var list
above it. All were implicit globals used only in their function.
@IamPete1

Copy link
Copy Markdown
Member

I think we should probably wait for #337 to go in.

@bnsgeyer

Copy link
Copy Markdown
Contributor

Yes that would be most appreciated.

@j-w9

j-w9 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

sounds good! can you drop a note here once merged so i can update accordingly if necessary?

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

This is a behaviour-neutral ESLint cleanup of AnalyticTune. The lint result and the base/head output equivalence both check out at this head, and only one small commit-message fix is suggested.

Should fix

  • Commit 573af9b: the message says it also drops temp_data in SysID's compensation loops, but the commit only touches AnalyticTune.js and SysID/SysID.js:727/:733 still assign it. Drop that sentence when you next rebase.

Note

The suggestion to hoist the rpm/ele_prefix declarations was withdrawn: it would not change behaviour and is style only.

j-w9 added 2 commits November 8, 2026 22:48
dbl and triple in the notch filter were implicit globals that nothing
read; declaring them only made them unused locals. Drop them and prune
their suppressions.
@j-w9
j-w9 force-pushed the lint-AnalyticTune branch from bd8dbb3 to a7c2699 Compare November 9, 2026 03:49
@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 a7c269955d.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/webtools/346/2.html#prWebTools-346

This is a behaviour-neutral ESLint cleanup of AnalyticTune. Lint and the base/head output comparisons check out at a7c2699, and nothing needs changing.
Since the last review, the force-push reworded d500b16 (formerly 573af9b) so it no longer mentions SysID's temp_data, which fixes the one open item. The tree is unchanged.

Note

The earlier rpm/ele_prefix hoisting suggestion stays withdrawn.

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.

5 participants