tmk / tmk_keyboard

Keyboard firmwares for Atmel AVR and Cortex-M
4k stars 1.71k forks source link

Modifier keys on higher layers are not released if the function key is released first #397

Closed andrewcchen closed 7 years ago

andrewcchen commented 8 years ago

With a function layer activated with ACTION_LAYER_MOMENTARY bound to a fn key, if the fn key is released before the modifier key is (physically) released, the modifier key is never marked as released, and continue to be registered as pressed to the computer.

This bug does not occur if the key is a normal key (tested with left direction key).

To reproduce:

  1. Bind a layer with ACTION_LAYER_MOMENTARY to a fn key.
  2. Map a key on that layer to a modifier key (tested with RSHIFT, RCTRL, RALT, RGUI)
  3. Press (hold) fn key + modifier key
  4. Release fn key and then release modifier key
  5. Observe that the modifier key is still registered as pressed

Version: git master

Board: gh60 (rev. chn) (only difference from rev a/b is different gpio pins for matrix)

Config: lufa build BOOTMAGIC_ENABLE = yes CONSOLE_ENABLE = yes COMMAND_ENABLE = yes USB_6KRO_ENABLE = yes BACKLIGHT_ENABLE = yes

andrewcchen commented 8 years ago

This might be the cause:

In action_layer.c:

static void layer_state_set(uint32_t state) {
    // ...
    clear_keyboard_but_mods(); // To avoid stuck keys
}

Modifier keys are not cleared without checking whether they are pressed on a higher level or not.

tmk commented 8 years ago

You might want to check this entry on WIki. https://github.com/tmk/tmk_keyboard/wiki/FAQ-Keymap#modifierlayer-stuck

andrewcchen commented 8 years ago

Thanks, I didn't see that before.

What I understood from those links (especially #248), is that a layout like below just won't work properly, correct?

keymaps[][] = {
    [0] = KEYMAP(FN0, UP  ),
    [1] = KEYMAP(TRNS,RSFT),
};
fn_actions[] = {
    [0] = ACTION_LAYER_MOMENTARY(1),
};

But I thought if process_action() saved the action code that was pressed for every physical key, and released the saved version when the physical key is released, the problem wouldn't occur. An implementation like this would fix it for this case, however I'm not sure how it would interact with other action types. Could you have a look at it and evaluate if it could be a viable solution?

tmk commented 7 years ago

Fixed at ba2883f