-
-
Notifications
You must be signed in to change notification settings - Fork 152
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
base: master
Are you sure you want to change the base?
Conversation
""" WalkthroughThree 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
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
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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 issueDivision-by-zero guard missing when
ticks === 0
_drawVerticalGridLines()
now accepts a customticks
argument, butPsd
graphs may passticksCount = 0
whenmaxY === minY
(e.g. very short logs).
HEIGHT / ticks
and(ticks – i)/ticks
will then throwInfinity/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 innerfor (let i = 0; i < pointsPerSegment / 2; i++)
reuses the same identifieri
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 distinctpointsPerSegment
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:
- 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
andgraph_spectrum_calc.js
).- 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 logsThe 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 blackChanging 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-declaresi
, 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:
- Rename the inner index (
k
/segIdx
) for clarity.- 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
📒 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 toObject.prototype.hasOwnProperty.call(...)
.- if (Object.hasOwn(this._flightLog.getMainFieldIndexes(), fieldName)) { + if (Object.prototype.hasOwnProperty.call(this._flightLog.getMainFieldIndexes(), fieldName)) {
There was a problem hiding this 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
📒 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 optionalticks
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.
There was a problem hiding this 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 variablei
reduces readabilityA second
for (let i = …)
is nested inside another loop that already declaresi
.
Althoughlet
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 callCreating and windowing a fresh
Array(pointsPerSegment)
on every invocation is unnecessary and generates significant GC pressure for large analyses.
Compute the window once perpointsPerSegment
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.
There was a problem hiding this 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 issueFix analyserZoomY handling and improve segment size scaling logic
The current implementation for calculating
pointsPerSegment
has several issues:
- It doesn't guard against invalid
analyserZoomY
values (0 or negative values would cause problems)- The zoom scaling logic is unintuitive and produces abrupt jumps in resolution
- 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 matrixThe 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 resultsThe code computes PSD then only uses it to extract
psdLength
andmaxNoise
, 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 optionsThe 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 functionThe 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
: UseMath.sqrt(re*re + im*im)
instead ofMath.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 floorThe 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
📒 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 functionThe overall approach for PSD calculation using the Welch method is well-implemented, with appropriate handling of the data and return of relevant metadata.
There was a problem hiding this 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 invalidanalyserZoomY
and improve scale mappingThe current logic for calculating
pointsPerSegment
has several issues:
multiplier = Math.floor(1 / analyserZoomY)
will throw ifanalyserZoomY
is zero- For negative zoom factors, the logic produces nonsensical segment sizes
- The step-wise doubling causes abrupt jumps in resolution
688-709
: Add validation for pointsPerSegment in _fft_segmentedThe 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
📒 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 calculationThe 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 selectionGood 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 accessAdding 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 methodsThe same
scaled
parameter approach is consistently applied here, maintaining code consistency. Good practice.
598-682
: Thorough PSD implementation with proper handling of corner casesThe PSD implementation follows standard signal processing practices:
- Uses Welch method with overlapping segments
- Properly applies window functions and scaling
- Converts to dB scale with appropriate noise floor
- Handles special cases like empty segments
Consider adding some documentation on the specific parameters and their expected ranges, especially for the scaling parameter.
@haslinghuis |
@demvlad done - we can always revert :) |
There was a problem hiding this 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
📒 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.
…he power specral density chart
There was a problem hiding this 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 getValueFromMatrixFFTThis 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
📒 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 visualizationThe enum extensions for
PSD_VS_THROTTLE
andPSD_VS_RPM
are properly defined and follow the existing pattern.
174-180
: LGTM: Correctly handling new spectrum typesThe switch case handling for the new PSD types properly delegates to
_drawFrequencyVsXGraph
with thedrawPSD
flag set to true.
413-413
: LGTM: Well-structured method parameterAdding the optional
drawPSD
parameter with a default value offalse
maintains backward compatibility.
487-515
: LGTM: Proper PSD visualization scalingThe 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 initializationInitializing
mouseFrequency
and including the new PSD spectrum types in the conditional check is correct.
1512-1521
: LGTM: Well-implemented PSD value display at cursorThe 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 cursorThe 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 handlingThe margin handling for the new PSD spectrum types follows the same pattern as their non-PSD counterparts.
src/graph_spectrum_plot.js
Outdated
GraphSpectrumPlot.getPSDbyFreq = function(frequency) { | ||
const freqIndex = Math.round(2 * frequency / this._fftData.blackBoxRate * (this._fftData.psdOutput.length - 1) ); | ||
return this._fftData.psdOutput[freqIndex]; | ||
}; |
There was a problem hiding this comment.
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.
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]; | |
}; |
Co-authored-by: Mark Haslinghuis <mark@numloq.nl>
There was a problem hiding this 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 suggestionAdd 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 suggestionAdd 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
📒 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>
There was a problem hiding this 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 suggestionAdd 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 suggestionAdd 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
📒 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
andPSD_VS_RPM
appropriately call the existing_drawFrequencyVsXGraph
method with thedrawPSD
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.
There was a problem hiding this 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 (
multipiler
→multiplier
) is fixed, but the zoom-factor logic could still be improved.The current approach still has potential issues:
- Division by zero if
analyserZoomY
is 0- Abrupt changes in resolution with step-wise doubling
- 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 thatpointsPerSegment
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:
- Uses an appropriate matrix allocation pattern with better initialization
- Correctly handles data chunking and averaging
- 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
📒 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 incorrectthis.analyserTimeRange.in
.
132-132
: LGTM! Rename for improved clarity.Consistently renaming
maxNoiseIdx
tomaxNoiseFrequency
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
…or some log files" This reverts commit 77acf1e.
|
There was a problem hiding this 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
tomultiplier
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
📒 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
tomaxNoiseFrequency
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 ofscaled
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.
this._drawAxisLabel(
Perhaps, the coderabbit is right. the last comma is not nesassary by coding rule. |
it has reverted:) |
@coderabbitai |
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed:
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:
// 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;
}
// 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;
}
// 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. |
@coderabbitai |
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:
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:
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:
Length of output: 14549 🏁 Script executed:
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 ( Your current options are:
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
|
@coderabbitai |
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed:
Length of output: 6671 @demvlad Based on examining the codebase, I can only see the FFT.complex constructor and its What I can see from the code:
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:
Would you like me to help search for more information about the specific FFT library being used? (⌐■_■)🐇 |
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:
If you're looking for a faster FFT implementation, you might consider:
Would you like me to help you implement any specific optimization for your use case? 🏁 Script executed:
Length of output: 2206 🏁 Script executed:
Length of output: 919 🏁 Script executed:
Length of output: 6000 🏁 Script executed:
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:
When instantiating FFT.complex, the constructor takes these parameters: const fft = new FFT.complex(size, false);
If you're looking for ways to optimize your FFT usage, you could:
Would you like me to provide example code for any of these approaches? ✏️ Learnings added
|
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.
The PSD charts (unfiltered and filtered). The color scaling is as -40dB - Black color, +10dB - White color
The casual spectrum charts (default scaling):


Will good to show the PSD value in the mouse position by using Shift key
Summary by CodeRabbit
New Features
Style