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

calc _blackBoxRate based on real timestamps #710

Closed
wants to merge 4 commits into from

Conversation

mituritsyn
Copy link
Contributor

@mituritsyn mituritsyn commented Feb 28, 2024

In some conditions the actual logging speed may differ from that set by the settings, it may be worth minimising such situations at the firmware level, but if we get such data we will be able to adequately handle them.
Issue was reported by @demvlad

Uploading HQProp.zip…

This comment has been minimized.

@demvlad
Copy link
Contributor

demvlad commented Feb 28, 2024

Yes! You found log records count computing in 3 rows code!
What i think.
I try to use actual log rate for good log file from my other quad. For example, one log has 2002Hz actual rate instead of 2000 from config data. But even in this case i see that frequency axis has main ticks labels with fractional (not int) values. May be use actual rates in case of significantly rate difference only (5% for example).
The situations like my FC are rare. Maybe show some warning at the spectrum chart in this case.
I have my realization of resolving this issue what is showed warnings at spectrum chart. But it has bad realization of logs records count computing. I can add warning show into your code.

@demvlad
Copy link
Contributor

demvlad commented Feb 29, 2024

or would you mind if I will use your codes fragment in my realization?

@mituritsyn
Copy link
Contributor Author

or would you mind if I will use your codes fragment in my realization?

I have a feeling that this implementation has significantly increased graphs loading time, I want to test another way to calculate the data rows count, but it is not so elegant and is based on some assumptions that need to be tested as well.

@demvlad
Copy link
Contributor

demvlad commented Mar 1, 2024

I have a feeling that this implementation has significantly increased graphs loading time, I want to test another way to calculate the data rows count, but it is not so elegant and is based on some assumptions that need to be tested as well.

This is good, if faster solution is exists.
I've notice, that this procedure have some delay for big time interval. There is the time limit 5min flight time in source specter code.
But i think, maybe better to make this checking once when log is loaded. As we look, this is a bad situation till. Maybe better to informed about it after log loading. And use this data from log class in spectrum chart as ready.

I am not full study till, how works log data parsing code... I have some questions to minmax computing and want to resolve their in future.

This comment has been minimized.

js/graph_spectrum_calc.js Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Mar 7, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

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

See analysis details on SonarCloud

@haslinghuis
Copy link
Member

@mituritsyn please rebase - to resolve conflicts.

@nerdCopter
Copy link
Member

@demvlad , does this fit into your #711 somehow, or is it a duplication in essence?
when i attempted to merge locally there is obvious conflict.
This PR will need closure, but unsure if a new "PWA" compatible/rebase version need to be opened or not.

@demvlad
Copy link
Contributor

demvlad commented Mar 27, 2024

@demvlad , does this fit into your #711

@nerdCopter Yes, this improvement is closed in #711.
The PR #718 completes this work.

@mituritsyn mituritsyn closed this Mar 28, 2024
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.

None yet

5 participants