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

feat: use console.time for timing #2408

Merged
merged 2 commits into from Feb 14, 2021

Conversation

chmelevskij
Copy link
Member

Fixes #2406

@McGiverGim used console.time stuff.

@TheIsotopes could you verify it works?

@TheIsotopes
Copy link
Contributor

yes, works again ... thx 👍

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.

The microtime function is only used twice. One where you have fixed with this, the other here:

ConfigInserter.prototype.insertConfig = function (firmware, input) {
const timeParsingStart = microtime(); // track time
const customDefaultsArea = getCustomDefaultsArea(firmware);
if (!customDefaultsArea || customDefaultsArea.endAddress - customDefaultsArea.startAddress === 0) {
return false;
} else if (input.length >= customDefaultsArea.endAddress - customDefaultsArea.startAddress) {
throw new Error(`Custom defaults area too small (${customDefaultsArea.endAddress - customDefaultsArea.startAddress} bytes), ${input.length + 1} bytes needed.`);
}
generateData(firmware, input, customDefaultsArea.startAddress);
console.log(`Custom defaults inserted in: ${microtime() - timeParsingStart.toFixed(4)} seconds.`);
return true;
}

I think is better to remove the function at all and fix the problem in both places.

@mikeller mikeller added this to the 10.8.0 milestone Feb 9, 2021
@limonspb
Copy link
Member

fixes the issue for me too

Remove `microtime` and console log in favour of `console.time`
@sonarcloud
Copy link

sonarcloud bot commented Feb 14, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.1% 0.1% Duplication

@mikeller mikeller merged commit e8f5e4e into betaflight:master Feb 14, 2021
@asizon
Copy link
Member

asizon commented Feb 14, 2021

Here is some wrong:
image

@TheIsotopes
Copy link
Contributor

@asizon #2414 will fix this

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.

Configurator does not load hex files
6 participants