qmk / qmk_firmware

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

[Bug] Iris rev4 rotary encoder sends repeated keystrokes #10739

Closed phmongeau closed 1 year ago

phmongeau commented 3 years ago

Describe the Bug

I have the following encoder update function in my keymap. If I compile and flash on the master branch, rotating the encoder by one click sends a bunch of KC_VOLU followed by KC_VOLD If I revert to the commit right before 201c5bfa5cdce723f148b6e9a276219c8f2e8613 from #7505 and recompile everything works perfectly and I get one KC_VOLU or KC_VOLD per click.

void encoder_update_user(uint8_t index, bool clockwise) {
    clockwise ? tap_code(KC_VOLU) : tap_code(KC_VOLD);
}

System Information

Additional Context

I found the problematic commit from this reddit thread so I'm not the only one with this issue: https://www.reddit.com/r/olkb/comments/eiogxt/help_rev_4_iris_rightside_slave_encoder/

But it looks like nobody reported it, unless there's already an issue, but I could find it.

fauxpark commented 3 years ago

Add #define TAP_CODE_DELAY 10 to your config.h.

phmongeau commented 3 years ago

I already tried #define TAP_CODE_DELAY 10, it didn't help

fauxpark commented 3 years ago

Try adjusting ENCODER_RESOLUTION then.

phmongeau commented 3 years ago

I tried a few different values for ENCODER_RESOLUTION and that also didn't help. I spent a couple hours yesterday debugging this and the only thing that worked was reverting 201c5bf

There's also a comment on that commit from someone having the same issue

phmongeau commented 3 years ago

I poked around a little to try to figure out what's going on. I added a if (delta) uprintf("delta %d\n", delta); in encoder_update_raw(). Turning the knob one click prints

delta -1
delta -84
delta 85

which means encoder_update_user will get called once with clockwise == false, 84 times with clockwise == true and another 84 times with clockwise == true

encoder_update_raw doesn't use ENCODER_RESOLUTION at all so I'm a bit confused about how this is supposed to work.

zvecr commented 1 year ago

This issue has been automatically closed because it has not had any recent activity. If this issue is still valid, re-open the issue and let us know.