search

qmk / qmk_firmware

Open-source keyboard firmware for Atmel AVR and Arm USB families
https://qmk.fm
GNU General Public License v2.0
17.47k stars 37.7k forks source link

[Bug] NKRO distorts keymap on Drop CTRL board #23939

Open Izumemori opened 2 weeks ago

Izumemori commented 2 weeks ago

Describe the Bug

When enabling NKRO (via the QK_MAGIC_TOGGLE_NKRO or QK_MAGIC_NKRO_ON Magic Keycode) the keyboard keymap gets distorted. For instance W types 5, X types K, \ toggles Caps lock (even though I have no Caps lock in my actual keymap!). Disabling NKRO brings the keymap back to normal but C, I, and Backspace stop functioning and everything else is typed as if caps lock is enabled while not actually enabled (Pressing shift doesn't do anything). Only a power cycle of the keyboard fixes this, as long as NKRO is off or not written to EEPROM before the power cycle.

Keyboard Used

massdrop/ctrl

Link to product page (if applicable)

https://drop.com/buy/drop-ctrl-mechanical-keyboard

Operating System

Arch Linux 64bit & NixOS 24.11 unstable

qmk doctor Output

Ψ QMK Doctor is checking your environment.
Ψ CLI version: 1.1.5
Ψ QMK home: /workspaces/qmk_firmware
Ψ Detected Linux (Debian GNU/Linux 11 (bullseye)).
⚠ Missing or outdated udev rules for 'atmel-dfu' boards. Run 'sudo cp /workspaces/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'kiibohd' boards. Run 'sudo cp /workspaces/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'stm32-dfu' boards. Run 'sudo cp /workspaces/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'apm32-dfu' boards. Run 'sudo cp /workspaces/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'gd32v-dfu' boards. Run 'sudo cp /workspaces/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'wb32-dfu' boards. Run 'sudo cp /workspaces/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'bootloadhid' boards. Run 'sudo cp /workspaces/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'usbasploader' boards. Run 'sudo cp /workspaces/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'usbtinyisp' boards. Run 'sudo cp /workspaces/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'md-boot' boards. Run 'sudo cp /workspaces/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'caterina' boards. Run 'sudo cp /workspaces/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'hid-bootloader' boards. Run 'sudo cp /workspaces/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
Ψ Testing userspace candidate: /workspaces/qmk_userspace -- Valid `qmk.json`
Ψ QMK userspace: /workspaces/qmk_userspace
Ψ Userspace enabled: True
Ψ Git branch: master
Ψ Repo version: 0.25.8
⚠ The official repository does not seem to be configured as git remote "upstream".
Ψ All dependencies are installed.
Ψ Found arm-none-eabi-gcc version 8.3.1
Ψ Found avr-gcc version 8.3.0
Ψ Found avrdude version 6.3-20171130
Ψ Found dfu-programmer version 0.6.1
Ψ Found dfu-util version 0.9
Ψ Submodules are up to date.
Ψ Submodule status:
Ψ - lib/chibios: 2024-02-17 19:20:06 +0000 --  (be44b3305)
Ψ - lib/chibios-contrib: 2024-04-03 20:39:24 +0800 --  (77cb0a4f)
Ψ - lib/googletest: 2021-06-11 06:37:43 -0700 --  (e2239ee6)
Ψ - lib/lufa: 2022-08-26 12:09:55 +1000 --  (549b97320)
Ψ - lib/vusb: 2022-06-13 09:18:17 +1000 --  (819dbc1)
Ψ - lib/printf: 2022-06-29 23:59:58 +0300 --  (c2e3b4e)
Ψ - lib/pico-sdk: 2023-02-12 20:19:37 +0100 --  (a3398d8)
Ψ - lib/lvgl: 2022-04-11 04:44:53 -0600 --  (e19410f8)
Ψ QMK is ready to go, but minor problems were found

Is AutoHotKey / Karabiner installed

Other keyboard-related software installed

None

Additional Context

I have recently upgraded my keyboards firmware from a 3 year old version (fd54992e1f65713a93115a0a1737e97d3e2d8522) to the latest commit. Everything worked fine with the old firmware.

My config is at https://github.com/Izumemori/qmk_userspace, but it also happens with the default keymap at commit 751fbd7.

The udev rules from the qmk doctor output shouldn't matter since I'm developing in a dev container.

sigprof commented 2 weeks ago

This was probably broken by #22267 — apparently the NKRO implementation for arm_atsam uses a completely separate HID interface and therefore does not use a report ID byte for NKRO, but the new code always allocates uint8_t report_id; in report_nkro_t.

The ATSAM case was apparently the only one where NKRO_SHARED_EP was not defined in the old code (and apparently the same thing had been done with MOUSE_SHARED_EP, so the mouse emulation might also be broken on that platform).

#define NKRO_SHARED_EP
/* key report size(NKRO or boot mode) */
#if defined(NKRO_ENABLE)
#    if defined(PROTOCOL_LUFA) || defined(PROTOCOL_CHIBIOS)
#        include "protocol/usb_descriptor.h"
#        define NKRO_REPORT_BITS (SHARED_EPSIZE - 2)
#    elif defined(PROTOCOL_ARM_ATSAM)
#        include "protocol/arm_atsam/usb/udi_device_epsize.h"
#        define NKRO_REPORT_BITS (NKRO_EPSIZE - 1)
#        undef NKRO_SHARED_EP
#        undef MOUSE_SHARED_EP
#    else
#        error "NKRO not supported with this protocol"
#    endif
#endif
Izumemori commented 2 weeks ago

This seems to be what caused it. My local changes are:

diff --git a/tmk_core/protocol/host.c b/tmk_core/protocol/host.c
index 732fbdc37d..d5d56fcec7 100644
--- a/tmk_core/protocol/host.c
+++ b/tmk_core/protocol/host.c
@@ -97,7 +97,9 @@ void host_keyboard_send(report_keyboard_t *report) {

 void host_nkro_send(report_nkro_t *report) {
     if (!driver) return;
+#ifdef NKRO_SHARED_EP
     report->report_id = REPORT_ID_NKRO;
+#endif
     (*driver->send_nkro)(report);

     if (debug_keyboard) {
diff --git a/tmk_core/protocol/report.h b/tmk_core/protocol/report.h
index 0e4f6e9def..03acb3799e 100644
--- a/tmk_core/protocol/report.h
+++ b/tmk_core/protocol/report.h
@@ -133,8 +133,21 @@ enum desktop_usages {
 };

 // clang-format on
-
-#define NKRO_REPORT_BITS 30
+#if defined(PROTOCOL_ARM_ATSAM)
+#    include "protocol/arm_atsam/usb/udi_device_epsize.h"
+#    define NKRO_REPORT_BITS (NKRO_EPSIZE - 1)
+#    undef NKRO_SHARED_EP
+#    undef MOUSE_SHARED_EP
+#else
+#    define NKRO_REPORT_BITS 30
+#endif

 #ifdef KEYBOARD_SHARED_EP
 #    define KEYBOARD_REPORT_SIZE 9
@@ -178,7 +191,9 @@ typedef struct {
 } PACKED report_keyboard_t;

 typedef struct {
+#ifdef NKRO_SHARED_EP
     uint8_t report_id;
+#endif
     uint8_t mods;
     uint8_t bits[NKRO_REPORT_BITS];
 } PACKED report_nkro_t;

With that at least NKRO works again, have not tried mouse emulation yet.

But since host_mouse_send already respects MOUSE_SHARED_EP, I don't see why it shouldn't work https://github.com/qmk/qmk_firmware/blob/4864d5afca09cbd4b0bfc7e7cef505ad602b0c9c/tmk_core/protocol/host.c#L112-L130