From 8fe35afdd6dd1b6f9691090eb1d1ab4d6b8f23c8 Mon Sep 17 00:00:00 2001 From: Fred Sundvik Date: Sat, 7 Apr 2018 22:50:46 +0300 Subject: [PATCH] Fix #1708 When two keys that use the same keycode, but different modifiers were pressed at the same time, the second keypress wasn't registered. This is fixed by forcing a key release when we detect a new press for the same keycode. --- tests/basic/test_keypress.cpp | 10 ++++++---- tmk_core/common/action.c | 7 +++++++ tmk_core/common/report.c | 26 ++++++++++++++++++++++++++ tmk_core/common/report.h | 2 ++ 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/tests/basic/test_keypress.cpp b/tests/basic/test_keypress.cpp index a1f6970e64c1..e98661517318 100644 --- a/tests/basic/test_keypress.cpp +++ b/tests/basic/test_keypress.cpp @@ -162,6 +162,7 @@ TEST_F(KeyPress, PressPlusEqualDontReleaseBeforePress) { testing::Mock::VerifyAndClearExpectations(&driver); press_key(0, 1); // KC_EQL + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())); EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_EQL))); run_one_scan_loop(); testing::Mock::VerifyAndClearExpectations(&driver); @@ -194,7 +195,6 @@ TEST_F(KeyPress, PressEqualPlusReleaseBeforePress) { testing::Mock::VerifyAndClearExpectations(&driver); press_key(1, 1); // KC_PLUS - // BUG: Should release KC_EQL first EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT))); EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT, KC_EQL))); run_one_scan_loop(); @@ -217,9 +217,11 @@ TEST_F(KeyPress, PressEqualPlusDontReleaseBeforePress) { testing::Mock::VerifyAndClearExpectations(&driver); press_key(1, 1); // KC_PLUS - // BUG: Should relase KQ_EQL first - // BUG: Sends the same thing twice - EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT, KC_EQL))).Times(2); + // BUG: The sequence is a bit strange, but it works, the end result is that + // KC_PLUS is sent + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT, KC_EQL))); + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT))); + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT, KC_EQL))); run_one_scan_loop(); testing::Mock::VerifyAndClearExpectations(&driver); diff --git a/tmk_core/common/action.c b/tmk_core/common/action.c index fff834791552..7e1a6cef3326 100644 --- a/tmk_core/common/action.c +++ b/tmk_core/common/action.c @@ -759,6 +759,13 @@ void register_code(uint8_t code) */ #endif { + // Force a new key press if the key is already pressed + // without this, keys with the same keycode, but different + // modifiers will be reported incorrectly, see issue #1708 + if (is_key_pressed(keyboard_report, code)) { + del_key(code); + send_keyboard_report(); + } add_key(code); send_keyboard_report(); } diff --git a/tmk_core/common/report.c b/tmk_core/common/report.c index eb3b44312ffd..4018b13754e7 100644 --- a/tmk_core/common/report.c +++ b/tmk_core/common/report.c @@ -62,6 +62,32 @@ uint8_t get_first_key(report_keyboard_t* keyboard_report) #endif } +/** \brief Checks if a key is pressed in the report + * + * Returns true if the keyboard_report reports that the key is pressed, otherwise false + * Note: The function doesn't support modifers currently, and it returns false for KC_NO + */ +bool is_key_pressed(report_keyboard_t* keyboard_report, uint8_t key) { + if (key == KC_NO) { + return false; + } +#ifdef NKRO_ENABLE + if (keyboard_protocol && keymap_config.nkro) { + if ((key>>3) < KEYBOARD_REPORT_BITS) { + return keyboard_report->nkro.bits[key>>3] & 1<<(code&7); + } else { + return false; + } + } +#endif + for (int i; i < KEYBOARD_REPORT_KEYS; i++) { + if (keyboard_report->keys[i] == key) { + return true; + } + } + return false; +} + /** \brief add key byte * * FIXME: Needs doc diff --git a/tmk_core/common/report.h b/tmk_core/common/report.h index 6c27eb9dc650..01fba3028656 100644 --- a/tmk_core/common/report.h +++ b/tmk_core/common/report.h @@ -19,6 +19,7 @@ along with this program. If not, see . #define REPORT_H #include +#include #include "keycode.h" @@ -174,6 +175,7 @@ typedef struct { uint8_t has_anykey(report_keyboard_t* keyboard_report); uint8_t get_first_key(report_keyboard_t* keyboard_report); +bool is_key_pressed(report_keyboard_t* keyboard_report, uint8_t key); void add_key_byte(report_keyboard_t* keyboard_report, uint8_t code); void del_key_byte(report_keyboard_t* keyboard_report, uint8_t code);