Removed superfluous dataBuffer in graph_spectrum to prevent the random spectrum type change bug #841
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…m spectrum type change bug
## Walkthrough
The changes refactor how spectrum data buffers are managed by removing reliance on a global `dataBuffer` object. Functions now explicitly pass `fieldIndex`, `curve`, and `fieldName` as parameters. The logic for restoring or mutating a global data buffer was eliminated, and related function signatures and calls were updated accordingly.
## Changes
| File(s) | Change Summary |
|-----------------------|--------------------------------------------------------------------------------------------------------|
| src/graph_spectrum.js | Refactored `dataLoad` to accept explicit parameters; updated calls in `plotSpectrum` and event handlers; removed global `dataBuffer` logic. |
| src/graph_spectrum_calc.js | Changed `GraphSpectrumCalc.setDataBuffer` to accept three parameters instead of a single object. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant UI
participant FlightLogAnalyser
participant GraphSpectrumCalc
UI->>FlightLogAnalyser: plotSpectrum(fieldIndex, curve, fieldName)
FlightLogAnalyser->>GraphSpectrumCalc: setDataBuffer(fieldIndex, curve, fieldName)
GraphSpectrumCalc-->>FlightLogAnalyser: (updates internal buffer)
FlightLogAnalyser-->>UI: (renders spectrum plot)Possibly related PRs
Suggested labels
Suggested reviewers
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
|
Preview URL: https://71e3bbcf.betaflight-blackbox.pages.dev |
Contributor
Author
|
@coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1215 🏁 Script executed: Length of output: 77 🏁 Script executed: Length of output: 2823 @demvlad Yes, this solution (PR #841) is significantly better than #839. Here's why: Fundamental Approach DifferencePR #839 (Reactive Fix):
PR #841 (Proactive Fix):
Why PR #841 is Superior
The trade-off is that PR #841 is more invasive to the existing API, but this is a worthwhile change for the improved reliability and maintainability it provides. As you noted in the PR description, PR #839 "did not fully prevent the random dataBuffer destruction bug" - this confirms that the band-aid approach wasn't sufficient, making the more thorough fix in PR #841 the right choice. |
haslinghuis
approved these changes
Jun 7, 2025
nerdCopter
approved these changes
Jun 9, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The continue of #837 #838 #839 PRs.
The last #839 PR has not full the random dataBuffer destroing bug prevention
There is no dataBuffer, there is no this bug by using this PR
Summary by CodeRabbit