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: combo doesn't interrupt tap-dance #1701

Open
yanshay opened this issue Mar 10, 2023 · 3 comments
Open

Bug: combo doesn't interrupt tap-dance #1701

yanshay opened this issue Mar 10, 2023 · 3 comments

Comments

@yanshay
Copy link

yanshay commented Mar 10, 2023

tap-dance is interrupted by key press, meaning, given a tap-dance A B, pressing A then C quickly (before tapping-term-ms) yields AC (and thats the documented behavior).

However, given same tap-dance and a combo that generated D keypress, pressing A then the combo quickly (before tapping-term-ms) generate yields DA and not the expected AD.

I believe that's a bug.
Is that indeed the case? Is it solvable? Any workaround?

I tested if hold-tap, which also triggers bindings similar to combo, interrupts tap-dance, and it does. So seems the way tap-dance issues the binding is not done right.

@yanshay yanshay changed the title Bug?: combo doesn't interrupt tap-dance Bug: combo doesn't interrupt tap-dance Mar 10, 2023
@yanshay yanshay changed the title Bug: combo doesn't interrupt tap-dance Bug: combo doesn't interrupt tap-dance not sticky-key Mar 12, 2023
@yanshay yanshay changed the title Bug: combo doesn't interrupt tap-dance not sticky-key Bug: combo doesn't interrupt tap-dance nor sticky-key Mar 12, 2023
@yanshay yanshay changed the title Bug: combo doesn't interrupt tap-dance nor sticky-key Bug: combo doesn't interrupt tap-dance Mar 12, 2023
@yanshay
Copy link
Author

yanshay commented Mar 13, 2023

I found a simple solution - switching the order between tap-dance and combo in the CMakeLists.txt .
However, the current order was set in #1518 to allow tap-dance of combos.
So it seems without code changes need to pick which limitation is preferred.

tap-dance needs a position_state_change to get interrupted, combo swallows those events before they arrive to tap-dance so tap-dance isn't interrupted when combo is pressed.
I tried raising position_changed events since combo is sort of a new key, using the virtual_key_position, but those virtual key positions generated additional key presses, probably by other handlers.
Is there a way to communicate directly between behaviors, and have combo be aware of tap-dance to interrupt it directly as a special case? Maybe send events to only a single behavior and not all?

P.S. I tried another option, to have tap-dance interrupt using keycode_state_changed. That worked, but when using tap-dance that trigger mods with combos that trigger mod_morph it didn't work because mod_morph checks mod status before it issues the key press, and that's before tap-dance knows sees the keypress keycode state change so the mod_morph always issues the un-moded key.

All in all I exhausted my ideas, can anyone help with this?

@yanshay
Copy link
Author

yanshay commented Mar 13, 2023

I tried a few more options to raise position_state_changed events and the below ended up working.
Raising position_state_changed events at position -1 did the trick, but I don't know exactly how its handled. But it does interrupt tap-dance and fixes all issues I had with this.

Maybe someone can comment if that's a proper solution?

static inline int press_combo_behavior(struct combo_cfg *combo, int32_t timestamp) {
    struct zmk_behavior_binding_event event = {
        .position = combo->virtual_key_position,
        .timestamp = timestamp,
    };

    last_combo_timestamp = timestamp;

    ZMK_EVENT_RAISE(new_zmk_position_state_changed(
        (struct zmk_position_state_changed){.source = 0, // not ZMK_POSITION_STATE_CHANGE_SOURCE_LOCAL,
                                            .state = true,
                                            .position = -1, // not combo->virtual_key_position,
                                            .timestamp = timestamp}));

    return behavior_keymap_binding_pressed(&combo->behavior, event);
}

static inline int release_combo_behavior(struct combo_cfg *combo, int32_t timestamp) {
    struct zmk_behavior_binding_event event = {
        .position = combo->virtual_key_position,
        .timestamp = timestamp,
    };

    ZMK_EVENT_RAISE(new_zmk_position_state_changed(
        (struct zmk_position_state_changed){.source = 0, // not ZMK_POSITION_STATE_CHANGE_SOURCE_LOCAL,
                                            .state = false,
                                            .position = -1, // not combo->virtual_key_position,
                                            .timestamp = timestamp}));

    return behavior_keymap_binding_released(&combo->behavior, event);
}

@jjangga0214
Copy link

jjangga0214 commented Mar 16, 2023

I believe tap dance has a bug that doesn't preserve key input as a queue in the right order.
It even drops the input.

For instance, I set the letter n as a tab dance("nn" triggers another feature).
And when I type some words like "and" quickly, it becomes "an", dropping "d".

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