-
-
Notifications
You must be signed in to change notification settings - Fork 39.6k
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
Enhancement of WPM feature #11727
Enhancement of WPM feature #11727
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for these great updates!
88e8974
to
490dab3
Compare
Sorry about that, GitHub decided to delete the |
Thank you for your contribution! |
And additional code cleanup
490dab3
to
533dc62
Compare
@drashna Since this patch, pressing multiple (counted) keys simultaneously makes the WPM skyrocket, often even overflowing the uint8_t it's stored in. As far as I've been able to test, I can't seem to reproduce that with the last version of wpm.c before this. The reason seems to be that, previously, the old calculation of current_wpm += ((uint8_t)(60000 / timer_elapsed(wpm_timer) / WPM_ESTIMATED_WORD_SIZE) - current_wpm) * wpm_smoothing; Not sure if the potential overflow there was intentional or not... That said, I'm not entirely sure which of the algorithms is the most correct, but I never really found either of them to be far off other measurements, except for this difference. Edit: This might be a better solution, clamping the value instead of just relying on a random-ish value after uint8_t overflow: uint16_t latest_wpm = 60000 / timer_elapsed(wpm_timer) / WPM_ESTIMATED_WORD_SIZE;
if (latest_wpm > UINT8_MAX) {
latest_wpm = UINT8_MAX;
}
current_wpm += (latest_wpm - current_wpm) * wpm_smoothing; |
Types of Changes
Checklist
And additional code cleanup