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.48k stars 37.73k forks source link

[Bug] Encoder Issues on the Planck Rev 7 #24043

Open drewlwhitney opened 2 days ago

drewlwhitney commented 2 days ago

Describe the Bug

Hello!

I've run into an issue with the current version of QMK and using rotary encoders on the Planck. I have an encoder on the top left, and when I turn it, the keyboard sends only the clockwise action at random detent intervals. I am using this encoder.

People on the Discord server have suggested the encoder is the problem. However, after going through three different encoders, all of them exhibit the same behavior. Furthermore, I can confirm that this issue is specific to the Planck. I have a Nyquist/Levinson Rev 4 that uses the exact same encoder and the exact same keymap as my Planck. As in, I literally copy/pasted the contents of the keymap file from the Planck to the Levinson, and the encoder on the Levinson works properly. I also have identical settings for the config.h and rules.mk files.

I can get the encoder to work QMK versions from before encoder abstraction was added, but encoder mappings for the Planck are not supported at that point in the commit history. Neither encoder mappings nor the user-side encoder function work for the Planck.

Specifically, this encoder works properly before the "Add encoder abstraction" commit, which is 9d9cdaa. I compiled firmware on commit 2eb9ff8 and the encoder worked properly. On 9d9cdaa, the encoder breaks.

I think this can be isolated to the Planck directory, since other keyboards work fine. I am not familiar enough with how encoders are implemented in QMK to find the issue myself, but I am happy to test any solutions. I would also appreciate if any other Planck users with encoders could test them on the current version. I've pasted my keymap below (I know my process_record_user is horrendous. There is a separate issue with the GUI key not being suppressed properly for key overrides that I'm manually fixing).

Let me know what I can do to help!

#include QMK_KEYBOARD_H

const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = {
    [0] = LAYOUT_ortho_4x12(
        KC_MPLY,          KC_Q,          KC_W,           KC_E,                  KC_R,          KC_T,         KC_Y,         KC_U,           KC_I,         KC_O,          KC_P,          KC_BSPC,
        KC_TAB,           KC_A,          KC_S,           KC_D,                  KC_F,          KC_G,         KC_H,         KC_J,           KC_K,         KC_L,          KC_SCLN,       KC_QUOT,
        KC_LSFT,          KC_Z,          KC_X,           KC_C,                  KC_V,          KC_B,         KC_N,         KC_M,           KC_COMM,      KC_DOT,        KC_SLSH,       ALT_T(KC_ESC),
        KC_LCTL,          LT(5, KC_ESC), KC_LALT,        LGUI_T(LALT(KC_SPC)),  MO(2),         KC_SPC,       KC_ENT,       MO(3),          KC_LEFT,      KC_DOWN,       KC_UP,         KC_RGHT
    ),
    [1] = LAYOUT_ortho_4x12(
        KC_MPLY,          KC_Q,          KC_W,           KC_E,                  KC_R,         KC_T,          KC_Y,         KC_U,           KC_I,         KC_O,          KC_P,          KC_BSPC,
        KC_TAB,           KC_A,          KC_S,           KC_D,                  KC_F,         KC_G,          KC_H,         KC_J,           KC_K,         KC_L,          KC_SCLN,       KC_QUOT,
        KC_LSFT,          KC_Z,          KC_X,           KC_C,                  KC_V,         KC_B,          KC_N,         KC_M,           KC_COMM,      KC_DOT,        KC_SLSH,       ALT_T(KC_ESC),
        KC_LCTL,          LT(5, KC_ESC), KC_LALT,        LGUI_T(LALT(KC_SPC)),  KC_SPC,       KC_SPC,        KC_ENT,       MO(3),          KC_LEFT,      KC_VOLD,       KC_VOLU,       KC_RGHT
    ),
    [2] = LAYOUT_ortho_4x12(
        C(A(KC_DEL)),     KC_EXLM,       KC_AT,          KC_HASH,               KC_DLR,       KC_PERC,       KC_CIRC,      KC_AMPR,        KC_ASTR,      KC_LPRN,       KC_RPRN,       KC_BSPC,
        KC_DOT,           KC_1,          KC_2,           KC_3,                  KC_4,         KC_5,          KC_6,         KC_7,           KC_8,         KC_9,          KC_0,          KC_COLN,
        SFT_T(KC_COMM),   KC_6,          KC_7,           KC_8,                  KC_9,         KC_0,          KC_PLUS,      KC_MINS,        KC_COMM,      KC_DOT,        KC_SLSH,       ALT_T(KC_ESC),
        CTL_T(KC_COLN),   LT(5, KC_ESC), KC_LALT,        LGUI_T(LALT(KC_SPC)),  KC_NO,        KC_SPC,        KC_ENT,       LT(3, KC_SPC),  KC_LEFT,      KC_DOWN,       KC_UP,         KC_RGHT
    ),
    [3] = LAYOUT_ortho_4x12(
        C(A(KC_DEL)),     KC_EXLM,       KC_AT,          KC_HASH,               KC_DLR,       KC_PERC,       KC_CIRC,      KC_AMPR,        KC_ASTR,      KC_LPRN,       KC_RPRN,       KC_BSPC,
        KC_TAB,           KC_BSLS,       KC_LBRC,        KC_RBRC,               KC_MINS,      KC_EQL,        KC_HOME,      KC_LEFT,        KC_DOWN,      KC_UP,         KC_RGHT,       KC_END,
        KC_LSFT,          KC_PIPE,       KC_LCBR,        KC_RCBR,               KC_UNDS,      KC_PLUS,       A(KC_F4),     KC_F2,          KC_COMM,      KC_RABK,       KC_F11,        ALT_T(KC_ESC),
        KC_LCTL,          LT(5, KC_ESC), KC_LALT,        LGUI_T(LALT(KC_SPC)),  MO(4),        KC_SPC,        KC_ENT,       KC_NO,          KC_LEFT,      KC_DOWN,       KC_UP,         KC_RGHT
    ),
    [4] = LAYOUT_ortho_4x12(
        C(A(KC_DEL)),     KC_EXLM,       KC_AT,          KC_HASH,               KC_DLR,       KC_PERC,       KC_CIRC,      KC_AMPR,        KC_ASTR,      KC_LPRN,       KC_RPRN,       KC_BSPC,
        KC_TAB,           KC_1,          KC_2,           KC_3,                  KC_4,         KC_5,          KC_6,         KC_7,           KC_8,         KC_9,          KC_0,          KC_COLN,
        KC_LSFT,          KC_6,          KC_7,           KC_8,                  KC_9,         KC_0,          KC_PLUS,      KC_MINS,        KC_COMM,      KC_DOT,        KC_SLSH,       ALT_T(KC_ESC),
        KC_LCTL,          LT(5, KC_ESC), KC_LALT,        LGUI_T(LALT(KC_SPC)),  KC_NO,        KC_SPC,        KC_ENT,       MO(3),          KC_LEFT,      KC_DOWN,       KC_UP,         KC_RGHT
    ),
    [5] = LAYOUT_ortho_4x12(
        KC_MUTE,          KC_F1,         KC_F2,          KC_F3,                 KC_F4,        KC_F5,         KC_F6,        KC_F7,          KC_F8,        KC_F9,         KC_F10,        KC_F11,
        KC_DEL,           KC_NO,         KC_NO,          KC_NO,                 KC_NO,        TG(1),         KC_NO,        KC_NO,          KC_NO,        KC_NO,         KC_NO,         KC_F12,
        KC_LSFT,          KC_NO,         KC_NO,          KC_NO,                 KC_NO,        KC_NO,         KC_NO,        KC_NO,          KC_NO,        KC_NO,         KC_NO,         QK_BOOT,
        KC_LCTL,          KC_NO,         KC_LALT,        LGUI_T(LALT(KC_SPC)),  MO(2),        KC_SPC,        KC_ENT,       MO(3),          KC_LEFT,      KC_DOWN,       KC_UP,         KC_RGHT
    )
};

#ifdef ENCODER_MAP_ENABLE
enum custom_keycodes {
    ENCODER_VOLU,
    ENCODER_VOLD,
    ENCODER_WH_U,
    ENCODER_WH_D,
};

const uint16_t PROGMEM encoder_map[][NUM_ENCODERS][NUM_DIRECTIONS] = {
    [0] = {ENCODER_CCW_CW(ENCODER_VOLU, ENCODER_VOLD)},
    [1] = {ENCODER_CCW_CW(ENCODER_VOLU, ENCODER_VOLD)},
    [2] = {ENCODER_CCW_CW(KC_RIGHT,     KC_LEFT     )},
    [3] = {ENCODER_CCW_CW(ENCODER_WH_D, ENCODER_WH_U)},
    [4] = {ENCODER_CCW_CW(KC_RIGHT,     KC_LEFT     )},
    [5] = {ENCODER_CCW_CW(KC_MNXT,      KC_MPRV     )},
};
#endif

extern inline void nullify_override(uint8_t mods, uint8_t mods_to_suppress, void action(uint8_t), uint8_t keycode) {
    tap_code(KC_F18);
    unregister_mods(mods_to_suppress);
    action(keycode);
    set_mods(mods);
    if (mods & ~MOD_MASK_GUI) {
        send_keyboard_report();
    }
}
extern inline void nullify_override16(uint8_t mods, uint8_t mods_to_suppress, void action(uint16_t), uint16_t keycode) {
    tap_code(KC_F18);
    unregister_mods(mods_to_suppress);
    action(keycode);
    set_mods(mods);
    if (mods & ~MOD_MASK_GUI) {
        send_keyboard_report();
    }
}

bool process_record_user(uint16_t keycode, keyrecord_t *record) {
    switch (keycode) {
        case LGUI_T(LALT(KC_SPC)):
            if (record->tap.count && record->event.pressed) {tap_code16(LALT(KC_SPC));}
            else {return true;}
            break;
        case ALT_T(KC_ESC):
            if (record->tap.count) {
                const uint8_t mods = get_mods();
                if (mods & MOD_MASK_GUI && !(mods & MOD_MASK_CS)) {nullify_override(mods, MOD_MASK_GUI, record->event.pressed ? register_code : unregister_code, KC_GRV);}
                else if (mods & MOD_MASK_SHIFT && !(mods & MOD_MASK_CG)) {
                    record->event.pressed ? register_code16(KC_TILD) : unregister_code16(KC_TILD);
                }
                else {
                    if (record->event.pressed) {return true;}
                    else {
                        unregister_code(KC_GRV);
                        unregister_code(KC_ESC);
                        unregister_weak_mods(MOD_MASK_SHIFT);
                    }
                }
            } 
            else {return true;}
            break;
        case CTL_T(KC_COLN):
            if (record->tap.count && record->event.pressed) {tap_code16(KC_COLN);} 
            else {return true;}
            break;
        case KC_END: {
            const uint8_t mods = get_mods();
            if (mods & MOD_MASK_GUI) {nullify_override16(mods, MOD_MASK_GUI, record->event.pressed ? register_code16 : unregister_code16, C(KC_END));}
            // prevent ctrl + end
            else if (mods & MOD_MASK_CTRL) {
                if (record->event.pressed) {
                    unregister_mods(MOD_MASK_CTRL);
                    register_code(KC_END);
                    set_mods(mods);
                } else {return true;}
            }
            else {
                if (record->event.pressed) {return true;}
                else {
                    unregister_code(KC_END);
                    unregister_weak_mods(MOD_MASK_CTRL);
                }
            }
            break;
        }
        case KC_HOME: {
            const uint8_t mods = get_mods();
            if (mods & MOD_MASK_GUI) {nullify_override16(mods, MOD_MASK_GUI, record->event.pressed ? register_code16 : unregister_code16, C(KC_HOME));}
            // prevent ctrl + home
            else if (mods & MOD_MASK_CTRL) {
                if (record->event.pressed) {
                    unregister_mods(MOD_MASK_CTRL);
                    register_code(KC_HOME);
                    set_mods(mods);
                } else {return true;}
            }
            else {
                if (record->event.pressed) {return true;}
                else {
                    unregister_code(KC_HOME);
                    unregister_weak_mods(MOD_MASK_CTRL);
                }
            }
            break;
        }
#ifdef ENCODER_MAP_ENABLE
        // Encoder events
        // NOTE TO SELF: You will have to manually suppress the GUI and ALT keys
        // because of separate keyup and keydown events ಥ_ಥ
        case ENCODER_VOLD: {
            if (record->event.pressed) {
                const uint8_t mods = get_mods();
                if (mods & MOD_MASK_CTRL || mods & MOD_MASK_SHIFT) {
                    tap_code(KC_LEFT);
                }
                else if (mods & MOD_MASK_GUI) {
                    register_mods(MOD_MASK_CTRL);
                    unregister_mods(MOD_MASK_GUI);        
                    tap_code(KC_MINS);
                    unregister_mods(MOD_MASK_CTRL);
                    set_mods(mods);
                }
                else if (mods & MOD_MASK_ALT) {
                    del_mods(MOD_MASK_ALT);
                    tap_code16(C(S(KC_TAB)));
                    set_mods(mods);
                }
                else {
                    tap_code(KC_VOLD);
                }                    
            }
            return false;
            break;
        }
        case ENCODER_VOLU: {
            if (record->event.pressed) {
                const uint8_t mods = get_mods();
                if (mods & MOD_MASK_CTRL || mods & MOD_MASK_SHIFT) {
                    tap_code(KC_RGHT);
                }
                else if (mods & MOD_MASK_GUI) {
                    register_mods(MOD_MASK_CTRL);
                    unregister_mods(MOD_MASK_GUI);        
                    tap_code(KC_EQL);
                    unregister_mods(MOD_MASK_CTRL);
                    set_mods(mods);
                }
                else if (mods & MOD_MASK_ALT) {
                    del_mods(MOD_MASK_ALT);
                    tap_code16(C(KC_TAB));
                    set_mods(mods);
                }
                else {
                    tap_code(KC_VOLU);
                }                    
            }
            return false;
            break;
        }                
        case ENCODER_WH_U: {
            if (record->event.pressed) {
                tap_code(KC_WH_U);
                tap_code(KC_WH_U);                    
            }
            return false;
            break;
        }
        case ENCODER_WH_D: {
            if (record->event.pressed) {
                tap_code(KC_WH_D);
                tap_code(KC_WH_D);                    
            }
            return false;
            break;
        }
#endif
        default:
            return true;
            break;
    }
    return false;
}

Keyboard Used

planck/rev7

Link to product page (if applicable)

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

Operating System

Windows 11

qmk doctor Output

Ψ QMK Doctor is checking your environment.
Ψ CLI version: 1.1.5
Ψ QMK home: C:/Users/drewl/qmk_firmware
Ψ Detected Windows 10 (10.0.22631).
Ψ QMK MSYS version: 1.9.0
Ψ Userspace enabled: False
Ψ Git branch: master
Ψ Repo version: 0.25.9
⚠ Git has unstashed/uncommitted changes.
Ψ - Latest master: 2024-07-03 22:00:53 +1000 (bc8ac86422) -- Minimum python version listing. (#23989)
Ψ - Latest upstream/master: 2024-07-03 22:00:53 +1000 (bc8ac86422) -- Minimum python version listing. (#23989)
Ψ - Latest upstream/develop: None
Ψ - Common ancestor with upstream/master: 2024-07-03 22:00:53 +1000 (bc8ac86422) -- Minimum python version listing. (#23989)
Ψ - Common ancestor with upstream/develop: None
Ψ All dependencies are installed.
Ψ Found arm-none-eabi-gcc version 12.2.0
Ψ Found avr-gcc version 12.2.0
⚠ We do not recommend avr-gcc newer than 8. Downgrading to 8.x is recommended.
Ψ Found avrdude version 7.0
Ψ Found dfu-programmer version 1.1.0
Ψ Found dfu-util version 0.11
Ψ 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

QMK Toolbox

Additional Context

I have #define ENCODER_MAP_ENABLE in config.h.

tzarc commented 2 days ago

Can you try https://github.com/qmk/qmk_firmware/pull/23967 please?

drewlwhitney commented 2 days ago

Yes absolutely! My previous solution for gaining access to older versions has been downloading a zip and then sort of stitching it together with the compiler. Is there a Git command that lets me get a specific commit version?

tzarc commented 2 days ago

Can try:

# assuming QMK is your upstream remote as per the docs
git fetch -f upstream refs/pull/23967/head:pr/23967
git merge --no-commit --squash pr/23967
drewlwhitney commented 2 days ago

I'm currently getting this issue:

error: Your local changes to the following files would be overwritten by merge: keyboards/planck/rev7/keymaps/default/config.h keyboards/planck/rev7/keymaps/default/keymap.c keyboards/planck/rev7/matrix.c keyboards/planck/rev7/readme.md

:279: trailing whitespace. If you enable `ENCODER_MAP_ENABLE`, defined `const uint16_t PROGMEM encoder_map[][NUM_ENCODERS][NUM_DIRECTIONS]` and configure your keycode. And, If you enable `ENCODER_MAP_ENABLE`, does not use `encoder_update_user` directly. warning: 1 line adds whitespace errors. Merge with strategy ort failed.
tzarc commented 2 days ago

That seems to indicate you've modified those files locally? I've just attempted it with current master and it merged fine.

drewlwhitney commented 2 days ago

Oops! I didn't realize I also needed to run git commit. My Git noobiness is showing.

However, yippee! That version works perfectly! The encoder reads correctly with a resolution of two, and the encoder map is being used. It's even working with mod-tap keys, so I can use my mod-tap Alt/Esc key to cycle through tabs with the encoder now.

I no longer have to have two separate qmk versions on my computer! Thank you so much tzarc! If there are any other tests you would like me to run, I will absolutely do them.

Have a wonderful night!