diff options
| -rw-r--r-- | tests/basic/keymap.c | 3 | ||||
| -rw-r--r-- | tests/basic/test_keypress.cpp | 118 | ||||
| -rw-r--r-- | tests/test_common/test_fixture.cpp | 7 | ||||
| -rw-r--r-- | tmk_core/common/action.c | 7 | ||||
| -rw-r--r-- | tmk_core/common/report.c | 26 | ||||
| -rw-r--r-- | tmk_core/common/report.h | 2 |
6 files changed, 158 insertions, 5 deletions
diff --git a/tests/basic/keymap.c b/tests/basic/keymap.c index 02b3cc961a..951ce8e0cc 100644 --- a/tests/basic/keymap.c +++ b/tests/basic/keymap.c @@ -28,7 +28,7 @@ const uint16_t PROGMEM { // 0 1 2 3 4 5 6 7 8 9 {KC_A, KC_B, KC_NO, KC_LSFT, KC_RSFT, KC_LCTL, COMBO1, SFT_T(KC_P), M(0), KC_NO}, - {KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO}, + {KC_EQL, KC_PLUS, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO}, {KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO}, {KC_C, KC_D, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO}, }, @@ -43,3 +43,4 @@ const macro_t *action_get_macro(keyrecord_t *record, uint8_t id, uint8_t opt) { } return MACRO_NONE; }; + diff --git a/tests/basic/test_keypress.cpp b/tests/basic/test_keypress.cpp index e5247911cb..551458a0d9 100644 --- a/tests/basic/test_keypress.cpp +++ b/tests/basic/test_keypress.cpp @@ -18,6 +18,7 @@ using testing::_; using testing::Return; +using testing::InSequence; class KeyPress : public TestFixture {}; @@ -121,4 +122,119 @@ TEST_F(KeyPress, RightShiftLeftControlAndCharWithTheSameKey) { EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_RSFT, KC_RCTRL))); EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())); keyboard_task(); -}
\ No newline at end of file +} + +TEST_F(KeyPress, PressPlusEqualReleaseBeforePress) { + TestDriver driver; + InSequence s; + + press_key(1, 1); // KC_PLUS + 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); + + release_key(1, 1); // KC_PLUS + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT))); + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + press_key(0, 1); // KC_EQL + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_EQL))); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + release_key(0, 1); // KC_EQL + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); +} + +TEST_F(KeyPress, PressPlusEqualDontReleaseBeforePress) { + TestDriver driver; + InSequence s; + + press_key(1, 1); // KC_PLUS + 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); + + 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); + + release_key(1, 1); //KC_PLS + // BUG: Should really still return KC_EQL, but this is fine too + // It's also called twice for some reason + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(2); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + release_key(0, 1); // KC_EQL + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); +} + +TEST_F(KeyPress, PressEqualPlusReleaseBeforePress) { + TestDriver driver; + InSequence s; + + press_key(0, 1); // KC_EQL + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_EQL))); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + release_key(0, 1); // KQ_EQL + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + press_key(1, 1); // KC_PLUS + 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); + + release_key(1, 1); // KC_PLUS + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT))); + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); +} + +TEST_F(KeyPress, PressEqualPlusDontReleaseBeforePress) { + TestDriver driver; + InSequence s; + + press_key(0, 1); // KC_EQL + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_EQL))); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + press_key(1, 1); // KC_PLUS + // 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); + + release_key(0, 1); //KC_EQL + // I guess it's fine to still report shift here + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT))); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + release_key(1, 1); // KC_PLUS + // This report is not needed + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT))); + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); +} diff --git a/tests/test_common/test_fixture.cpp b/tests/test_common/test_fixture.cpp index 8caf1fca4a..20ed838eb6 100644 --- a/tests/test_common/test_fixture.cpp +++ b/tests/test_common/test_fixture.cpp @@ -32,14 +32,15 @@ TestFixture::TestFixture() {} TestFixture::~TestFixture() { TestDriver driver; - layer_clear(); - clear_all_keys(); // Run for a while to make sure all keys are completely released EXPECT_CALL(driver, send_keyboard_mock(_)).Times(AnyNumber()); + layer_clear(); + clear_all_keys(); idle_for(TAPPING_TERM + 10); testing::Mock::VerifyAndClearExpectations(&driver); // Verify that the matrix really is cleared - EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(Between(0, 1)); + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(0); + idle_for(TAPPING_TERM + 10); } void TestFixture::run_one_scan_loop() { diff --git a/tmk_core/common/action.c b/tmk_core/common/action.c index 0ec4a43eee..2deafd465f 100644 --- a/tmk_core/common/action.c +++ b/tmk_core/common/action.c @@ -754,6 +754,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 f4758b48ec..f4cdca7281 100644 --- a/tmk_core/common/report.c +++ b/tmk_core/common/report.c @@ -68,6 +68,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<<(key&7); + } else { + return false; + } + } +#endif + for (int i=0; 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 ecd5da89a0..645e016121 100644 --- a/tmk_core/common/report.h +++ b/tmk_core/common/report.h @@ -19,6 +19,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>. #define REPORT_H #include <stdint.h> +#include <stdbool.h> #include "keycode.h" /* report id */ @@ -236,6 +237,7 @@ static inline uint16_t KEYCODE2CONSUMER(uint8_t key) { 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); |