Skip to content
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

[refactor] lexical scoping #692

Closed

Conversation

haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Jan 23, 2024

TBD

@haslinghuis haslinghuis self-assigned this Jan 23, 2024
Copy link
Member

@McGiverGim McGiverGim left a comment

Choose a reason for hiding this comment

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

There is no way way to fix this without returning to the old var?

This comment has been minimized.

@nerdCopter
Copy link
Member

a585376f no change in behavior for me (still Cannot plot analyser TypeError: Cannot read properties of undefined (reading '0') for Freq vs Throttle).

@tbolin
Copy link
Contributor

tbolin commented Jan 23, 2024

I don't think the variable scopes are the problem. There is a possibility that 'vsBinIndex' is 100, which cause an out of bounds array access on line 124 in graph_spectrum_calc.js
bild

@nerdCopter
Copy link
Member

nerdCopter commented Jan 23, 2024

i put line 124 inside of try/catch and yes, many errors. graph rendered successfully.
image
(reading 200..299) on my log. 0..599 on the test 1.txt log.

@haslinghuis haslinghuis force-pushed the fix-freq-analyzer-switch branch from a585376 to 1e38671 Compare January 23, 2024 20:52

This comment has been minimized.

@haslinghuis haslinghuis force-pushed the fix-freq-analyzer-switch branch from 1e38671 to baee59d Compare January 23, 2024 21:28
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@haslinghuis haslinghuis marked this pull request as draft January 23, 2024 21:28
Copy link

Do you want to test this code? Here you have an automated build:
Betaflight-Blackbox-Explorer-Linux
Betaflight-Blackbox-Explorer-macOS
Betaflight-Blackbox-Explorer-Windows
WARNING: It may be unstable and result in corrupted configurations or data loss. Use only for testing!

@nerdCopter
Copy link
Member

close this in favor of merged #693 correct?

@haslinghuis
Copy link
Member Author

Wanted to apply lexical scoping instead.

@haslinghuis haslinghuis changed the title Fix frequency analyzer [refactor] lexical scoping Jan 24, 2024
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs within a week.

Copy link

github-actions bot commented Mar 3, 2024

Pull request closed automatically as inactive.

@github-actions github-actions bot closed this Mar 3, 2024
@haslinghuis haslinghuis deleted the fix-freq-analyzer-switch branch March 3, 2024 10:38
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.

4 participants