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

Control how interrupting the tapping function works without using "TAPPING_FORCE_HOLD" #18770

Closed
Ndot opened this issue Oct 19, 2022 · 2 comments

Comments

@Ndot
Copy link
Contributor

Ndot commented Oct 19, 2022

Issue Description

Hi,

I have the following situation: key "LT(_MOVE, KC_BSPC)" the layer "_MOVE" has on the same key as "KC_I" the key "KC_UP".

When I do the sequence:
-> tap key.
-> press key and hold (here the tapping function starts)
-> interrupt with "KC_I" (the result is "KC_I", as if I was not holding the key "LT(_MOVE, KC_BSPC)". The hold function is never active it's like I never pressed it, I have to let go and press again.)

Basically when the keyboard enters the repeating function if I interrupt, I would expect the hold function to kick in because I'm holding the key down so I should get the key in the "_MOVE" layer. But instead I get the normal layer like I'm not pressing the "LT(_MOVE, KC_BSPC)" key.

This is solvable with "TAPPING_FORCE_HOLD" but I lose the tapping function which I don't want to.

Is there a way to achieve this and control how interrupting the tapping function works.

Thanks

@sigprof
Copy link
Contributor

sigprof commented Oct 19, 2022

If I understand you correctly, the behavior you want for LT(_MOVE, KC_BSPC) is:

  1. Tap ⇒ tap KC_BSPC
  2. Hold ⇒ enable the _MOVE layer
  3. Tap then hold without pressing any other key at the same time within the tapping term ⇒ tap KC_BSPC, then hold KC_BSPC
  4. Tap then hold and press another key at the same time within the tapping term ⇒ tap KC_BSPC, then enable the _MOVE layer

Achieving this behavior with just the core tapping code does not seem to be possible:

  • If you don't enable TAPPING_FORCE_HOLD, the tapping code processes only the first tap or hold action; once the first tap has been detected, subsequent presses of the same key within the tapping term get processed immediately with increased record->tap.count, and (what is important here) without queuing other events to distinguish between a tap or a hold for subsequent key actions (all actions after the first are assumed to be taps).
  • If you enable TAPPING_FORCE_HOLD, the tapping state just gets reset immediately after the key is released, therefore nothing gets carried from the first tap to the second — the second press just gets processed exactly as the first one, therefore you will lose the item 3 of the desired behavior.

Possible options that I could think of:

a) If you are OK with always enabling the HOLD_ON_OTHER_KEY_PRESS behavior (or the per key variant of it just for your LT(_MOVE, KC_BSPC) key), you can enable TAPPING_FORCE_HOLD (can use the per key variant too) and then try doing something like this:

bool process_record_user(uint16_t keycode, keyrecord_t *record) {
    switch (keycode) {
        case LT(_MOVE, KC_BSPC):
            if (!record->tap.count && record->tap.interrupted) {
                if (record->event.pressed) {
                    layer_on(_MOVE);
                } else {
                    layer_off(_MOVE);
                }
            } else {
                if (record->event.pressed) {
                    register_code(KC_BSPC);
                } else {
                    unregister_code(KC_BSPC);
                }
            }
            return false;
    }
    return true;
}

But the problem here is that any hold of that key will now be interpreted as a (delayed) hold of KC_BSPC; the only way to get the layer switch behavior would be to press another key withing the tapping term.

b) If you don't like the hold behavior change from a), you may try to fix that using this (complicated and untested) code:

bool process_record_user(uint16_t keycode, keyrecord_t *record) {
    switch (keycode) {
        case LT(_MOVE, KC_BSPC):
            if (record->event.pressed) {
                static uint32_t last_press_timestamp;
                uint16_t event_delay = timer_read() - (uint16_t)(record->event.time & ~1U);
                uint32_t event_timestamp = timer_read32() - event_delay;
                bool within_tapping_term = TIMER_DIFF_32(event_timestamp, last_press_timestamp) < TAPPING_TERM;
                last_press_timestamp = event_timestamp;
                if (!record->tap.count && !within_tapping_term) {
                    // first hold - force layer switch
                    record->tap.interrupted = true;
                }
            }
            if (!record->tap.count && record->tap.interrupted) {
                // First hold, or an interrupted hold after a tap
                if (record->event.pressed) {
                    layer_on(_MOVE);
                } else {
                    layer_off(_MOVE);
                }
            } else {
                // Tap, or a non-interrupted hold after a tap
                if (record->event.pressed) {
                    register_code(KC_BSPC);
                } else {
                    unregister_code(KC_BSPC);
                }
            }
            return false;
    }
    return true;
}

This will still effectively force HOLD_ON_OTHER_KEY_PRESS behavior, but at least an uninterrupted hold without a preceding tap within the tapping term would be interpreted as a layer switch, not as a hold of the tap key. Also you might be able to change this code to avoid forcing the HOLD_ON_OTHER_KEY_PRESS behavior on the first hold by making the logic even more complicated.

c) You may try replacing LT() with a tap dance and implement the desired behavior in your custom tap dance code; however, you would probably need to use the develop branch for now to get the fix from #17935, otherwise you may encounter some broken behavior when trying to switch layers from your tap dance. And tap dances always have the HOLD_ON_OTHER_KEY_PRESS-like behavior (you can handle the interrrupted state as you want, but the decision needs to be made immediately when another key is pressed — any forms of delayed decision are not supported by the tap dance code).

@Ndot
Copy link
Contributor Author

Ndot commented Oct 21, 2022

Thanks for the suggestions they work great. The behavior is exactly what I was looking for. Forcing record->tap.interrupted if we passed the TAPPING_TERM did the trick (options b). The HOLD_ON_OTHER_KEY_PRESS behavior for this particular key is not relevant so it's totally ok. The only thing that didn't work was the layer_off call not sure why, but I just pushed that call outside the if statement this way we always remove the layer, maybe not necessary but works fine. I'll leave the final code bellow in case it's useful to someone else. Thanks.

bool process_record_user(uint16_t keycode, keyrecord_t *record) {
    switch (keycode) {
        case LT(_MOVE, KC_BSPC):

            if (record->event.pressed) {
                static uint32_t last_press_timestamp;
                uint16_t        event_delay         = timer_read() - (uint16_t)(record->event.time & ~1U);
                uint32_t        event_timestamp     = timer_read32() - event_delay;
                bool            within_tapping_term = TIMER_DIFF_32(event_timestamp, last_press_timestamp) < TAPPING_TERM;
                last_press_timestamp                = event_timestamp;

                if (!record->tap.count && !within_tapping_term) {
                    // first hold - force layer switch
                    record->tap.interrupted = true;
                }
            }

            if (!record->tap.count && record->tap.interrupted) {
                // First hold, or an interrupted hold after a tap
                if (record->event.pressed) {
                    layer_on(_MOVE);
                    return false; // To avoid calling layer_off.
                }
            } else {
                // Tap, or a non-interrupted hold after a tap
                if (record->event.pressed) {
                    register_code(KC_BSPC);
                } else {
                    unregister_code(KC_BSPC);
                }
            }
            layer_off(_MOVE);
            return false;
    }

    return true;
}

bool get_tapping_force_hold(uint16_t keycode, keyrecord_t *record) {
    switch (keycode) {
        case LT(_MOVE, KC_BSPC):
            return true;
    }

    return false;
}

@Ndot Ndot closed this as completed Oct 21, 2022
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