Skip to content

refactor: extract hardcoded plot-math literals into named constants - #172

Merged
nerdCopter merged 1 commit into
masterfrom
refactor/extract-plot-math-constants
Sep 2, 2026
Merged

refactor: extract hardcoded plot-math literals into named constants#172
nerdCopter merged 1 commit into
masterfrom
refactor/extract-plot-math-constants

Conversation

@nerdCopter

@nerdCopter nerdCopter commented Sep 2, 2026

Copy link
Copy Markdown
Owner

AI Generated pull-request

Extracts hardcoded numeric literals flagged by CodeRabbit on PR#166 (pre-existing on master, relocated but not introduced by that PR) into named constants per AGENTS.md's constants policy.

Constants added (src/constants.rs)

  • NYQUIST_DIVISOR (2.0) — shared Nyquist-divisor pattern in plot_bode.rs and plot_motor_spectrums.rs
  • MIN_PLOT_FREQUENCY_HZ (1.0) — Bode plot frequency-axis floor
  • MOTOR_SPECTRUM_AXIS_ORIGIN (0.0) — shared X/Y axis origin in plot_motor_spectrums.rs
  • MOTOR_SPECTRUM_Y_LABEL_PRECISION_THRESHOLD (5.0) — label-precision threshold in plot_motor_spectrums.rs, distinct from MOTOR_SPECTRUM_Y_AXIS_MAX
  • STFT_OVERLAP_COMPLEMENT_BASE (1.0) — overlap-complement base in plot_psd_db_heatmap.rs

Verification

  • cargo clippy --all-targets --all-features -- -D warnings: clean
  • cargo fmt --all: applied
  • cargo test --verbose: 83 passed, 0 failed
  • cargo build --release: succeeds
  • Output parity: built master (bc439f0) and this branch, ran both with --extended --bode --butterworth --estimate-optimal-p against 4 flight logs (BTFL, EmuFlight, LUX, HELIO), md5sum-compared all 62 generated PNG/markdown outputs — byte-identical.

Closes #167

Summary by CodeRabbit

  • Improvements
    • Updated Bode, motor spectrum, and spectrogram visualizations with centralized frequency, axis, and overlap settings.
    • Maintained consistent plot boundaries, including minimum frequency and Nyquist limits.
    • Standardized motor spectrum axis origins and label precision behavior.

Adds NYQUIST_DIVISOR, MIN_PLOT_FREQUENCY_HZ, MOTOR_SPECTRUM_AXIS_ORIGIN,
MOTOR_SPECTRUM_Y_LABEL_PRECISION_THRESHOLD, and STFT_OVERLAP_COMPLEMENT_BASE
to constants.rs, replacing hardcoded literals in plot_bode.rs,
plot_motor_spectrums.rs, and plot_psd_db_heatmap.rs. Pure naming refactor,
no logic change; output verified byte-identical to master via md5sum across
4 flight logs with --extended --bode --butterworth --estimate-optimal-p.

Closes #167
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: eeba7cc3-45f3-46e1-9c1f-cfc5e8d8b0c7

📥 Commits

Reviewing files that changed from the base of the PR and between bc439f0 and 08e1a50.

📒 Files selected for processing (4)
  • src/constants.rs
  • src/plot_functions/plot_bode.rs
  • src/plot_functions/plot_motor_spectrums.rs
  • src/plot_functions/plot_psd_db_heatmap.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (1)
All constants go in `src/constants.rs` — no hardcoded magic numbers in function code Axis indices (0=Roll, 1=Pitch, 2=Yaw) must use the `src/axis_names.rs` module with `AXIS_COUNT`, `AXIS_NAMES`, and `axis_name()` function instead of creati...

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • src/plot_functions/plot_motor_spectrums.rs
  • src/plot_functions/plot_bode.rs
  • src/constants.rs
  • src/plot_functions/plot_psd_db_heatmap.rs
🔇 Additional comments (4)
src/plot_functions/plot_psd_db_heatmap.rs (1)

8-9: LGTM!

Also applies to: 61-62

src/constants.rs (1)

67-68: LGTM!

Also applies to: 70-72, 103-103

src/plot_functions/plot_bode.rs (1)

17-18: LGTM!

Also applies to: 249-250

src/plot_functions/plot_motor_spectrums.rs (1)

9-10: LGTM!

Also applies to: 194-194, 249-256, 278-280


📝 Walkthrough

Walkthrough

The change extracts plot-math literals into public constants. Bode, motor spectrum, and STFT heatmap code now use named constants while preserving the existing numeric behavior.

Changes

Plot Math Constants

Layer / File(s) Summary
Frequency-axis constants and usage
src/constants.rs, src/plot_functions/plot_bode.rs, src/plot_functions/plot_motor_spectrums.rs
Adds shared constants for the minimum plot frequency and Nyquist divisor. Bode and motor spectrum calculations use these constants.
Motor spectrum axis formatting
src/plot_functions/plot_motor_spectrums.rs
Uses named constants for the motor spectrum axis origin and Y-axis label precision threshold.
STFT overlap calculation
src/constants.rs, src/plot_functions/plot_psd_db_heatmap.rs
Adds the STFT overlap-complement base constant and uses it in hop-size calculation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 08e1a

This change only names existing plot-math values without changing generated behavior; the validated outputs remain identical, so no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: extracting hardcoded plot-math literals into named constants.
Linked Issues check ✅ Passed The pull request satisfies issue #167. It replaces the specified Nyquist, minimum-frequency, axis-origin, label-precision, and STFT overlap-complement literals with named constants while preserving nu…
Out of Scope Changes check ✅ Passed The changes are limited to the constants policy refactor described in issue #167. No unrelated code or logic changes are present.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files.
Full details: Linked Issues check

Explanation

The pull request satisfies issue #167. It replaces the specified Nyquist, minimum-frequency, axis-origin, label-precision, and STFT overlap-complement literals with named constants while preserving numeric behavior.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/extract-plot-math-constants

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nerdCopter
nerdCopter marked this pull request as ready for review September 2, 2026 16:47
@nerdCopter

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nerdCopter

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nerdCopter

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nerdCopter
nerdCopter merged commit e47ffe3 into master Sep 2, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: extract hardcoded plot-math literals into named constants

1 participant