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] Incompatibility Between ALT_REPEAT and sm_td Mappings #27

Open
chuan2984 opened this issue Oct 19, 2024 · 2 comments
Open

[Bug] Incompatibility Between ALT_REPEAT and sm_td Mappings #27

chuan2984 opened this issue Oct 19, 2024 · 2 comments

Comments

@chuan2984
Copy link

Problem Summary

In my QMK setup, I have the REPEAT key feature working, but there’s an issue when trying to use it with ALT_REPEAT for certain keys. Specifically, the ALT_REPEAT function fails when used with keys defined using sm_td mappings, such as a, k, and d. However, ALT_REPEAT works as expected with other keys.

Here's the setup for my mod-tap keys, structured similarly to a homerow mod configuration:

void on_smtd_action(uint16_t keycode, smtd_action action, uint8_t tap_count) {
    switch (keycode) {
        SMTD_MT(CKC_A, KC_A, KC_LEFT_GUI)
        SMTD_MT(CKC_S, KC_S, KC_LEFT_ALT)
        SMTD_MT(CKC_D, KC_D, KC_LSFT)
        SMTD_MT(CKC_F, KC_F, KC_LEFT_CTRL)
        SMTD_MT(CKC_J, KC_J, KC_RIGHT_CTRL)
        SMTD_MT(CKC_K, KC_K, KC_RIGHT_SHIFT)
        SMTD_MT(CKC_L, KC_L, KC_LEFT_ALT)
        SMTD_MT(CKC_SCLN, KC_SCLN, KC_RIGHT_GUI)
    }
}

Problem Behavior

When I attempt to execute a command like Ctrl+D by holding down the CKC_F (which acts as Ctrl) and pressing d, followed by the ALT_REPEAT key, nothing happens. However, this combination works with keys that aren't defined using sm_td. For example, Ctrl+U followed by ALT_REPEAT correctly repeats Ctrl+D.

After enabling QMK debugging and adding print statements in process_record_user, I observed that when holding down CKC_F and tapping a, k, or d (which are mapped using sm_td), the print statements aren't triggered. This suggests that process_smtd(keycode, record) is returning false. However, tapping other non-sm_td keys correctly triggers the print statements, implying that process_smtd returns true for those keys.

Here’s my process_record_user function:

static uint16_t last_two_keys[2] = {KC_NO, KC_NO};
static uint32_t last_keypress_timer = 0;

bool process_record_user(uint16_t keycode, keyrecord_t *record) {
    if (!process_smtd(keycode, record)) {
        return false;
    }

    if (record->event.pressed) {
        uint32_t time_elapsed = timer_elapsed32(last_keypress_timer);
        uprintf("Time elapsed: %d\n", time_elapsed);
        if (time_elapsed < TWO_KEY_WINDOW) {
            last_two_keys[1] = last_two_keys[0];
            last_two_keys[0] = keycode;
            uprintf("two keys key0: %d, key1: %d\n", last_two_keys[0], last_two_keys[1]);
        } else {
            last_two_keys[1] = KC_NO;
            last_two_keys[0] = keycode;
            uprintf("zero keys key0: %d, key1: %d\n", last_two_keys[0], last_two_keys[1]);
        }
        last_keypress_timer = timer_read32();
    }

    switch (keycode) {
        case C_PBRAC:
            if (record->event.pressed) {
                SEND_STRING("{}");
                register_code(KC_LEFT);
                unregister_code(KC_LEFT);
            }
            return false;
    }
    return true;
}

The core issue is that process_smtd(keycode, record) returns false for the mod-tap keys when using the sm_td configuration, and thus the rest of the logic in process_record_user is skipped. I'd appreciate any insights on how to fix or adjust this behavior, as I'm not entirely sure what the correct expected behavior should be.

@chuan2984 chuan2984 changed the title [Bug] inconsistencies between normal and sm_td key when a sm_td mod is held [Bug] Incompatibility Between ALT_REPEAT and sm_td Mappings Oct 19, 2024
@stasmarkin
Copy link
Owner

Hello, @chuan2984! Thank you for the in-depth description and such a thorough investigation! Unfortunately, there is nothing you can do in version 0.4.0 about it. I didn't add support for key repeats or alt_repeats. The good news is that I'm developing version 0.5.0, which is supposed to fix that.

The current problem with repeats in version 0.4.0 is the way process_smtd() works. That function receives CKC_D and produces a KC_D tap by calling the tap_code(KC_D) function, which directly sends a signal to the OS. However, QMK's repeat feature reads incoming events before actually calling process_record_user(), so it knows about CKC_D but doesn't know anything about tap_code(KC_D) in process_smtd().

While writing that, I got an idea. Maybe defining get_alt_repeat_key_keycode_user() (see https://docs.qmk.fm/features/repeat_key) with CKC_D -> KC_U might fix that, maybe not. There is also tricky behavior for holding CKC_F, so that might affect the repeat feature too.

@chuan2984
Copy link
Author

However, QMK's repeat feature reads incoming events before actually calling process_record_user(), so it knows about CKC_D but doesn't know anything about tap_code(KC_D) in process_smtd().

ah, i see thisexplains it, i knew there must be something that alt_repeat does before process_record_user() but I was too lazy to read the source code. thanks a lot for the answer.
Im not in a rush or anything so ill wait for future releases, just wanted to bring this to your attention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants