From 0391801267799dfe233cd0962357a0bf332c3908 Mon Sep 17 00:00:00 2001 From: vectorstorm Date: Mon, 27 Dec 2021 11:52:56 +1100 Subject: [PATCH] Fixes potential wpm sampling overflow, along with code comment fixes (#15277) Co-authored-by: Trevor Powell --- docs/feature_wpm.md | 2 +- quantum/wpm.c | 75 ++++++++++++++++++++++++++------------------- quantum/wpm.h | 2 +- 3 files changed, 45 insertions(+), 34 deletions(-) diff --git a/docs/feature_wpm.md b/docs/feature_wpm.md index 87145c97e3..9f98ef8336 100644 --- a/docs/feature_wpm.md +++ b/docs/feature_wpm.md @@ -16,7 +16,7 @@ For split keyboards using soft serial, the computed WPM score will be available | `WPM_ALLOW_COUNT_REGRESSION` | _Not defined_ | If defined allows the WPM to be decreased when hitting Delete or Backspace | | `WPM_UNFILTERED` | _Not defined_ | If undefined (the default), WPM values will be smoothed to avoid sudden changes in value | | `WPM_SAMPLE_SECONDS` | `5` | This defines how many seconds of typing to average, when calculating WPM | -| `WPM_SAMPLE_PERIODS` | `50` | This defines how many sampling periods to use when calculating WPM | +| `WPM_SAMPLE_PERIODS` | `25` | This defines how many sampling periods to use when calculating WPM | | `WPM_LAUNCH_CONTROL` | _Not defined_ | If defined, WPM values will be calculated using partial buffers when typing begins | 'WPM_UNFILTERED' is potentially useful if you're filtering data in some other way (and also because it reduces the code required for the WPM feature), or if reducing measurement latency to a minimum is important for you. diff --git a/quantum/wpm.c b/quantum/wpm.c index 925e2c416e..62d4128b8e 100644 --- a/quantum/wpm.c +++ b/quantum/wpm.c @@ -22,33 +22,37 @@ // WPM Stuff static uint8_t current_wpm = 0; static uint32_t wpm_timer = 0; -#ifndef WPM_UNFILTERED -static uint32_t smoothing_timer = 0; -#endif /* The WPM calculation works by specifying a certain number of 'periods' inside * a ring buffer, and we count the number of keypresses which occur in each of * those periods. Then to calculate WPM, we add up all of the keypresses in * the whole ring buffer, divide by the number of keypresses in a 'word', and - * then adjust for how much time is captured by our ring buffer. Right now - * the ring buffer is hardcoded below to be six half-second periods, accounting - * for a total WPM sampling period of up to three seconds of typing. + * then adjust for how much time is captured by our ring buffer. The size + * of the ring buffer can be configured using the keymap configuration + * value `WPM_SAMPLE_PERIODS`. * - * Whenever our WPM drops to absolute zero due to no typing occurring within - * any contiguous three seconds, we reset and start measuring fresh, - * which lets our WPM immediately reach the correct value even before a full - * three second sampling buffer has been filled. */ #define MAX_PERIODS (WPM_SAMPLE_PERIODS) #define PERIOD_DURATION (1000 * WPM_SAMPLE_SECONDS / MAX_PERIODS) -#define LATENCY (100) -static int8_t period_presses[MAX_PERIODS] = {0}; + +static int16_t period_presses[MAX_PERIODS] = {0}; static uint8_t current_period = 0; static uint8_t periods = 1; #if !defined(WPM_UNFILTERED) -static uint8_t prev_wpm = 0; -static uint8_t next_wpm = 0; +/* LATENCY is used as part of filtering, and controls how quickly the reported + * WPM trails behind our actual instantaneous measured WPM value, and is + * defined in milliseconds. So for LATENCY == 100, the displayed WPM is + * smoothed out over periods of 0.1 seconds. This results in a nice, + * smoothly-moving reported WPM value which nevertheless is never more than + * 0.1 seconds behind the typist's actual current WPM. + * + * LATENCY is not used if WPM_UNFILTERED is defined. + */ +# define LATENCY (100) +static uint32_t smoothing_timer = 0; +static uint8_t prev_wpm = 0; +static uint8_t next_wpm = 0; #endif void set_current_wpm(uint8_t new_wpm) { current_wpm = new_wpm; } @@ -71,7 +75,7 @@ __attribute__((weak)) bool wpm_keycode_user(uint16_t keycode) { return false; } -#ifdef WPM_ALLOW_COUNT_REGRESSION +#if defined(WPM_ALLOW_COUNT_REGRESSION) __attribute__((weak)) uint8_t wpm_regress_count(uint16_t keycode) { bool weak_modded = (keycode >= QK_LCTL && keycode < QK_LSFT) || (keycode >= QK_RCTL && keycode < QK_RSFT); @@ -95,12 +99,12 @@ __attribute__((weak)) uint8_t wpm_regress_count(uint16_t keycode) { // Outside 'raw' mode we smooth results over time. void update_wpm(uint16_t keycode) { - if (wpm_keycode(keycode)) { + if (wpm_keycode(keycode) && period_presses[current_period] < INT16_MAX) { period_presses[current_period]++; } -#ifdef WPM_ALLOW_COUNT_REGRESSION +#if defined(WPM_ALLOW_COUNT_REGRESSION) uint8_t regress = wpm_regress_count(keycode); - if (regress) { + if (regress && period_presses[current_period] > INT16_MIN) { period_presses[current_period]--; } #endif @@ -116,32 +120,41 @@ void decay_wpm(void) { } int32_t elapsed = timer_elapsed32(wpm_timer); uint32_t duration = (((periods)*PERIOD_DURATION) + elapsed); - uint32_t wpm_now = (60000 * presses) / (duration * WPM_ESTIMATED_WORD_SIZE); - wpm_now = (wpm_now > 240) ? 240 : wpm_now; + int32_t wpm_now = (60000 * presses) / (duration * WPM_ESTIMATED_WORD_SIZE); + + if (wpm_now < 0) // set some reasonable WPM measurement limits + wpm_now = 0; + if (wpm_now > 240) wpm_now = 240; if (elapsed > PERIOD_DURATION) { current_period = (current_period + 1) % MAX_PERIODS; period_presses[current_period] = 0; periods = (periods < MAX_PERIODS - 1) ? periods + 1 : MAX_PERIODS - 1; elapsed = 0; - /* if (wpm_timer == 0) { */ - wpm_timer = timer_read32(); - /* } else { */ - /* wpm_timer += PERIOD_DURATION; */ - /* } */ + wpm_timer = timer_read32(); } if (presses < 2) // don't guess high WPM based on a single keypress. wpm_now = 0; -#if defined WPM_LAUNCH_CONTROL +#if defined(WPM_LAUNCH_CONTROL) + /* + * If the `WPM_LAUNCH_CONTROL` option is enabled, then whenever our WPM + * drops to absolute zero due to no typing occurring within our sample + * ring buffer, we reset and start measuring fresh, which lets our WPM + * immediately reach the correct value even before a full sampling buffer + * has been filled. + */ if (presses == 0) { - current_period = 0; - periods = 0; - wpm_now = 0; + current_period = 0; + periods = 0; + wpm_now = 0; + period_presses[0] = 0; } #endif // WPM_LAUNCH_CONTROL -#ifndef WPM_UNFILTERED +#if defined(WPM_UNFILTERED) + current_wpm = wpm_now; +#else int32_t latency = timer_elapsed32(smoothing_timer); if (latency > LATENCY) { smoothing_timer = timer_read32(); @@ -150,7 +163,5 @@ void decay_wpm(void) { } current_wpm = prev_wpm + (latency * ((int)next_wpm - (int)prev_wpm) / LATENCY); -#else - current_wpm = wpm_now; #endif } diff --git a/quantum/wpm.h b/quantum/wpm.h index c8e7d26684..305d75b450 100644 --- a/quantum/wpm.h +++ b/quantum/wpm.h @@ -26,7 +26,7 @@ # define WPM_SAMPLE_SECONDS 5 #endif #ifndef WPM_SAMPLE_PERIODS -# define WPM_SAMPLE_PERIODS 50 +# define WPM_SAMPLE_PERIODS 25 #endif bool wpm_keycode(uint16_t keycode);