Fix qmk#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

@@ -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);
 

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();
         }

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

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"
 
 
@@ -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);