Skip to content

Added power spectral density charts #827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

demvlad
Copy link
Contributor

@demvlad demvlad commented May 10, 2025

The continue of #826 PRs
The power spectral density (PSD) charts with Throttle and RPM. The PSD values units is dBm/Hz. The logorithm scaling can show the all values range at the chart.
PSD_menu
The PSD charts (unfiltered and filtered). The color scaling is as -40dB - Black color, +10dB - White color
psd_heatmap
psd_heatmap_filtered

The casual spectrum charts (default scaling):
spectrum_heatmap
spectrum_heatmap_filtered

Will good to show the PSD value in the mouse position by using Shift key

Summary by CodeRabbit

  • New Features

    • Added three new spectrum type options: Power Spectral Density (PSD), PSD vs Throttle, and PSD vs RPM.
    • Enabled visualization of PSD data with accurate scaling, labels, and interactive mouse-over details.
    • Enhanced spectrum selection dropdown to include the new PSD options for easier access.
  • Style

    • Updated graph layouts, axis units, and color scaling to support the new PSD spectrum types.
    • Improved grid lines and margin adjustments for consistent display of PSD visualizations.

Copy link

coderabbitai bot commented May 10, 2025

"""

Walkthrough

Three new power spectral density (PSD) analysis options are integrated into the spectrum analysis UI and backend. PSD, PSD vs Throttle, and PSD vs RPM are now selectable, with corresponding data loading, calculation, and plotting logic added. The UI, data processing, and visualization layers are updated to support these new spectrum types.

Changes

Files/Paths Change Summary
index.html Reordered and expanded spectrum type dropdown options; added "PSD vs Throttle" and "PSD vs RPM"; repositioned existing options.
src/graph_spectrum.js Extended dataLoad method to support new PSD spectrum types by invoking PSD-specific data loaders with appropriate flags.
src/graph_spectrum_calc.js Added new private method for PSD vs variable calculation using Welch’s method; updated frequency vs throttle/rpm methods to accept PSD mode; enhanced sample retrieval with scaling option; fixed typos and improved variable naming.
src/graph_spectrum_plot.js Added new PSD spectrum types to enum; extended graph drawing methods to handle PSD and variants with adjusted heatmap scaling; enhanced axis labels, grid lines, mouse interaction, margin calculation; added helper methods for PSD data retrieval.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UI (index.html)
    participant Analyser (graph_spectrum.js)
    participant Calc (graph_spectrum_calc.js)
    participant Plot (graph_spectrum_plot.js)

    User->>UI (index.html): Selects "Power spectral density" (or PSD vs Throttle/RPM)
    UI->>Analyser: Triggers spectrum type change
    Analyser->>Calc: Calls dataLoadPSD or dataLoadFrequencyVsThrottle/Rpm(drawPSD=true)
    Calc->>Calc: Computes PSD (Welch's method) or PSD vs variable
    Calc-->>Analyser: Returns PSD data
    Analyser->>Plot: Requests graph redraw with PSD data
    Plot->>Plot: Renders PSD graph or heatmap
    Plot-->>User: Displays updated PSD visualization
Loading

Possibly related PRs

Suggested reviewers

  • haslinghuis

Poem

In the meadow of code where the signals play,
A rabbit hopped in with PSD today.
Throttle and RPM, now friends with the sound,
Spectral secrets in decibels abound.
With Welch’s method and heatmaps anew,
The spectrum’s a garden—enjoy the view!
🐇✨
"""

Tip

⚡️ Faster reviews with caching
  • CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings.

Enjoy the performance boost—your workflow just got faster.

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🔭 Outside diff range comments (1)
src/graph_spectrum_plot.js (1)

1095-1113: ⚠️ Potential issue

Division-by-zero guard missing when ticks === 0

_drawVerticalGridLines() now accepts a custom ticks argument, but Psd graphs may pass ticksCount = 0 when maxY === minY (e.g. very short logs).
HEIGHT / ticks and (ticks – i)/ticks will then throw Infinity/NaN, crashing the canvas draw.

Add a safety clamp:

- const ticks = Math.max(1, passedTicks ?? 5);
+ ticks = Math.max(1, ticks);

(and early-return if maxValue === minValue).

Also applies to: 1112-1120

🧹 Nitpick comments (7)
src/graph_spectrum_calc.js.bak (2)

611-628: Variable shadowing inside _fft_segmented loop
The inner for (let i = 0; i < pointsPerSegment / 2; i++) reuses the same identifier i as the outer loop variable, which can be confusing and error-prone when the code is modified. Rename the inner index (e.g. k) for clarity.


532-548: Unnecessary allocation of full-length Hanning window for scale calculation
window = Array(pointsPerSegment).fill(1) followed by _hanningWindow allocates a throw-away array every PSD call.
Compute the window once per distinct pointsPerSegment length and cache it, or derive the sum analytically (Welch window has known closed-form sums). This reduces memory churn during interactive zooming.

index.html (1)

461-463: Ensure option values align with JS spectrum constants and consider grouping
The new <option> entries—“Power spectral density” (value="4"), “PSD vs Throttle” (value="5"), and “PSD vs RPM” (value="6")—are correctly inserted after “Freq. vs RPM” (value="3") and before “Error vs Setpoint” (value="2"). Please verify that:

  1. The numeric values (4, 5, 6) exactly match the spectrum type constants/enums in your plotting and calculation code (e.g. in graph_spectrum.js and graph_spectrum_calc.js).
  2. All downstream logic (switch/case branches, saved settings, URL parameters, etc.) handles these new indices correctly.

Optionally, for maintainability, you could group the three PSD options into an <optgroup> (e.g., <optgroup label="PSD Charts">…</optgroup>) to visually separate them from the other spectrum types.

src/graph_spectrum_plot.js (2)

498-511: Hard-coded dB clamping will mis-represent quiet or very loud logs

The PSD heat-map maps every point into a fixed –40 … +10 dBm window.
Logs recorded with different gain, noise floor or throttle range can easily produce values outside this span, causing:

  • Severe clipping (everything below –40 dBm becomes identically dark),
  • Washed-out highlights when the true maximum is well below +10 dBm.

To preserve dynamic range, compute the window from the data itself (for instance, use the 1st and 99th percentile) or expose it as a user-adjustable setting.


1238-1241: Fullscreen gradient now renders fully opaque black

Changing both colour-stops to pure black removes the subtle fade that helped visually separate the plot area from imported curves and markers.
Was this intentional? If not, consider reverting the top stop to a slightly transparent value (e.g. rgba(0,0,0,0.9)) to keep depth cues.

src/graph_spectrum_calc.js (2)

599-680: Inner-loop variable shadowing and large-array hot-loops in _psd

The nested for (let i …) twice re-declares i, shadowing the outer loop.
While legal, it complicates reading and risks accidental misuse.

Also, combining JS numeric loops with millions of iterations (large logs × many segments) is CPU-heavy. Two quick wins:

  1. Rename the inner index (k/segIdx) for clarity.
  2. Pre-compute constants (2 * maxFrequency / dataCount, window coefficients) outside the hot-path.
for (let bin = 0; bin < dataCount; bin++) {
  let binSum = 0;
  for (let segIdx = 0; segIdx < segmentsCount; segIdx++) {
    let p = scale * fftOutput[segIdx][bin] ** 2;
    …

686-707: Potential memory blow-up in _fft_segmented

output.push(magnitudes) stores Float64Array(pointsPerSegment/2) for every segment; with long logs and small overlap this can reach hundreds of MB.
If only the averaged PSD is needed, accumulate in-place instead:

if (accum == null) accum = new Float64Array(pointsPerSegment/2);
for (let bin = 0; bin < accum.length; bin++) {
  accum[bin] += magnitudes[bin];
}
segmentsCount++;
…
return [accum, segmentsCount];

This divides memory by O(Nsegments).

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 20cec88 and 02b9527.

📒 Files selected for processing (5)
  • index.html (1 hunks)
  • src/graph_spectrum.js (2 hunks)
  • src/graph_spectrum_calc.js (7 hunks)
  • src/graph_spectrum_calc.js.bak (1 hunks)
  • src/graph_spectrum_plot.js (15 hunks)
🔇 Additional comments (1)
src/graph_spectrum_calc.js.bak (1)

338-346: Object.hasOwn limits browser compatibility
Object.hasOwn() is only available in modern runtimes (ES2022). For wider compatibility (e.g. Chromium‐based Edge < 106, older Electron), fall back to Object.prototype.hasOwnProperty.call(...).

-    if (Object.hasOwn(this._flightLog.getMainFieldIndexes(), fieldName)) {
+    if (Object.prototype.hasOwnProperty.call(this._flightLog.getMainFieldIndexes(), fieldName)) {

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/graph_spectrum_plot.js (2)

326-406: PSD graph implementation is well-structured.

The new _drawPowerSpectralDensityGraph method follows the same patterns as existing graph methods. I especially like the 10dB step alignment for the y-axis and proper handling of data ranges.

One minor observation: the comment on line 345 has a typo "Allign" instead of "Align".

-  // Allign y axis range by 10db
+  // Align y axis range by 10dB

482-510: PSD heat map handling works well, but could use a small enhancement.

The implementation properly handles PSD data with appropriate value clamping and normalization. The hard-coded min/max values (-40, 10 dBm) are used for scaling.

For better maintainability, consider extracting these magic numbers as named constants at the top of the file.

 const BLUR_FILTER_PIXEL = 1,
   DEFAULT_FONT_FACE = "Verdana, Arial, sans-serif",
   DEFAULT_MARK_LINE_WIDTH = 2,
+  PSD_MIN_DBM = -40,
+  PSD_MAX_DBM = 10,
   MARGIN = 10,
   MARGIN_BOTTOM = 10,
   MARGIN_LEFT = 25,

Then use these constants in the _drawHeatMap method:

 if (drawPSD) {
-  const min = -40, max = 10; //limit values dBm
+  const min = PSD_MIN_DBM, max = PSD_MAX_DBM; //limit values dBm
   valuePlot = Math.max(this._fftData.fftOutput[j][i], min);
   valuePlot = Math.min(valuePlot, max);
   valuePlot = Math.round((valuePlot - min) * 100 / (max - min));
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2a798be and a1fc461.

📒 Files selected for processing (1)
  • src/graph_spectrum_plot.js (15 hunks)
🔇 Additional comments (9)
src/graph_spectrum_plot.js (9)

20-22: Adding new spectrum types looks clean.

The three new enum entries for power spectral density visualization follow the existing naming pattern and sequential numbering, making integration seamless with the rest of the codebase.


174-188: Implementation of PSD visualization handlers looks good.

The new handlers for PSD-related spectrum types are well-integrated into the existing switch-case structure:

  • PSD vs Throttle/RPM reuse existing drawing logic with a new parameter
  • Power Spectral Density gets its own dedicated drawing method

This approach maintains a clean separation of concerns while minimizing code duplication.


408-408: Good parameter addition with backward compatibility.

Adding a default parameter drawPSD = false to the _drawFrequencyVsXGraph method maintains compatibility with existing code while extending functionality for PSD visualization.


455-479: Clean conditional handling for different spectrum types.

The nested if-else statements handling the different spectrum types are clear and logically structured. Good work extending the existing code to handle the new PSD spectrum types while maintaining the correct units for each type.


1095-1107: Enhanced vertical grid lines function with optional ticks parameter.

The _drawVerticalGridLines function has been improved to accept an optional ticks parameter with a default value of 5. This makes the function more flexible and reusable.


1466-1508: Mouse position handling for new spectrum types.

The mouse position drawing logic has been properly extended to handle the new PSD-related spectrum types. The correct unit label "dBm/Hz" is used for power spectral density.


1580-1586: Margin handling for consistent layout.

The _getActualMarginLeft method has been updated to handle the new PSD spectrum types with appropriate margin settings, ensuring a consistent layout across all visualization types.


1238-1240: Adjusted gradient opacity for fullscreen mode.

The background gradient opacity for fullscreen mode has been slightly adjusted, which should improve readability of the visualization.


972-972: Changed "Max motor noise" to "Max noise"

The label for the maximum noise frequency marker has been renamed from "Max motor noise" to "Max noise", which is more generic and appropriate for different data sources.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
src/graph_spectrum_calc.js (3)

698-702: Shadowing loop variable i reduces readability

A second for (let i = …) is nested inside another loop that already declares i.
Although let prevents functional errors, re-using the same variable name forces readers to track scopes mentally.

-  for (let i = 0; i < pointsPerSegment / 2; i++) {
-    const re = fftComplex[2 * i];
-    const im = fftComplex[2 * i + 1];
-    magnitudes[i] = Math.hypot(re, im);
-  }
+  for (let k = 0; k < pointsPerSegment / 2; k++) {
+    const re = fftComplex[2 * k];
+    const im = fftComplex[2 * k + 1];
+    magnitudes[k] = Math.hypot(re, im);
+  }

Renaming the inner index (e.g., to k) clarifies intent and avoids cognitive overhead.


608-625: Avoid re-creating the Hanning window array for every PSD call

Creating and windowing a fresh Array(pointsPerSegment) on every invocation is unnecessary and generates significant GC pressure for large analyses.
Compute the window once per pointsPerSegment value and cache it:

-    const window = Array(pointsPerSegment).fill(1);
-    this._hanningWindow(window, pointsPerSegment);
+    if (!this._hanningCache) { this._hanningCache = new Map(); }
+    let window = this._hanningCache.get(pointsPerSegment);
+    if (!window) {
+      window = Float64Array.from({length: pointsPerSegment}, () => 1);
+      this._hanningWindow(window, pointsPerSegment);
+      this._hanningCache.set(pointsPerSegment, window);
+    }

This micro-optimisation pays off when the viewer redraws repeatedly while users tweak zoom levels.


648-670: Quadratic loop inside PSD can be vectorised

for i (frequency bins) × for j (segments) gives O(N × S) complexity. For large logs this becomes the top CPU hotspot.

Consider accumulating the squared magnitudes using typed-array operations or for-loop unrolling, or delegate the heavy lifting to a WebWorker to keep the UI fluid.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a1fc461 and 6a5d9a5.

📒 Files selected for processing (1)
  • src/graph_spectrum_calc.js (7 hunks)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/graph_spectrum_calc.js (1)

112-120: ⚠️ Potential issue

Fix analyserZoomY handling and improve segment size scaling logic

The current implementation for calculating pointsPerSegment has several issues:

  1. It doesn't guard against invalid analyserZoomY values (0 or negative values would cause problems)
  2. The zoom scaling logic is unintuitive and produces abrupt jumps in resolution
  3. Current implementation may lead to unpredictable behavior when zooming out (analyserZoomY > 1)
-  let pointsPerSegment = 512;
-  const multiplier = Math.floor(1 / analyserZoomY); // 0. ... 10
-  if (multiplier == 0) {
-    pointsPerSegment = 256;
-  } else if (multiplier > 1) {
-    pointsPerSegment *= 2 ** Math.floor(multiplier / 2);
-  }
+  // Initialize with reasonable default and validate zoom factor
+  let pointsPerSegment = 512;
+  
+  if (analyserZoomY <= 0) {
+    console.warn("Invalid analyserZoomY value, using default");
+    analyserZoomY = 1;
+  }
+  
+  // Create smoother scaling between zoom levels:
+  // - When zooming in (analyserZoomY < 1), increase points for better resolution
+  // - When zooming out (analyserZoomY > 1), decrease points for faster updates
+  const MIN_SEGMENT = 256;
+  const MAX_SEGMENT = 4096;
+  pointsPerSegment = Math.round(Math.min(MAX_SEGMENT, 
+                                Math.max(MIN_SEGMENT, 512 / analyserZoomY)));
🧹 Nitpick comments (6)
src/graph_spectrum_calc.js (6)

233-236: Optimize memory allocation for the FFT matrix

The matrix initialization uses Math.floor(fftChunkLength / 2) to size the columns, which is a good improvement over the original approach but still an approximation.

   // Matrix where each row represents a bin of vs values, and the columns are amplitudes at frequencies
-  const matrixFftOutput = new Array(NUM_VS_BINS)
-        .fill(null)
-        .map(() => (new Float64Array(Math.floor(fftChunkLength / 2))).fill(-70));
+  // Wait to know the exact PSD output length before allocating
+  const psd = this._psd(new Float64Array(fftChunkLength), fftChunkLength, 0, 'density');
+  const psdLength = psd.psdOutput.length;
+  
+  const matrixFftOutput = new Array(NUM_VS_BINS)
+        .fill(null)
+        .map(() => (new Float64Array(psdLength)).fill(-70));

242-243: Consider reusing PSD calculation results

The code computes PSD then only uses it to extract psdLength and maxNoise, but doesn't reuse the actual PSD output, requiring recomputation for each chunk.

-    const psd = this._psd(fftInput, fftChunkLength, 0, 'density');
-    psdLength = psd.psdOutput.length;
-    maxNoise = Math.max(psd.max, maxNoise);
+    const psd = this._psd(fftInput, fftChunkLength, 0, 'density');
+    psdLength = psd.psdOutput.length;
+    maxNoise = Math.max(psd.max, maxNoise);
+    
+    // Store PSD output for later use to avoid recomputation
+    const psdOutput = psd.psdOutput;

603-631: Add documentation for the PSD scaling options

The scaling logic is complex and would benefit from more detailed documentation to explain the different options and their use cases.

 // Compute power scale coef
   let scale = 1;
+  // Scale options:
+  // - 'density': Scales to produce power spectral density (power per frequency unit)
+  // - 'spectrum': Scales to produce power spectrum (total power)
+  // 
+  // Window normalization:
+  // When applying a window function, appropriate normalization is needed to
+  // maintain correct amplitude scaling of the spectrum
   if (userSettings.analyserHanning) {
     const window = Array(pointsPerSegment).fill(1);
     this._hanningWindow(window, pointsPerSegment);

637-644: Document the early exit condition in _psd function

The code includes an early exit when segmentsCount === 0, but doesn't explain when this might occur, making it harder to understand and maintain.

-      // Early exit if no segments were processed
+      // Early exit if no segments were processed
+      // This can happen if:
+      // 1. The input samples array is empty
+      // 2. pointsPerSegment is larger than the samples length
+      // 3. The segmentation loop in _fft_segmented didn't produce any valid segments
       if (segmentsCount === 0) {

700-704: Use Math.sqrt(re*re + im*im) instead of Math.hypot

For better performance in this critical loop, consider using direct calculation instead of Math.hypot.

   for (let i = 0; i < pointsPerSegment / 2; i++) {
     const re = fftComplex[2 * i];
     const im = fftComplex[2 * i + 1];
-    magnitudes[i] = Math.hypot(re, im);
+    magnitudes[i] = Math.sqrt(re * re + im * im);
   }

663-667: Consider adding configurable noise floor

The hardcoded minimum average of 1e-7 (-70dB) could be made configurable to allow users to adjust the noise floor.

-    const min_avg = 1e-7; // limit min value for -70db
+    // Limit min value for noise floor (default -70dB)
+    const noiseFloorDb = userSettings.psdNoiseFloorDb || -70;
+    const min_avg = 10 ** (noiseFloorDb / 10);
     let avg = psdOutput[i] / segmentsCount;
     avg = Math.max(avg, min_avg);
     psdOutput[i] = 10 * Math.log10(avg);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a5d9a5 and eafa1bb.

📒 Files selected for processing (1)
  • src/graph_spectrum_calc.js (7 hunks)
🔇 Additional comments (1)
src/graph_spectrum_calc.js (1)

109-135: Good implementation of the PSD calculation function

The overall approach for PSD calculation using the Welch method is well-implemented, with appropriate handling of the data and return of relevant metadata.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
src/graph_spectrum_calc.js (2)

112-120: Guard against invalid analyserZoomY and improve scale mapping

The current logic for calculating pointsPerSegment has several issues:

  1. multiplier = Math.floor(1 / analyserZoomY) will throw if analyserZoomY is zero
  2. For negative zoom factors, the logic produces nonsensical segment sizes
  3. The step-wise doubling causes abrupt jumps in resolution

688-709: Add validation for pointsPerSegment in _fft_segmented

The function doesn't validate that pointsPerSegment is positive and non-zero, which could lead to division by zero or other unexpected behaviors.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eafa1bb and 80707ba.

📒 Files selected for processing (1)
  • src/graph_spectrum_calc.js (8 hunks)
🔇 Additional comments (5)
src/graph_spectrum_calc.js (5)

221-291: Good implementation of PSD vs X calculation

The new _dataLoadPowerSpectralDensityVsX method properly implements PSD calculation for varying throttle/RPM values. The optimized matrix size (line 235) fixes the memory waste issue identified in a previous review.


293-304: Clean API design for FFT/PSD selection

Good approach to maintain backward compatibility while adding PSD functionality. The boolean parameter provides a clean way to toggle between calculation methods without duplicating code.


387-409: Well-designed raw data access

Adding the scaled parameter allows getting raw data values when needed for PSD calculations while maintaining backward compatibility. This is a clean approach.


422-437: Consistent raw data access for vs X methods

The same scaled parameter approach is consistently applied here, maintaining code consistency. Good practice.


598-682: Thorough PSD implementation with proper handling of corner cases

The PSD implementation follows standard signal processing practices:

  1. Uses Welch method with overlapping segments
  2. Properly applies window functions and scaling
  3. Converts to dB scale with appropriate noise floor
  4. Handles special cases like empty segments

Consider adding some documentation on the specific parameters and their expected ranges, especially for the scaling parameter.

@demvlad
Copy link
Contributor Author

demvlad commented May 10, 2025

@haslinghuis
is possible to merge #826 to continue work in this PR?

@haslinghuis
Copy link
Member

@demvlad done - we can always revert :)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/graph_spectrum_plot.js (1)

482-510: Consider extracting PSD scaling values as constants.

The PSD min/max values (-40, 10) are hardcoded in the implementation. These values should be extracted as named constants at the top of the file for better maintainability.

 const BLUR_FILTER_PIXEL = 1,
   DEFAULT_FONT_FACE = "Verdana, Arial, sans-serif",
   DEFAULT_MARK_LINE_WIDTH = 2,
   MARGIN = 10,
   MARGIN_BOTTOM = 10,
   MARGIN_LEFT = 25,
   MARGIN_LEFT_FULLSCREEN = 35,
   MAX_SETPOINT_DEFAULT = 100,
   PID_ERROR_VERTICAL_CHUNK = 5,
+  PSD_MIN_DB = -40,
+  PSD_MAX_DB = 10,
   ZOOM_X_MAX = 5;

// Then use these constants in the _drawHeatMap method:
      if (drawPSD) {
-        const min = -40, max = 10; //limit values dBm
+        const min = PSD_MIN_DB, max = PSD_MAX_DB; //limit values dBm
         valuePlot = Math.max(this._fftData.fftOutput[j][i], min);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 01a0c98 and 7a59f8a.

📒 Files selected for processing (2)
  • src/graph_spectrum_calc.js (5 hunks)
  • src/graph_spectrum_plot.js (14 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/graph_spectrum_calc.js
🔇 Additional comments (7)
src/graph_spectrum_plot.js (7)

21-22: LGTM! New spectrum types added to the enum.

These new enum values appropriately extend the spectrum type options to include PSD vs Throttle and PSD vs RPM visualizations.


174-180: LGTM! Properly handling new spectrum types in the drawing logic.

The changes correctly extend the switch statement to handle the new PSD spectrum types, passing true to the drawing function to indicate PSD mode.


408-408: LGTM! Function parameter updated to support PSD drawing.

The optional drawPSD parameter with default value ensures backward compatibility.


1544-1554: LGTM! Mouse position indicator shows dBm/Hz values.

Good feature enhancement to display PSD values with correct units (dBm/Hz) when hovering over the chart.


1603-1610: LGTM! Margin logic updated for new spectrum types.

The _getActualMarginLeft function correctly includes the new spectrum types in determining appropriate margin settings.


455-468: LGTM! Vertical grid lines drawing handles PSD spectrum types.

The conditional statements are properly extended to handle the new spectrum types when drawing grid lines with appropriate units.


1477-1516: Mouse position handling updated correctly for PSD spectrum types.

The _drawMousePosition method has been appropriately updated to recognize the new spectrum types and correctly assigns the proper unit labels.

@demvlad
Copy link
Contributor Author

demvlad commented May 11, 2025

Added PSD values label at mouse position cursor by Shift key
PSD_marker

@demvlad
Copy link
Contributor Author

demvlad commented May 11, 2025

Added PSD values label on the mouse position cursor at the power spectral density chart.
PSD_marker2

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/graph_spectrum_plot.js (1)

531-540: Add error handling to getValueFromMatrixFFT

This new method retrieves matrix values but lacks validation for edge cases where indices could be out of bounds.

The previously suggested fix addresses this properly:

GraphSpectrumPlot.getValueFromMatrixFFT  = function(frequency, vsArgument) {
  const NUM_VS_BINS = 100;  // redefinition of value from graph_spectrum_calc.js module!
  const matrixFFT = this._fftData;
+  if (!matrixFFT || !matrixFFT.fftOutput || !matrixFFT.vsRange) {
+    return 0; // Return safe default if data is missing
+  }
  let vsArgumentIndex = Math.round(NUM_VS_BINS * (vsArgument - matrixFFT.vsRange.min) / (matrixFFT.vsRange.max - matrixFFT.vsRange.min));
  if (vsArgumentIndex === NUM_VS_BINS) {
    vsArgumentIndex = NUM_VS_BINS - 1;
  }
+  // Ensure vsArgumentIndex is within valid bounds
+  vsArgumentIndex = Math.max(0, Math.min(vsArgumentIndex, matrixFFT.fftOutput.length - 1));
  const freqIndex = Math.round(2 * frequency / matrixFFT.blackBoxRate * (matrixFFT.fftOutput[0].length - 1) );
+  // Ensure freqIndex is within valid bounds
+  const maxFreqIndex = matrixFFT.fftOutput[vsArgumentIndex]?.length - 1 || 0;
+  const safeFreqIndex = Math.max(0, Math.min(freqIndex, maxFreqIndex));
-  return matrixFFT.fftOutput[vsArgumentIndex][freqIndex];
+  return matrixFFT.fftOutput[vsArgumentIndex]?.[safeFreqIndex] ?? 0;
};
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7a59f8a and c128b2a.

📒 Files selected for processing (1)
  • src/graph_spectrum_plot.js (14 hunks)
🔇 Additional comments (8)
src/graph_spectrum_plot.js (8)

21-22: LGTM: Adding new spectrum types for PSD visualization

The enum extensions for PSD_VS_THROTTLE and PSD_VS_RPM are properly defined and follow the existing pattern.


174-180: LGTM: Correctly handling new spectrum types

The switch case handling for the new PSD types properly delegates to _drawFrequencyVsXGraph with the drawPSD flag set to true.


413-413: LGTM: Well-structured method parameter

Adding the optional drawPSD parameter with a default value of false maintains backward compatibility.


487-515: LGTM: Proper PSD visualization scaling

The conditional scaling for PSD values appropriately clamps the values between -40 and 10 dBm, normalizing them to the 0-100 scale for color intensity.


1482-1489: LGTM: Proper variable initialization

Initializing mouseFrequency and including the new PSD spectrum types in the conditional check is correct.


1512-1521: LGTM: Well-implemented PSD value display at cursor

The code correctly shows PSD values at the mouse cursor position when hovering over the power spectral density graph.


1560-1570: LGTM: Well-implemented matrix value display at cursor

The code correctly shows the FFT matrix value at the mouse cursor position when hovering over PSD vs Throttle or PSD vs RPM graphs.


1624-1625: LGTM: Consistent margin handling

The margin handling for the new PSD spectrum types follows the same pattern as their non-PSD counterparts.

Comment on lines 408 to 411
GraphSpectrumPlot.getPSDbyFreq = function(frequency) {
const freqIndex = Math.round(2 * frequency / this._fftData.blackBoxRate * (this._fftData.psdOutput.length - 1) );
return this._fftData.psdOutput[freqIndex];
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add bounds checking to getPSDbyFreq

This new method lacks validation for the calculated index and doesn't handle potential null/undefined values in the FFT data.

GraphSpectrumPlot.getPSDbyFreq = function(frequency) {
+  if (!this._fftData || !this._fftData.psdOutput || !this._fftData.psdOutput.length) {
+    return 0; // Return safe default if data is missing
+  }
  const freqIndex = Math.round(2 * frequency / this._fftData.blackBoxRate * (this._fftData.psdOutput.length - 1));
+  // Ensure freqIndex is within valid bounds
+  const safeIndex = Math.max(0, Math.min(freqIndex, this._fftData.psdOutput.length - 1));
-  return this._fftData.psdOutput[freqIndex];
+  return this._fftData.psdOutput[safeIndex];
};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
GraphSpectrumPlot.getPSDbyFreq = function(frequency) {
const freqIndex = Math.round(2 * frequency / this._fftData.blackBoxRate * (this._fftData.psdOutput.length - 1) );
return this._fftData.psdOutput[freqIndex];
};
GraphSpectrumPlot.getPSDbyFreq = function(frequency) {
if (!this._fftData || !this._fftData.psdOutput || !this._fftData.psdOutput.length) {
return 0; // Return safe default if data is missing
}
const freqIndex = Math.round(2 * frequency / this._fftData.blackBoxRate * (this._fftData.psdOutput.length - 1));
// Ensure freqIndex is within valid bounds
const safeIndex = Math.max(0, Math.min(freqIndex, this._fftData.psdOutput.length - 1));
return this._fftData.psdOutput[safeIndex];
};

@demvlad demvlad marked this pull request as draft May 12, 2025 16:16
@demvlad
Copy link
Contributor Author

demvlad commented May 12, 2025

Added PSD value to maximal noise PSD marker
PSD_marker

@demvlad demvlad marked this pull request as ready for review May 12, 2025 17:37
demvlad and others added 2 commits May 12, 2025 21:16
Co-authored-by: Mark Haslinghuis <mark@numloq.nl>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (3)
src/graph_spectrum_plot.js (3)

1574-1580: Fix indentation in drawing code.

The indentation of this block is inconsistent with the project standard.

          this._drawAxisLabel(
            canvasCtx,
            label,
            mouseX - 30,
            mouseY - 4,
-            "left",
+            "left"
          );

533-544: 🛠️ Refactor suggestion

Add error handling to getValueFromMatrixFFT.

This method lacks validation for the matrixFFT object and bounds checking for array indices.

 GraphSpectrumPlot.getValueFromMatrixFFT  = function(frequency, vsArgument) {
   const NUM_VS_BINS = 100;  // redefinition of value from graph_spectrum_calc.js module!
   const matrixFFT = this._fftData;
+  if (!matrixFFT || !matrixFFT.fftOutput || !matrixFFT.vsRange) {
+    return 0; // Return safe default if data is missing
+  }
   let vsArgumentIndex = Math.round(NUM_VS_BINS * (vsArgument - matrixFFT.vsRange.min) / (matrixFFT.vsRange.max - matrixFFT.vsRange.min));
   if (vsArgumentIndex === NUM_VS_BINS) {
     vsArgumentIndex = NUM_VS_BINS - 1;
   }
+  // Ensure vsArgumentIndex is within valid bounds
+  vsArgumentIndex = Math.max(0, Math.min(vsArgumentIndex, matrixFFT.fftOutput.length - 1));
   let freqIndex = Math.round(2 * frequency / matrixFFT.blackBoxRate * (matrixFFT.fftLength - 1));
   freqIndex = Math.max(freqIndex, 0);
   freqIndex = Math.min(freqIndex, matrixFFT.fftLength - 1);
-  return matrixFFT.fftOutput[vsArgumentIndex][freqIndex];
+  return matrixFFT.fftOutput[vsArgumentIndex]?.[freqIndex] ?? 0;
 };

408-413: 🛠️ Refactor suggestion

Add error handling to getPSDbyFreq.

The method is missing null/undefined checks for the FFT data.

 GraphSpectrumPlot.getPSDbyFreq  = function(frequency) {
+  if (!this._fftData || !this._fftData.psdOutput) {
+    return 0;
+  }
   let freqIndex = Math.round(2 * frequency / this._fftData.blackBoxRate * (this._fftData.psdOutput.length - 1) );
   freqIndex = Math.min(freqIndex, this._fftData.psdOutput.length - 1);
   freqIndex = Math.max(freqIndex, 0);
-  return this._fftData.psdOutput.length ? this._fftData.psdOutput[freqIndex] : 0;
+  return this._fftData.psdOutput.length ? this._fftData.psdOutput[freqIndex] : 0;
 };
🧹 Nitpick comments (1)
src/graph_spectrum_plot.js (1)

1571-1581: Use strict equality operator for consistency.

The comparison operators for spectrum types should use === instead of == for type safety and consistency with the rest of the code.

-        if (this._spectrumType == SPECTRUM_TYPE.PSD_VS_THROTTLE ||
-            this._spectrumType == SPECTRUM_TYPE.PSD_VS_RPM) {
+        if (this._spectrumType === SPECTRUM_TYPE.PSD_VS_THROTTLE ||
+            this._spectrumType === SPECTRUM_TYPE.PSD_VS_RPM) {
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 30b62b1 and 3877f84.

📒 Files selected for processing (2)
  • index.html (1 hunks)
  • src/graph_spectrum_plot.js (17 hunks)
✅ Files skipped from review due to trivial changes (1)
  • index.html
🔇 Additional comments (9)
src/graph_spectrum_plot.js (9)

18-22: Well-organized update to the SPECTRUM_TYPE enum.

The addition of new spectrum types is nicely organized with sequential values, maintaining the existing structure.


174-180: LGTM - Clear handling of new PSD spectrum types.

The switch cases for new PSD types correctly reuse the existing drawing function with the PSD flag enabled.


415-415: LGTM - Good backward compatibility.

Adding the drawPSD parameter with a default value of false ensures backward compatibility with existing code.


489-517: LGTM - Well-implemented PSD heat map scaling.

The conditional logic for PSD data appropriately clamps values between -40 and 10 dBm and scales them to a 0-100 range for color intensity.


1492-1500: LGTM - Good inclusion of new spectrum types.

The condition properly includes the new PSD spectrum types for mouse position handling.


1523-1532: LGTM - Good addition of PSD value display at cursor.

This implements the requested feature to show PSD values at the mouse cursor position.


1281-1288: LGTM - Good enhancement to interest frequency labels.

The addition of PSD values to interest frequency labels provides more useful information to the user.


395-395: LGTM - Simple line change.

The change maintains the existing functionality.


1635-1636: LGTM - Consistent margin handling.

The margin calculation correctly includes the new PSD spectrum types.

Co-authored-by: Mark Haslinghuis <mark@numloq.nl>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (2)
src/graph_spectrum_plot.js (2)

533-544: 🛠️ Refactor suggestion

Add robust validation for matrix FFT data access.

The getValueFromMatrixFFT function needs additional checks for null/undefined values to prevent potential runtime errors.

 GraphSpectrumPlot.getValueFromMatrixFFT  = function(frequency, vsArgument) {
   const NUM_VS_BINS = 100;  // redefinition of value from graph_spectrum_calc.js module!
   const matrixFFT = this._fftData;
+  if (!matrixFFT || !matrixFFT.fftOutput || !matrixFFT.vsRange) {
+    return 0; // Return safe default if data is missing
+  }
   let vsArgumentIndex = Math.round(NUM_VS_BINS * (vsArgument - matrixFFT.vsRange.min) / (matrixFFT.vsRange.max - matrixFFT.vsRange.min));
   if (vsArgumentIndex === NUM_VS_BINS) {
     vsArgumentIndex = NUM_VS_BINS - 1;
   }
+  vsArgumentIndex = Math.max(0, Math.min(vsArgumentIndex, (matrixFFT.fftOutput.length || 1) - 1));
   let freqIndex = Math.round(2 * frequency / matrixFFT.blackBoxRate * (matrixFFT.fftLength - 1));
   freqIndex = Math.max(freqIndex, 0);
   freqIndex = Math.min(freqIndex, matrixFFT.fftLength - 1);
-  return matrixFFT.fftOutput[vsArgumentIndex][freqIndex];
+  return matrixFFT.fftOutput[vsArgumentIndex] ? matrixFFT.fftOutput[vsArgumentIndex][freqIndex] : 0;
 };

408-413: 🛠️ Refactor suggestion

Add null/undefined checks and better bounds validation.

The getPSDbyFreq function could be more robust with additional checks.

 GraphSpectrumPlot.getPSDbyFreq  = function(frequency) {
+  if (!this._fftData || !this._fftData.psdOutput || !this._fftData.psdOutput.length) {
+    return 0; // Return safe default if data is missing
+  }
   let freqIndex = Math.round(2 * frequency / this._fftData.blackBoxRate * (this._fftData.psdOutput.length - 1) );
   freqIndex = Math.min(freqIndex, this._fftData.psdOutput.length - 1);
   freqIndex = Math.max(freqIndex, 0);
-  return this._fftData.psdOutput.length ? this._fftData.psdOutput[freqIndex] : 0;
+  return this._fftData.psdOutput[freqIndex];
 };
🧹 Nitpick comments (1)
src/graph_spectrum_plot.js (1)

1571-1581: Fix indentation to match project style.

The indentation in this section (4 spaces) doesn't match the 2-space indentation used in the rest of the file.

         if (this._spectrumType === SPECTRUM_TYPE.PSD_VS_THROTTLE ||
             this._spectrumType === SPECTRUM_TYPE.PSD_VS_RPM) {
           const label = Math.round(this.getValueFromMatrixFFT(mouseFrequency, vsArgValue)).toString() + "dBm/Hz";
           this._drawAxisLabel(
-            canvasCtx,
-            label,
-            mouseX - 30,
-            mouseY - 4,
-            "left",
+          canvasCtx,
+          label,
+          mouseX - 30,
+          mouseY - 4,
+          "left",
           );
         }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3877f84 and 6334c89.

📒 Files selected for processing (1)
  • src/graph_spectrum_plot.js (17 hunks)
🔇 Additional comments (7)
src/graph_spectrum_plot.js (7)

18-22: LGTM: Enum values updated for new PSD chart types.

The SPECTRUM_TYPE enum has been cleanly extended to include new power spectral density visualization types.


174-180: LGTM: New PSD chart types correctly integrated.

The new cases for PSD_VS_THROTTLE and PSD_VS_RPM appropriately call the existing _drawFrequencyVsXGraph method with the drawPSD flag set to true.


415-415: LGTM: Function signature updated properly.

The optional drawPSD parameter with default value makes the function backward compatible.


489-517: LGTM: Heat map handling with PSD-specific scaling.

The conditional logic to handle PSD visualization with appropriate scaling from -40dB to +10dB is well implemented.


1281-1288: LGTM: Enhanced interest frequency label with PSD value.

The conditional logic to add PSD value information to interest frequency labels improves the user experience.


1523-1532: LGTM: PSD value display at cursor position.

This functionality enhances user interaction by displaying the PSD value at the current mouse position.


1635-1636: LGTM: Margin handling extended for new spectrum types.

The _getActualMarginLeft function has been properly updated to include the new PSD spectrum types.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (2)
src/graph_spectrum_calc.js (2)

112-118: Fixed variable name and zoom-factor handling, but consider further improvements.

The typo in variable name (multipilermultiplier) is fixed, but the zoom-factor logic could still be improved.

The current approach still has potential issues:

  1. Division by zero if analyserZoomY is 0
  2. Abrupt changes in resolution with step-wise doubling
  3. Negative zoom factors would produce nonsensical results

Consider implementing the safer approach suggested in previous reviews:

-  const multiplier = Math.floor(1 / analyserZoomY); // 0. ... 10
-  if (multiplier == 0) {
-    pointsPerSegment = 256;
-  } else if (multiplier > 1) {
-    pointsPerSegment *= 2 ** Math.floor(multiplier / 2);
-  }
+  if (analyserZoomY <= 0) {
+    console.warn("analyserZoomY must be > 0 – falling back to 1");
+    analyserZoomY = 1;
+  }
+  
+  // Continuous mapping: zoom-in ⇒ longer segments, zoom-out ⇒ shorter.
+  // Clamp to a practical range to avoid heavy memory consumption.
+  const MAX_SEGMENT = 4096;
+  const MIN_SEGMENT = 256;
+  pointsPerSegment = Math.round(
+    Math.min(MAX_SEGMENT, 
+             Math.max(MIN_SEGMENT, 512 * analyserZoomY))
+  );

686-686: Consider adding input validation to prevent issues with invalid segment sizes.

The _fft_segmented function doesn't validate that pointsPerSegment is positive and non-zero, which could lead to division by zero or other unexpected behaviors.

 GraphSpectrumCalc._fft_segmented  = function(samples, pointsPerSegment, overlapCount) {
+  // Validate inputs
+  if (pointsPerSegment <= 0) {
+    console.error("pointsPerSegment must be positive");
+    return [];
+  }
+  
   const samplesCount = samples.length,
         fftLength = Math.floor(pointsPerSegment / 2);
🧹 Nitpick comments (1)
src/graph_spectrum_calc.js (1)

221-290: LGTM! Well-implemented new power spectral density calculation method.

The new _dataLoadPowerSpectralDensityVsX method effectively implements PSD calculations for the variable spectrum analysis. The code:

  1. Uses an appropriate matrix allocation pattern with better initialization
  2. Correctly handles data chunking and averaging
  3. Follows the same overall structure as similar methods for consistency

Consider adding input validation for fftChunkLength to prevent potential issues with very small or invalid values:

+  if (fftChunkLength <= 0) {
+    console.warn("Invalid fftChunkLength:", fftChunkLength, "- using default");
+    fftChunkLength = 512;
+  }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6334c89 and 77acf1e.

📒 Files selected for processing (1)
  • src/graph_spectrum_calc.js (11 hunks)
🔇 Additional comments (5)
src/graph_spectrum_calc.js (5)

82-82: LGTM! Fixed property reference.

The code now correctly uses this._analyserTimeRange.in instead of the previously incorrect this.analyserTimeRange.in.


132-132: LGTM! Rename for improved clarity.

Consistently renaming maxNoiseIdx to maxNoiseFrequency throughout the code improves readability by clarifying that this value represents a frequency rather than just an index.

Also applies to: 583-584, 590-590, 641-641, 679-679


292-295: LGTM! Clean interface for selecting between PSD and FFT analysis.

The updated public methods with the drawPSD parameter provide a clean and backward-compatible way to choose between PSD and FFT analysis modes.

Also applies to: 297-300


421-421: LGTM! Added flexibility for scaled vs. unscaled sample processing.

Adding the scaled parameter to _getFlightSamplesFreqVsX enables reuse of this method for both raw and curve-scaled data analysis, which is particularly important for accurate PSD calculations.

Also applies to: 432-436


687-689: LGTM! Improved variable declarations and local calculation.

Defining fftLength locally and using it consistently improves readability and avoids recalculation.

Also applies to: 698-699

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
src/graph_spectrum_calc.js (3)

113-118: Typo fixed, but zoom logic could be improved.

The typo correction from multipiler to multiplier is good, but the zoom factor logic still has potential issues.

Consider adding validation and creating a more intuitive relationship between zoom factor and FFT resolution:

-  const multiplier = Math.floor(1 / analyserZoomY); // 0. ... 10
-  if (multiplier == 0) {
-    pointsPerSegment = 256;
-  } else if (multiplier > 1) {
-    pointsPerSegment *= 2 ** Math.floor(multiplier / 2);
-  }
+  if (analyserZoomY <= 0) {
+    console.warn("analyserZoomY must be > 0 – falling back to 1");
+    analyserZoomY = 1;
+  }
+
+  // Continuous mapping: zoom-in (smaller Y) ⇒ longer segments, zoom-out (larger Y) ⇒ shorter
+  // Clamp to a practical range to avoid heavy memory consumption
+  const MAX_SEGMENT = 4096;
+  const MIN_SEGMENT = 256;
+  pointsPerSegment = Math.round(
+    Math.min(MAX_SEGMENT, 
+             Math.max(MIN_SEGMENT, 512 / analyserZoomY))
+  );

221-290: PSD vs X implementation looks solid with minor optimization opportunities.

The new _dataLoadPowerSpectralDensityVsX function correctly implements power spectral density calculations binned by variable spectrum values.

The implementation could be optimized:

-  const matrixPsdOutput = new Array(NUM_VS_BINS)
-        .fill(null)
-        .map(() => (new Float64Array(psdLength)).fill(-100));
+  // Precisely allocate the needed memory (psdLength values per bin)
+  const matrixPsdOutput = new Array(NUM_VS_BINS)
+        .fill(null)
+        .map(() => (new Float64Array(psdLength)).fill(-100));

Also, consider adding validation for edge cases:

+  // Validate inputs
+  if (fftChunkLength <= 0) {
+    console.error("Invalid chunk length");
+    return {
+      fieldIndex: this._dataBuffer.fieldIndex,
+      fieldName: this._dataBuffer.fieldName,
+      fftLength: 0,
+      fftOutput: [],
+      maxNoise: 0,
+      blackBoxRate: this._blackBoxRate,
+      vsRange: { min: 0, max: 0 },
+    };
+  }

221-290: Consider reducing code duplication between FFT and PSD implementations.

There's significant duplication between _dataLoadFrequencyVsX and _dataLoadPowerSpectralDensityVsX methods.

You could extract common functionality into helper methods to reduce duplication and improve maintainability. For example:

// Example helper method to extract common VS binning logic
_binValuesByVsField(vsValues, samples, fftChunkIndex, fftChunkLength, minValue, maxValue, processFunc) {
  const numberSamples = new Uint32Array(NUM_VS_BINS);
  const matrixOutput = new Array(NUM_VS_BINS)
        .fill(null)
        .map(() => processFunc.createOutputArray());
        
  // Common binning logic
  for (const vsValueArray of vsValues) {
    // Calculate average vs value and bin index
    // ... existing code ...
    
    // Apply the processing function
    processFunc.processChunk(matrixOutput[vsBinIndex], samples, fftChunkIndex, fftChunkLength);
  }
  
  // Average the values
  // ... existing code ...
  
  return { matrixOutput, numberSamples };
}

This would make the code more maintainable and reduce the risk of bugs when updating one implementation but not the other.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 77acf1e and b1bd761.

📒 Files selected for processing (1)
  • src/graph_spectrum_calc.js (10 hunks)
🔇 Additional comments (11)
src/graph_spectrum_calc.js (11)

82-82: Good fix for incorrect property reference.

You've correctly updated the property reference to this._analyserTimeRange.in which fixes a potential undefined variable bug.


132-132: Improved naming clarity.

Renaming maxNoiseIdx to maxNoiseFrequency better communicates the meaning of this value.


292-295: Clean implementation of conditional PSD support for throttle.

The updated method elegantly supports both FFT and PSD analysis based on the drawPSD parameter.


297-299: Clean implementation of conditional PSD support for RPM.

The updated method correctly supports both FFT and PSD analysis for RPM data.


421-421: Good addition of scaled parameter for flexible data processing.

Adding the scaled parameter enables both raw and curve-scaled data retrieval, which supports the PSD calculations.


432-436: Correct implementation of conditional scaling.

The conditional logic correctly applies curve scaling only when requested, allowing raw data access for PSD calculations.


583-584: Improved naming for better code clarity.

Renaming to maxNoiseFrequency better communicates that this is a frequency value rather than an index.

Also applies to: 590-590


641-641: Consistent naming across early exit case.

Maintaining consistent naming with the rest of the codebase is good practice.


679-679: Consistent naming in return value.

This naming change maintains consistency throughout the codebase.


687-688: Good code refactoring with local variable.

Extracting fftLength into a local variable avoids repeated calculations and improves readability.


698-699: Efficient use of local variable in loop.

Using the pre-calculated fftLength in the loop initialization is more efficient.

@demvlad
Copy link
Contributor Author

demvlad commented May 13, 2025

this._drawAxisLabel(
canvasCtx,
label,
mouseX - 30,
mouseY - 4,

  •        "left",
    
  •        "left"
        );
    

Perhaps, the coderabbit is right. the last comma is not nesassary by coding rule.

@demvlad demvlad changed the title Added power spectral density charts with throttle and RPM Added power spectral density charts May 13, 2025
@demvlad
Copy link
Contributor Author

demvlad commented May 13, 2025

@demvlad done - we can always revert :)

it has reverted:)
Continue in this PR.

@demvlad
Copy link
Contributor Author

demvlad commented May 14, 2025

@coderabbitai
how can i quick get the oneside spectrum with power at 2 input size

Copy link

coderabbitai bot commented May 14, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Look for FFT and PSD implementation in the codebase
echo "Searching for FFT methods in graph_spectrum_calc.js..."
cat src/graph_spectrum_calc.js | grep -A 20 "_fft" || echo "No FFT method found"

echo -e "\nSearching for PSD methods in graph_spectrum_calc.js..."
cat src/graph_spectrum_calc.js | grep -A 20 "_psd" || echo "No PSD method found"

echo -e "\nChecking how one-sided spectrum is implemented..."
grep -A 10 "one-sided\|onesided\|one_sided" src/graph_spectrum_calc.js || echo "No direct references to one-sided spectrum found"

echo -e "\nLooking for power calculations in FFT or PSD methods..."
grep -A 5 "power.*input.*size\|input.*size.*power" src/graph_spectrum_calc.js || echo "No direct references to power at input size found"

echo -e "\nExamining more of the spectrum calculation code to understand context..."
grep -A 10 "spectrum\|Spectrum" src/graph_spectrum_calc.js

echo -e "\nLooking at the _fft method implementation..."
grep -n "_fft" src/graph_spectrum_calc.js | head -1

Length of output: 16278


@demvlad To get a one-sided spectrum with power at 2× input size, you have a few options based on the current implementation:

  1. Use the existing _fft_segmented method with modified scaling:
// Get one-sided spectrum with power scaled properly
function getOneSidedSpectrum(samples) {
  // Use existing FFT method - it already returns a one-sided spectrum
  const fftOutput = GraphSpectrumCalc._fft(samples);
  const fftLength = Math.floor(samples.length / 2);
  
  // Create one-sided spectrum with proper scaling
  const magnitudes = new Float64Array(fftLength);
  
  // DC component (index 0) is unique
  magnitudes[0] = Math.hypot(fftOutput[0], fftOutput[1]) / samples.length;
  
  // Scale by 2 for the rest (except Nyquist if even length)
  for (let i = 1; i < fftLength - 1; i++) {
    const re = fftOutput[2 * i];
    const im = fftOutput[2 * i + 1];
    // Multiply by 2 to preserve power in one-sided spectrum
    magnitudes[i] = 2 * Math.hypot(re, im) / samples.length;
  }
  
  // Nyquist frequency (if even length) is also unique
  if (samples.length % 2 === 0) {
    const nyquistIndex = fftLength - 1;
    magnitudes[nyquistIndex] = Math.hypot(
      fftOutput[2 * nyquistIndex], 
      fftOutput[2 * nyquistIndex + 1]
    ) / samples.length;
  }
  
  return magnitudes;
}
  1. Use the _psd method with 'spectrum' scaling:
// Use existing PSD implementation with 'spectrum' scaling
function getOneSidedPowerSpectrum(samples) {
  const pointsPerSegment = samples.length; // No segmentation
  const overlapCount = 0; // No overlap
  
  // 'spectrum' scaling properly scales one-sided spectrum
  const psd = GraphSpectrumCalc._psd(samples, pointsPerSegment, overlapCount, 'spectrum');
  
  return psd.psdOutput;
}
  1. If you need it for raw FFT without segmentation:
// Quick one-sided power spectrum from raw FFT
function quickOneSidedPowerSpectrum(samples) {
  const fftComplex = GraphSpectrumCalc._fft(samples);
  const fftLength = Math.floor(samples.length / 2);
  const magnitudes = new Float64Array(fftLength);
  
  // DC component (first bin)
  magnitudes[0] = (fftComplex[0]**2 + fftComplex[1]**2) / samples.length**2;
  
  // Scale middle bins by 2 to account for negative frequencies
  for (let i = 1; i < fftLength - 1; i++) {
    const re = fftComplex[2 * i];
    const im = fftComplex[2 * i + 1];
    magnitudes[i] = 2 * (re**2 + im**2) / samples.length**2;
  }
  
  // Nyquist frequency (if even length)
  if (samples.length % 2 === 0) {
    const nyquistIndex = fftLength - 1;
    magnitudes[nyquistIndex] = (fftComplex[2 * nyquistIndex]**2 + 
                               fftComplex[2 * nyquistIndex + 1]**2) / samples.length**2;
  }
  
  return magnitudes;
}

The key point is multiplying non-DC/non-Nyquist bins by 2 to account for the power from negative frequencies that are excluded in a one-sided spectrum.

@demvlad
Copy link
Contributor Author

demvlad commented May 14, 2025

@coderabbitai
Has our FFT library more quick fft method for me

Copy link

coderabbitai bot commented May 14, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@demvlad Let me check if there are any faster FFT methods in the codebase.

@demvlad Based on examining the codebase, I found that the FFT implementation in the Blackbox Log Viewer uses a classic Cooley-Tukey radix-2 algorithm for FFT calculations. Here are the FFT methods available:

  1. _fft(samples) - The core FFT implementation using the Cooley-Tukey algorithm
  2. _fft_segmented(samples, pointsPerSegment, overlapCount) - Applies FFT to segments of the signal with overlap, which is useful for PSD calculations

I don't see any more optimized variants of the FFT algorithm in the codebase. If you need a faster implementation, you have a few options:

  1. Use the existing _fft_segmented with carefully chosen parameters - smaller segments can be faster but at the cost of frequency resolution
  2. Consider implementing or integrating a more optimized FFT library like:
    • Web Audio API's built-in FFT (if running in browser context)
    • A WebAssembly-based FFT implementation (like FFTW compiled to WASM)
    • A typed array optimized FFT implementation

For your specific use case of getting a one-sided spectrum with power at 2× input size, the existing methods should work with the scaling adjustments I suggested in my previous response.

Would you like me to help you adapt the existing implementation to be more optimized for your specific use case?

(⌐■_■)🐇


🏁 Script executed:

#!/bin/bash
# Looking for FFT-related implementations in the codebase
echo "Searching for FFT methods in graph_spectrum_calc.js..."
grep -n "_fft" src/graph_spectrum_calc.js
echo ""

echo "Examining FFT implementations..."
grep -A 30 "_fft.*=" src/graph_spectrum_calc.js || echo "No direct FFT method definition found"
echo ""

echo "Checking for additional FFT-related methods or optimizations..."
grep -A 5 "FFT\|fft" src/graph_spectrum_calc.js | grep -v "_fft"
echo ""

echo "Looking for any external FFT libraries or imports..."
grep -A 2 "import.*fft\|require.*fft" src/* || echo "No external FFT libraries imported"
echo ""

echo "Examining general structure of graph_spectrum_calc.js..."
head -n 20 src/graph_spectrum_calc.js

Length of output: 14549


🏁 Script executed:

#!/bin/bash
# Let's examine the FFT implementations in more detail
echo "Examining the _fft method in detail:"
grep -n "_fft = " src/graph_spectrum_calc.js -A 40

echo -e "\nLooking for any other FFT implementation that might be faster:"
grep -n "fft.*=" src/graph_spectrum_calc.js | grep -v "_fft ="

echo -e "\nChecking for any inline comments about FFT optimization:"
grep -i "fast\|quick\|optim" src/graph_spectrum_calc.js | grep -i "fft"

echo -e "\nLooking at _fft_segmented to understand its relationship to _fft:"
grep -n "_fft_segmented" src/graph_spectrum_calc.js -A 15

Length of output: 4440


@demvlad Looking at the codebase, there's no specialized "quick FFT" method beyond the existing implementations. The code uses an external FFT library (FFT.complex) with its simple() method for the core calculations.

Your current options are:

  1. Use the existing _fft method directly - this is the most basic approach
  2. Use _fft_segmented with carefully chosen parameters - which can be more efficient for certain use cases

For a one-sided spectrum with power at 2× input size, you could implement a small wrapper function around the existing FFT:

function quickOneSidedPowerSpectrum(samples) {
  // Get complex FFT result
  const fftComplex = GraphSpectrumCalc._fft(samples);
  const fftLength = Math.floor(samples.length / 2);
  
  // Create power spectrum output
  const powerSpectrum = new Float64Array(fftLength);
  
  // DC component (not doubled)
  powerSpectrum[0] = (fftComplex[0]**2 + fftComplex[1]**2) / samples.length**2;
  
  // Scale by 2 for all other frequencies (except Nyquist if even length)
  for (let i = 1; i < fftLength - (samples.length % 2 === 0 ? 1 : 0); i++) {
    const re = fftComplex[2*i];
    const im = fftComplex[2*i+1];
    powerSpectrum[i] = 2 * (re**2 + im**2) / samples.length**2;
  }
  
  // Handle Nyquist frequency (if even length)
  if (samples.length % 2 === 0) {
    const nyquistIdx = fftLength - 1;
    powerSpectrum[nyquistIdx] = (fftComplex[2*nyquistIdx]**2 + fftComplex[2*nyquistIdx+1]**2) / samples.length**2;
  }
  
  return powerSpectrum;
}

This gives you one-sided power spectrum with the appropriate 2× scaling for non-DC, non-Nyquist components.

Would this work for your use case?

(⌐■_■)🐇


✏️ Learnings added
Learnt from: demvlad
URL: https://github.com/betaflight/blackbox-log-viewer/pull/827

Timestamp: 2025-05-14T08:46:49.597Z
Learning: The FFT implementation in Betaflight Blackbox Log Viewer uses a Cooley-Tukey radix-2 algorithm with two main methods: _fft(samples) for the core algorithm and _fft_segmented(samples, pointsPerSegment, overlapCount) for applying FFT to overlapping segments of the signal.

Learnt from: demvlad
URL: https://github.com/betaflight/blackbox-log-viewer/pull/827

Timestamp: 2025-05-14T08:46:49.597Z
Learning: The FFT implementation in Betaflight Blackbox Log Viewer uses a Cooley-Tukey radix-2 algorithm with two main methods: _fft(samples) for the core algorithm and _fft_segmented(samples, pointsPerSegment, overlapCount) for applying FFT to overlapping segments of the signal.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@demvlad
Copy link
Contributor Author

demvlad commented May 14, 2025

@coderabbitai
Has the FFT.complex library other methods or options to get fft?

Copy link

coderabbitai bot commented May 14, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for FFT.complex references to understand the library and its methods
echo "Searching for FFT.complex references:"
grep -n "FFT\.complex" src/* | grep -v "test"

echo -e "\nLooking for the FFT library definition or import:"
grep -n "FFT *=" src/* || grep -n "import.*FFT" src/* || grep -n "require.*FFT" src/*

echo -e "\nChecking for any files that might define the FFT library:"
find . -name "fft*.js" -o -name "*fft.js" -o -name "FFT*.js"

echo -e "\nExamining the source of graph_spectrum_calc.js where FFT is likely used:"
head -n 30 src/graph_spectrum_calc.js

echo -e "\nLooking at FFT usage in the codebase:"
grep -A 5 "FFT\." src/graph_spectrum_calc.js

Length of output: 6671


@demvlad Based on examining the codebase, I can only see the FFT.complex constructor and its simple() method being used. The library appears to be included from a third-party source rather than being defined within this codebase.

What I can see from the code:

  • Constructor: new FFT.complex(fftLength, false) - takes size and a boolean (likely for inverse FFT)
  • The simple() method: fft.simple(fftOutput, samples, type) - performs the FFT calculation

I don't see any other methods being used in the code, and without access to the library's full definition or documentation, I can't definitively list all available methods.

If you're looking for alternative FFT methods, you might want to:

  1. Check the original FFT library documentation (likely referenced in package.json or vendor files)
  2. Look at the actual library source if it's included in the vendor directory
  3. Consider implementing the power spectrum calculation as shown in my previous response, which would give you a one-sided spectrum with proper 2× scaling

Would you like me to help search for more information about the specific FFT library being used?

(⌐■_■)🐇

Copy link

coderabbitai bot commented May 14, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@demvlad Based on examining the codebase, the FFT.complex library appears to be used through a simple API. Here's what I found:

  1. Available Methods:

    • simple(signal, 0, target) - The main method used in the codebase for FFT computation
    • The library might have other methods not used in this codebase
  2. Options for FFT Computation:

    • When creating an FFT.complex instance, you pass the size parameter: new FFT.complex(size)
    • The simple() method takes three parameters:
      • signal: Input array to transform
      • 0: Zero value (appears to be a constant)
      • target: Output array where results are placed
  3. Alternative Approaches:

    • You can use different window functions (currently using windowHanning)
    • You can adjust the pointsPerSegment and overlapCount parameters when using _fft_segmented
    • You can experiment with raw FFT vs. segmented FFT depending on your needs

If you're looking for a faster FFT implementation, you might consider:

  1. Using smaller segments with _fft_segmented (trade-off: lower frequency resolution)
  2. Using a different window function that requires less computation
  3. Pre-allocating the FFT target array to avoid memory allocation during computation

Would you like me to help you implement any specific optimization for your use case?


🏁 Script executed:

#!/bin/bash
# Look for FFT.complex definition and usage in the codebase
echo "Searching for FFT.complex usage in graph_spectrum_calc.js..."
grep -n "FFT\.complex" src/graph_spectrum_calc.js -A 3 -B 3

echo -e "\nExamining the _fft implementation that uses FFT.complex..."
grep -n "_fft = " src/graph_spectrum_calc.js -A 30

echo -e "\nLooking for the FFT library definition..."
find . -name "*.js" -type f -exec grep -l "FFT\.complex =" {} \; || echo "No direct FFT.complex definition found"

echo -e "\nChecking script tags in index.html for external libraries..."
grep "<script" index.html | grep "src=" | grep -v "text/html"

echo -e "\nListing files in js/libraries directory..."
ls -la js/libraries/ 2>/dev/null

Length of output: 2206


🏁 Script executed:

#!/bin/bash
# Let's check the FFT library files
echo "Looking for complex.js and real.js files..."
find . -name "complex.js" -o -name "real.js"

echo -e "\nExamining complex.js (if found)..."
if [ -f "js/complex.js" ]; then
    head -n 50 js/complex.js
elif [ -f "js/libraries/complex.js" ]; then
    head -n 50 js/libraries/complex.js
else
    echo "complex.js not found in expected locations"
fi

echo -e "\nSearching for FFT.complex definition..."
if [ -f "js/complex.js" ]; then
    grep -A 20 "FFT.complex = " js/complex.js || echo "No direct definition found in js/complex.js"
elif [ -f "js/libraries/complex.js" ]; then
    grep -A 20 "FFT.complex = " js/libraries/complex.js || echo "No direct definition found in js/libraries/complex.js"
fi

echo -e "\nLooking for methods of FFT.complex..."
if [ -f "js/complex.js" ]; then
    grep -A 3 "FFT.complex.prototype" js/complex.js
elif [ -f "js/libraries/complex.js" ]; then
    grep -A 3 "FFT.complex.prototype" js/libraries/complex.js
fi

echo -e "\nChecking the FFT constructor and other methods..."
if [ -f "js/complex.js" ]; then
    grep -A 5 "function FFT" js/complex.js
elif [ -f "js/libraries/complex.js" ]; then
    grep -A 5 "function FFT" js/libraries/complex.js
fi

Length of output: 919


🏁 Script executed:

#!/bin/bash
# Examining the FFT library with correct paths
echo "Examining complex.js file..."
cat ./public/js/complex.js | head -n 100

echo -e "\nSearching for FFT.complex definition and methods..."
grep -A 20 "FFT.complex = " ./public/js/complex.js || echo "No direct definition found"

echo -e "\nLooking for prototype methods of FFT.complex..."
grep -A 3 "FFT.complex.prototype" ./public/js/complex.js

echo -e "\nLooking for the 'simple' method specifically..."
grep -A 15 "FFT.complex.prototype.simple" ./public/js/complex.js

echo -e "\nChecking if there are other important methods..."
grep -E "FFT.complex.prototype\.[a-zA-Z]+" ./public/js/complex.js | grep -v "simple"

Length of output: 6000


🏁 Script executed:

#!/bin/bash
# Let's look for the FFT.complex constructor and methods in different ways
echo "Looking for FFT.complex constructor..."
grep -A 5 "namespace.complex = " ./public/js/complex.js

echo -e "\nSearching for any FFT constructor..."
grep -A 5 "= function" ./public/js/complex.js | grep -A 5 "FFT"

echo -e "\nLooking for any prototype methods..."
grep -n "\.prototype\." ./public/js/complex.js

echo -e "\nExamining usage of FFT.complex in spectrum_calc.js..."
grep -n "fft\." src/graph_spectrum_calc.js

echo -e "\nLet's get more context around how FFT.complex is used..."
grep -n -A 5 -B 5 "new FFT\.complex" src/graph_spectrum_calc.js

echo -e "\nLet's look for the 'simple' method used in the code..."
grep -n -A 3 "fft\.simple" src/graph_spectrum_calc.js

Length of output: 2490


@demvlad Based on examining the FFT implementation in the codebase, I found that the FFT.complex library provides two main methods:

  1. simple(output, input, type) - This is the method currently used in the codebase with parameters:

    • output: The array where FFT results will be stored
    • input: The input data array
    • type: Can be 'real' (as seen in the code) or potentially other values
  2. process(output, outputOffset, outputStride, input, inputOffset, inputStride, type) - A more granular method that isn't currently used in the codebase but provides additional control with parameters:

    • More precise control over memory layout
    • Ability to specify offsets and strides for both input and output arrays
    • Could potentially be more efficient for certain use cases

When instantiating FFT.complex, the constructor takes these parameters:

const fft = new FFT.complex(size, false);
  • size: The length of data to be transformed
  • false: A configuration parameter (likely related to normalization)

If you're looking for ways to optimize your FFT usage, you could:

  1. Reuse FFT instances instead of creating new ones for each calculation
  2. Pre-allocate output arrays to reduce garbage collection overhead
  3. Try the more granular process() method if you need to work with specific data offsets or memory layouts
  4. Experiment with different FFT sizes (powers of 2 are typically most efficient)

Would you like me to provide example code for any of these approaches?


✏️ Learnings added
Learnt from: demvlad
URL: https://github.com/betaflight/blackbox-log-viewer/pull/827

Timestamp: 2025-05-14T08:52:53.004Z
Learning: The FFT.complex library in Betaflight Blackbox Log Viewer provides the simple() method for FFT computation, which takes a signal array, a zero value (0), and a target array for results. New instances are created with new FFT.complex(size) where size is the length of the data to be transformed.

Learnt from: demvlad
URL: https://github.com/betaflight/blackbox-log-viewer/pull/827

Timestamp: 2025-05-14T08:52:53.004Z
Learning: The FFT.complex library in Betaflight Blackbox Log Viewer provides two main methods: simple(output, input, type) and process(output, outputOffset, outputStride, input, inputOffset, inputStride, type). The 'simple' method is the one currently used in the codebase, while 'process' offers more granular control over memory layout with offsets and strides.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants