qmk / qmk_firmware

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

[Bug] ymdk_np21 works unreliably after update #15400

Open gudvinr opened 2 years ago

gudvinr commented 2 years ago

I used some older version of QMK for probably a year or so and decided to upgrade recently.
Everything worked without issues before update but then suddenly I have multiple problems with QMK.

I didn't pay attention to which version was working because I didn't think I could get malfunctioning keyboard after update.

Describe the Bug

  1. When I press any configured key (e.g. C(KC_F13), C(KC_V) and both tapdance actions for ACTION_TAP_DANCE_DOUBLE(S(KC_F16), C(S(KC_F16))) are ones that I use often) it can sometimes be ignore.
    I don't see any correlation with time between keypresses in any case.
  2. Sometimes I get multiple events fired. For example. one of the keys is A(KC_F4) and it could just happily wipe out all the windows I have.

System Information

Additional information

config.h diff ``` +#define TAPPING_TOGGLE 3 +#define TAPPING_TERM 175 ```
rules.mk diff ``` -BACKLIGHT_ENABLE = yes # Enable keyboard backlight functionality -RGBLIGHT_ENABLE = yes # Enable keyboard RGB underglow +BACKLIGHT_ENABLE = no # Enable keyboard backlight functionality +RGBLIGHT_ENABLE = no # Enable keyboard RGB underglow -LAYOUTS = ortho_6x4 numpad_6x4 +LAYOUTS = ortho_6x4 +KEY_LOCK_ENABLE = yes +TAP_DANCE_ENABLE=yes ```

dmesg

I also looked to dmesg and I see messages like that:
usb 1-4.3: reset low-speed USB device number 12 using xhci_hcd

This device is indeed that keyboard:

Bus 001 Device 012: ID 594d:5021 YMDK NP21

/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/16p, 480M
    ID 1d6b:0002 Linux Foundation 2.0 root hub
    |__ Port 4: Dev 11, If 0, Class=Hub, Driver=hub/4p, 480M
        ID 05e3:0610 Genesys Logic, Inc. Hub
        |__ Port 3: Dev 12, If 1, Class=Human Interface Device, Driver=usbhid, 1.5M
            ID 594d:5021  
        |__ Port 3: Dev 12, If 0, Class=Human Interface Device, Driver=usbhid, 1.5M
            ID 594d:5021

This keyboard plugged in USB3.0 hub which is plugged into USB3.0 port. I cannot test it with 2.0 port because I don't have neither USB2.0 ports nor USB2.0 hub.

gudvinr commented 1 year ago

So I did some git bisect and found out that issue started to happen (if it is indeed caused by qmk) before 0.14.3 because 0.14.3 produces this error. Looking at the date, good release was 0.8.141

However, it's impossible to build image with util/docker_build.sh because container produces an error about missing python packages and docker images in registry are not tagged properly.

Also still happens as of 0.19.10

Flashed ps2avrGB and kb doesn't do that with this firmware, which was what they had originally.

gudvinr commented 1 year ago

I finally got some time to debug this, built qmkfm/base_container at 9fc31b63ac2ef521853438aaba577852cf4701af and found out through bisect that c66df1664497546f32662409778731143e45a552 is the first bad commit. Thus 0.10.54 is the latest working release.

@noroadsleft it looks like your commit and seems rather large

noroadsleft commented 1 year ago

I have a commitment this morning, but I should be able to investigate this in the afternoon. (It's 8:21 AM local time right now.)

gudvinr commented 1 year ago

I mean, I can wait a bit more since I was able to cope with that for more than a year, lol. It's not an urgent matter at all.

gudvinr commented 1 year ago

So, I did some more digging, since this commit looks like it was merged squashed from PR #11053, I did bisect from 0.10.54 to pre-develop-merge-nov20 and pinpoint first bad commit there: 75a18e69f9d3b6dfa470d0a7dbd78408d6a1c496 is the first bad commit

It appears to be the same issue as #11389

drashna commented 1 year ago

Considering that the atomic block commit may be the issue, does adding #define IGNORE_ATOMIC_BLOCK to your config.h fix the issue?

gudvinr commented 1 year ago

Yes, with this workaround I no longer see the issue but I don't think that this is a fix if users need to add this themselves for each board. This bug is rather obscure so it is kinda tricky to figure this out.
There should be more general approach.