fix: spectrum analysis crash on long logs (Freq/PSD vs Throttle/RPM)

[haslinghuis]

fix: spectrum analysis crash on long logs (Freq/PSD vs Throttle/RPM)

Summary

Fixes #922 — "Freq. vs Throttle" / "Freq. vs RPM" (and the PSD variants) threw
TypeError: Cannot read properties of undefined (reading '0') and failed to open for
long (e.g. 15-minute) logs, while plain Frequency and v3.6.0 worked.

Root cause: the spectrum sample buffers in graph_spectrum_calc.js were sized from a
_blackBoxRate estimate ((MAX_ANALYSER_LENGTH / 1e6) * _blackBoxRate), but the real
frame count comes from an index-based chunk range that is decoupled from that rate. On a
long log the actual frame count exceeds the estimate, so the per-field vsValues buffers
overflow. The out-of-range reads in the min/max pass returned undefined → NaN, which
poisoned minValue/maxValue; the existing minValue > maxValue guard does not catch
NaN, so the bin index became Math.round(NaN) = NaN and matrixFftOutput[NaN][0] threw.

Changes (src/graph_spectrum_calc.js)

• Size buffers from the real frame count (sum of chunk.frames.length) instead of the
  rate estimate, in _getFlightSamplesFreqVsX and _getFlightSamplesFreq. This removes
  the overflow at its source. _getFlightSamplesFreq uses Math.max(frameCount, fftBufferSize)
  so the MIN_SPECTRUM_SAMPLES_COUNT (2048) FFT floor and zero-padding are preserved for
  short logs.
• NaN-aware range guard: the degenerate-range fallback now also rejects non-finite
  bounds and the equal-bounds case (!Number.isFinite(...) || minValue >= maxValue),
  falling back to a safe 0..100 range. This also covers a constant-VS log
  (maxValue === minValue) that would otherwise divide by zero.
• Clamp the VS bin index to [0, NUM_VS_BINS - 1] in _dataLoadFrequencyVsX and
  _dataLoadPowerSpectralDensityVsX as a second line of defense against an out-of-range
  matrix-row dereference.
• Empty-window guards: early return in dataLoadPSD when there are no samples
  (getNearPower2Value(0) → FFTComplex(0) would throw), and an empty-matrix fallback in
  _dataLoadPowerSpectralDensityVsX so fftOutput is never undefined.

Test plan

No automated test suite exists for this module (manual testing per the project setup).

• Open the log attached to Error constructing spectrum analysis (Freq. vs throttle & Freq. vs RPM) #922 (30-05-first-long-flight.zip); confirm Freq vs
  Throttle, Freq vs RPM, PSD vs Throttle and PSD vs RPM all render with no console error.
• Confirm plain Frequency and Power Spectral Density still render (no regression).
• Confirm short logs (< 5 min) render all spectrum types unchanged.
• Confirm a constant-throttle window falls back to a 0..100 axis without crashing.
• Confirm an empty/near-empty selected analyser range does not throw.
• npm run lint passes; prettier --check clean.

Notes

• No MSP / protocol / logged-field-definition changes — single-repo, client-side calculation fix.
• Out of scope (follow-up): _getFlightSamplesPidErrorVsSetpoint still uses the same
  rate-estimate sizing. It writes by index and slices to the sample count, so it cannot
  crash like the heat-map paths, but it shares the latent undersize and is worth a separate fix.

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Fixed graph calculation errors preventing crashes when analyzing flight data with edge-case sample ranges.
  • Improved handling of invalid range bounds in frequency analysis with automatic fallback to safe defaults.
  • Enhanced data buffer management to prevent data corruption and calculation errors in spectral analysis.

[coderabbitai]

Walkthrough

This PR fixes spectrum analysis failures (Freq. vs throttle, Freq. vs RPM) reported in issue #922 by adding guards for empty data, correcting FFT/sample buffer sizing to use actual frame counts, clamping VS bin indices before matrix indexing, and improving VS range validation to prevent undefined row access and NaN propagation.

Changes

Spectrum Analysis Bounds and Sizing Repairs

Layer / File(s)	Summary
Empty data and undefined return guards src/graph_spectrum_calc.js	dataLoadPSD now returns an empty, fully-shaped result when sample count is zero, and _dataLoadPowerSpectralDensityVsX initializes matrixPsdOutput even with no chunks, preventing undefined row dereferences.
FFT and sample buffer sizing from actual frame counts src/graph_spectrum_calc.js	_getFlightSamplesFreq and _getFlightSamplesFreqVsX now allocate buffers using real frameCount from selected chunks instead of _blackBoxRate estimates, preventing buffer truncation and downstream NaN poisoning.
VS bin index clamping for array safety src/graph_spectrum_calc.js	Both _dataLoadFrequencyVsX and _dataLoadPowerSpectralDensityVsX clamp computed VS bin indices to [0, NUM_VS_BINS-1] before matrix indexing, protecting against out-of-range access.
VS range bounds validation and fallback src/graph_spectrum_calc.js	_getFlightSamplesFreqVsX now rejects non-finite bounds and equal-bound degeneracy with warnings, falling back to safe 0..100 range instead of only handling minValue > maxValue.

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: fixing a spectrum analysis crash on long logs affecting Freq/PSD vs Throttle/RPM variants.
Linked Issues check	✅ Passed	All changes directly address issue #922: buffer sizing from real frame counts prevents overflow; NaN-aware guards prevent invalid indices; bin clamping and empty-window guards prevent undefined access.
Out of Scope Changes check	✅ Passed	All changes in src/graph_spectrum_calc.js are scoped to fixing the crash on long logs for Freq/PSD vs Throttle/RPM. The PR explicitly notes that _getFlightSamplesPidErrorVsSetpoint is out of scope for follow-up.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check	✅ Passed	The PR description is comprehensive, well-structured, and addresses all key aspects: a clear summary of the fix, root cause analysis, detailed changes, test plan with checkmarks, and relevant notes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sonarqubecloud]

Quality Gate failed

Failed conditions
21.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

[github-actions]

Preview URL: https://pr923.betaflight-blackbox.pages.dev

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/graph_spectrum_calc.js (1)

686-691: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Same rate-estimate undersizing exists here and can mis-size on long logs.

piderror/setpoint are sized from a _blackBoxRate estimate (300 * _blackBoxRate) rather than the actual frame count. On long logs where the real frame count exceeds the estimate, writes past the typed-array length are silently dropped while samplesCount keeps incrementing, so the returned count overshoots the sliced buffer length and the consumer loop reads undefined → NaN setpoints. This is the same class of bug just fixed in _getFlightSamplesFreq/_getFlightSamplesFreqVsX, and is noted as a follow-up in the PR description.

🛡️ Proposed fix to size from actual frame count

   const FIELD_SETPOINT_INDEX = this._flightLog.getMainFieldIndexByName(
     `setpoint[${axisIndex}]`,
   );

-  const piderror = new Int16Array(
-    (MAX_ANALYSER_LENGTH / (1000 * 1000)) * this._blackBoxRate,
-  );
-  const setpoint = new Int16Array(
-    (MAX_ANALYSER_LENGTH / (1000 * 1000)) * this._blackBoxRate,
-  );
+  let frameCount = 0;
+  for (const chunk of allChunks) {
+    frameCount += chunk.frames.length;
+  }
+  const piderror = new Int16Array(frameCount);
+  const setpoint = new Int16Array(frameCount);

Want me to open a follow-up issue to track this PID-error sizing fix?

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/graph_spectrum_calc.js` around lines 686 - 691, The piderror and setpoint
typed arrays are sized from the _blackBoxRate estimate which can undersize them;
instead size them from the actual frame count used when populating samples (use
the same actual frame/sample count variable used elsewhere — e.g. samplesCount,
totalFrames or frameCount — rather than (MAX_ANALYSER_LENGTH / (1000 * 1000)) *
this._blackBoxRate). Update the allocation for piderror and setpoint to use that
actual frame count (or Math.min(actualFrameCount, MAX_ANALYSER_LENGTH) as a
safety cap) and ensure any returned count is clamped to the arrays' length so
writes cannot silently drop and consumers never read beyond the buffer.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/graph_spectrum_calc.js`:
- Around line 686-691: The piderror and setpoint typed arrays are sized from the
_blackBoxRate estimate which can undersize them; instead size them from the
actual frame count used when populating samples (use the same actual
frame/sample count variable used elsewhere — e.g. samplesCount, totalFrames or
frameCount — rather than (MAX_ANALYSER_LENGTH / (1000 * 1000)) *
this._blackBoxRate). Update the allocation for piderror and setpoint to use that
actual frame count (or Math.min(actualFrameCount, MAX_ANALYSER_LENGTH) as a
safety cap) and ensure any returned count is clamped to the arrays' length so
writes cannot silently drop and consumers never read beyond the buffer.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f2c1226f-5b41-4a57-b335-76ba57624c09

📥 Commits

Reviewing files that changed from the base of the PR and between b3307e5 and b056d2f.

📒 Files selected for processing (1)

• src/graph_spectrum_calc.js