Added Power Spectral Density curves at the spectrum chart (Completion of #820 PR) #826
Conversation
Co-authored-by: MikeNomatter <[email protected]>
Co-authored-by: Mark Haslinghuis <[email protected]>
Co-authored-by: Mark Haslinghuis <[email protected]>
Co-authored-by: Mark Haslinghuis <[email protected]>
Co-authored-by: Mark Haslinghuis <[email protected]>
WalkthroughA new "Power spectral density" spectrum type was added to the application's spectrum analysis features. This includes updates to the HTML dropdown, data loading logic, PSD calculation methods, and graph rendering to fully support PSD visualization and user interaction within the existing spectrum analysis workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI (index.html)
participant FlightLogAnalyser
participant GraphSpectrumCalc
participant GraphSpectrumPlot
User->>UI (index.html): Select "Power spectral density" from dropdown
UI->>FlightLogAnalyser: Notify spectrum type change
FlightLogAnalyser->>GraphSpectrumCalc: dataLoadPSD(analyserZoomY)
GraphSpectrumCalc->>GraphSpectrumCalc: _getFlightSamplesFreq(false)
GraphSpectrumCalc->>GraphSpectrumCalc: _psd(samples, ...)
GraphSpectrumCalc-->>FlightLogAnalyser: Return PSD data
FlightLogAnalyser->>GraphSpectrumPlot: setData(PSD data)
GraphSpectrumPlot->>GraphSpectrumPlot: _drawPowerSpectralDensityGraph()
GraphSpectrumPlot-->>User: Render PSD graph
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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 (
|
demvlad
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/graph_spectrum.js (1)
192-199: Debounced PSD recalculation is still expensive – consider cachingEvery Y-zoom slider tick recomputes the entire PSD. With long logs the Welch routine is O(N·log N) per segment and can become sluggish even with the 100 ms debounce.
You could short-circuit ifanalyserZoomYhasn’t crossed a power-of-two boundary (becausepointsPerSegmentonly changes at those thresholds) or memoise the last(zoomY → psd)pair.src/graph_spectrum_calc.js (3)
109-135: Segment-length heuristic: works but can mis-scale on fine zoom levels
multipiler = Math.floor(1 / analyserZoomY)gives the same value for a large range of zooms (e.g. 1.01-1.99 all map to 0/1/2).
Two suggestions:
- Use
Math.round()so each 25 % zoom step changes the segment size more smoothly.- Guard against
pointsPerSegment < 128– below that, Welch PSD becomes very noisy.
573-594: Variable shadowing and small optimisation in inner loopsNested
for (let i …)shadows the outer loop index twice (_psdand_fft_segmented).
It’s legal but hurts readability and prevents some JS engines from hoisting. Consider distinct names (j,k) for the inner loops.Also, computing
freqScale = 2 * WIDTH / PLOTTED_BLACKBOX_RATEonce outside the loop would avoid recomputing per-point.
610-630: Minor readability nit – reuse loop variablesIn
_fft_segmentedthe outer index isi, and an inner loop re-declaresi, which is easy to mis-read. Renaming the inner one (for (let k = 0; k < n_per_seg / 2; k++)) makes the control flow clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
index.html(1 hunks)src/graph_spectrum.js(2 hunks)src/graph_spectrum_calc.js(4 hunks)src/graph_spectrum_plot.js(10 hunks)
🔇 Additional comments (4)
index.html (1)
462-462: Add "Power spectral density" option to spectrumTypeSelect
The new<option value="4">Power spectral density</option>correctly enables the PSD view in the dropdown. Please verify that the JavaScript handling (e.g., the enum or switch cases for value4) is updated to map this option to thePOWER_SPECTRAL_DENSITYlogic.src/graph_spectrum.js (1)
119-122: PSD branch wired-up correctlyThe new case statement cleanly delegates to
GraphSpectrumCalc.dataLoadPSDand guarantees that the returned structure is stored infftData. No issues spotted here.src/graph_spectrum_plot.js (2)
1075-1093: Vertical-grid refactor looks goodAllowing variable tick counts makes the helper reusable and doesn’t break existing callers. Nice improvement.
1447-1454: Mouse-over label unit is correct for PSDGood catch adding
"dBm/Hz"and extending the spectrum check – the UX remains consistent across modes.Also applies to: 1482-1484
demvlad
changed the title
|
Completion of #820 PR "Added power spectral density curves at the spectrum chart"
Power spectral density curve at the spectrum chart.
Default vertical slider position:
The maximal vertical slider position:
Summary by CodeRabbit
New Features
Style