stasmarkin / sm_td

SM Tap Dance user library for QMK
GNU General Public License v3.0
138 stars 5 forks source link

[Bug] Incompatibility Between ALT_REPEAT and sm_td Mappings #27

Open chuan2984 opened 1 day ago

chuan2984 commented 1 day ago

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.

stasmarkin commented 5 hours ago

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.