Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Mod-tap doesn't work with KC_TRANSPARENT #23365

Open
2 tasks
VitalyArtemiev opened this issue Mar 27, 2024 · 2 comments
Open
2 tasks

[Bug] Mod-tap doesn't work with KC_TRANSPARENT #23365

VitalyArtemiev opened this issue Mar 27, 2024 · 2 comments

Comments

@VitalyArtemiev
Copy link

VitalyArtemiev commented Mar 27, 2024

Describe the Bug

According to docs, MT(mod, kc) should work with KC_TRANSPARENT.

Currently, the kc argument of MT() is limited to the Basic Keycode set

KC_TRANSPARENT is present in Basic Keycode set: https://docs.qmk.fm/#/keycodes_basic?id=special-keys

Yet, when I setup a layer with keycodes such as LCTL_T(KC_TRANSPARENT), tapping the key does not yield the key from the preceding layer.

Here is what I was trying to achieve:

#define _ KC_TRANSPARENT

const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = {
    [CMK_BASE] = LAYOUT_91_ansi(
        KC_SLEP,KC_ESC, KC_F1,       KC_F2,       KC_F3,       KC_F4,       KC_F5, KC_F6, KC_F7,       KC_F8,       KC_F9,       KC_F10,        KC_F11,  KC_F12,   KC_INS,  KC_DEL, KC_MUTE,
        MC_1,   KC_GRV, KC_1,        KC_2,        KC_3,        KC_4,        KC_5,  KC_6,  KC_7,        KC_8,        KC_9,        KC_0,          KC_MINS, KC_EQL,   KC_BSPC,         KC_PGUP,
        MC_2,   KC_TAB, KC_Q,        KC_W,        KC_F,        KC_P,        KC_B,  KC_J,  KC_L,        KC_U,        KC_Y,        KC_SCLN,       KC_LBRC, KC_RBRC,  KC_BSLS,         KC_PGDN,
        MC_3,   KC_CAPS,KC_A,        KC_R,        KC_S,        KC_T,        KC_G,  KC_M,  KC_N,        KC_E,        KC_I,        KC_O,          KC_QUOT,           KC_ENT,          KC_HOME,
        MC_4,   KC_LSFT,             KC_X,        KC_C,        KC_D,        KC_V,  KC_Z,  KC_K,        KC_H,        KC_COMM,     KC_DOT,        KC_SLSH,           KC_RSFT, KC_UP,
        MC_5,   KC_LCTL,KC_LWIN,     KC_LALT,     MO(FN1),                  KC_SPC,                    LT(FN1,KC_SPC),           KC_RALT,       MO(FN1), KC_RCTL,  KC_LEFT, KC_DOWN,KC_RGHT),

    [QWR_BASE] = LAYOUT_91_ansi(
        KC_SLEP,KC_ESC, KC_F1,       KC_F2,       KC_F3,       KC_F4,       KC_F5, KC_F6, KC_F7,       KC_F8,       KC_F9,       KC_F10,         KC_F11,  KC_F12,  KC_INS,  KC_DEL, KC_MUTE,
        MC_1,   KC_GRV, KC_1,        KC_2,        KC_3,        KC_4,        KC_5,  KC_6,  KC_7,        KC_8,        KC_9,        KC_0,           KC_MINS, KC_EQL,  KC_BSPC,         KC_PGUP,
        MC_2,   KC_TAB, KC_Q,        KC_W,        KC_E,        KC_R,        KC_T,  KC_Y,  KC_U,        KC_I,        KC_O,        KC_P,           KC_LBRC, KC_RBRC, KC_BSLS,         KC_PGDN,
        MC_3,   KC_CAPS,KC_A,        KC_S,        KC_D,        KC_F,        KC_G,  KC_H,  KC_J,        KC_K,        KC_L,        KC_SCLN,        KC_QUOT,          KC_ENT,          KC_HOME,
        MC_4,   KC_LSFT,             KC_Z,        KC_X,        KC_C,        KC_V,  KC_B,  KC_N,        KC_M,        KC_COMM,     KC_DOT,         KC_SLSH,          KC_RSFT, KC_UP,
        MC_5,   KC_LCTL,KC_LWIN,     KC_LALT,     MO(FN1),                  KC_SPC,                    LT(FN1,KC_SPC),           KC_RALT,        MO(FN1), KC_RCTL, KC_LEFT, KC_DOWN,KC_RGHT),

    [MOD_TAP] = LAYOUT_91_ansi(
        _______,  _______,  _______,  _______,  _______,   _______,  _______,  _______,  _______,  _______,  _______,  _______,  _______,    _______,  _______,  _______,  _______,
        _______,  _______,  _______,  _______,  _______,   _______,  _______,  _______,  _______,  _______,  _______,  _______,  _______,    _______,  _______,            _______,
        _______,  _______,  _______,  _______,  _______,   _______,  _______,  _______,  _______,  _______,  _______,  _______,  _______,    _______,  _______,            _______,
        _______,  _______, LCTL_T(_),LSFT_T(_), LALT_T(_), LGUI_T(_),_______,  _______,RGUI_T(_),  RALT_T(_),RSFT_T(_),RCTL_T(_),_______,    _______,  _______,
        _______,  _______,            _______,  _______,   _______,  _______,  _______,  _______,  _______,  _______,  _______,  _______,              _______,  _______,
        _______,  _______,  _______,  _______,  _______,             _______,                      _______,            _______,  _______,    _______,  _______,  _______,  _______),

    [FN1] = LAYOUT_91_ansi(
        RGB_TOG,  _______,  KC_BRID,  KC_BRIU,  KC_TASK,  KC_FLXP,  RGB_VAD,   RGB_VAI,  KC_MPRV,  KC_MPLY,  KC_MNXT,  KC_MUTE,  KC_VOLD,    KC_VOLU,  RGB_RMOD, RGB_MOD,  NK_TOGG,
        TG(1),    _______,  _______,  _______,  _______,  _______,  _______,   _______,  _______,  _______,  _______,  _______,  _______,    _______,  _______,            _______,
        TG(2),    KC_SCRL,  _______,  _______,  _______,  _______,  _______,   _______,  _______,  KC_UP,    _______,  _______,  _______,    _______,  _______,            _______,
        TG(3),    _______,  KC_ESCAPE,_______,  _______,  _______,  _______,   _______,  KC_LEFT,  KC_DOWN,  KC_RIGHT, KC_ENTER, _______,              _______,            _______,
        TG(4),    _______,            _______,  _______,  _______,  _______,   _______,  _______,  _______,  _______,  _______,  _______,              _______,  _______,
        TG(5),    _______,  _______,  _______,  _______,            _______,                       KC_BSPC,            KC_DEL,   _______,    _______,  _______,  _______,  _______),

},

This would allow toggling mod-tap behaviour by toggling the MOD_TAP layer. I am using my dip switch to set layer 0 or layer 1 as default, which are COLEMAK and QWERTY layouts respectively.

You can find full example here: https://github.com/VitalyArtemiev/qmk_firmware/blob/mt_trsp_bug/keyboards/keychron/q11/ansi_encoder/keymaps/vital/keymap.c

Expected behaviour: When tapping the A key, I expect the letter A. When holding the A key, I expect the Ctrl modifier.
Actual behaviour: When tapping the A key, nothing. Holding the A key activates Ctrl as expected. This is regardless of the selected default layer (0 or 1).

Keyboard Used

keychron/q11/ansi_encoder

Link to product page (if applicable)

https://www.keychron.com/products/keychron-q11-qmk-custom-mechanical-keyboard

Operating System

Win10

qmk doctor Output

$ qmk doctor
Ψ QMK Doctor is checking your environment.
Ψ CLI version: 1.1.5
Ψ QMK home: C:/Users/vital/qmk_firmware
Ψ Detected Windows 10 (10.0.19045).
Ψ QMK MSYS version: 1.9.0
Ψ Userspace enabled: False
Ψ Git branch: master
Ψ Repo version: 0.24.6
⚠ Git has unstashed/uncommitted changes.
Ψ - Latest master: 2024-03-27 14:43:37 +0300 (4434af48b5) -- Add game layer (without mod-tap)
Ψ - Latest upstream/master: 2024-03-27 07:30:58 +0800 (1d58530e79) -- [Keyboard] Add T75 (#23344)
Ψ - Latest upstream/develop: None
Ψ - Common ancestor with upstream/master: 2024-03-22 18:30:30 -0500 (4afbade6d1) -- Add ES_GRV to latam language-specific keycodes (#23333)
Ψ - 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: 2023-04-15 13:48:04 +0000 --  (11edb1610)
Ψ - lib/chibios-contrib: 2023-11-27 18:15:44 +0100 --  (9d7a7f90)
Ψ - 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

  • AutoHotKey (Windows)
  • Karabiner (macOS)

Other keyboard-related software installed

No response

Additional Context

No response

@sigprof
Copy link
Contributor

sigprof commented Mar 27, 2024

Unfortunately, the behavior that you expected here is not actually implemented — KC_TRANSPARENT is handled only by layer_switch_get_layer() during the keymap lookup, and does not work when embedded inside MT() or LT(). So KC_TRANSPARENT is not really a basic keycode for this kind of usage (i.e., something that could be sent directly to the host by register_code()), even though it fits into a single byte.

And internally QMK has some other definitions of what a “basic” keycode is — e.g., IS_BASIC_KEYCODE() and BASIC_KEYCODE_RANGE accept only the KC_AKC_EXSEL range, which does not even include the modifiers, even though they are also a part of the Keyboard usage page; but IS_QK_BASIC() accepts the whole 0x000xFF range — i.e., anything which fits into a byte. So the issue of what is actually a basic keycode is somewhat complicated.

You may try to implement the behavior that you want in custom code — see the Intercepting Mod-Taps section for a way to change the tap function. Although you may need to be careful here — if the layer state changes between the press and release (which may happen for a tap if quick tap is enabled, which is the default), just using the current layer state to determine which keycode to register or unregister for a tap may result in stuck keys (although if you don't care about reporting holds with the quick tap feature properly, you may just call tap_code() on press and do nothing on release, which would avoid the problem).

Implementing that behavior in core (where it would need to work in the most generic way possible) may be difficult though — the problem is that it would require two instances of the source layers cache (one which would remember the layer where the MT() or LT() keycode came from, and another which would remember the layer where the underlying tap keycode had been found). And there is an even worse problem — what to do if the keycode under MT(mods, KC_TRANSPARENT) is not a basic keycode? (The existing code for mod-taps just does register_code(action.key.code) for a tap; this won't work if the second keymap lookup replaces KC_TRANSPARENT with a non-basic keycode.)

@VitalyArtemiev
Copy link
Author

That's what I suspected. Can I submit a pull request for docs to communicate this caveat more clearly?
Thank you for your time.

VitalyArtemiev added a commit to VitalyArtemiev/qmk_firmware that referenced this issue Mar 27, 2024
Docs claim MT works with KC_TRANSPARENT hwen that is not the case.
See qmk#23365
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants