KMKfw / kmk_firmware

Clackety Keyboards Powered by Python
https://kmkfw.zulipchat.com
Other
1.39k stars 475 forks source link

Automatic Modifier Chaining for OneShot Keys #427

Closed ldsands closed 1 year ago

ldsands commented 2 years ago

So as recommended by @xs5871 on the matrix channel I used Combos to allow for chaining modifier keys while using OneShot keys. I even added it to docs/oneshot.md. Unfortunately, there is a consistent application of the chained modifiers to the next two keys tapped/pressed after the OneShot modifier(s). This does stop it the timeout is reached, so it is still useful but still causes problems. I've tried both Chord and Sequence from Combos and I've tested the per_key_timeout using both True and False. I realize that just waiting for the timeout works or shortening the timeout. For me these options either makes the modifier chaining using this method unusable for some (less commonly used) shortcuts or I forget and the modifiers apply anyway.

Ideally I would like to be able to do something like the following when loading the OneShot module:

oneshot = OneShot(chain_modifiers=True)

Implementing this would almost certainly involve the chaining code to be located in the kmk/modules/oneshot.py and/or kmk/modules/holdtap.py files. If this were to be done in oneshot.py file I think this could be done in the process_key function. Below I just have a ModifierKey if statement and a print function (in oneshot.py) where I see this going but I'm not sure how to extract the modifier pressed (e.g. KC.LCTL) to then chain it and then return the chained modifiers to then apply it to the next key pressed/interruption or await the timeout.

def process_key(self, keyboard, current_key, is_pressed, int_coord):
    '''Release os key after interrupting keyup.'''
    for key, state in self.key_states.items():
        if key == current_key:
            continue

        if state.activated == ActivationType.PRESSED and is_pressed:
            state.activated = ActivationType.HOLD_TIMEOUT
        elif state.activated == ActivationType.RELEASED and is_pressed:
            state.activated = ActivationType.INTERRUPTED
        elif state.activated == ActivationType.INTERRUPTED:
            if isinstance(current_key.meta.kc, ModifierKey):  # TESTCODE:
                print("current_key is a modifier key")  # TESTCODE:
            self.ht_released(key, keyboard)

    return current_key

The way I thought I would go about solving this would be to do a check for a modifier key when a OneShot key is interrupted. If the interrupted key is another ModifierKey then that modifier would be extracted added to the first OneShot modifier and then return the two (now) chained modifiers to then await another interruption or timeout. Does this seem like a good approach to the problem?

Other complications/considerations:

I think I can help with this but I cannot seem to figure out how to extract the modifier being pressed to then return it for chaining. Any help with that and I might be able to do the rest on my own assuming that this should be done in the oneshot.py file, that holdtap.py file confuses me.

Thanks in advance for all of your help and your work on KMK!

xs5871 commented 2 years ago

Could you provide a code example? From your description it's not clear to me if that is a bug or intended.

ldsands commented 2 years ago

I think that there is a bug as well (based on your matrix/discord comments/replies to me (sholva) and theroncross that I found today). However, even with the bug fixed it would be nice to have this feature. Obviously, getting that bug fixed first would be preferable, because at least then the chording option would work.

Let me know if I should open up a seperate issue for the bug.

Below is the code that I use to allow for chaining OneShot modifier keys.

Long code chunk (click to load) ```python # # OneShot Keys, tap_time is tap timeout in ms (default: 1000ms) OS_LCTL = KC.OS(KC.LCTL, tap_time=None) OS_LSFT = KC.OS(KC.LSFT, tap_time=None) OS_LGUI = KC.OS(KC.LGUI, tap_time=None) OS_LALT = KC.OS(KC.LALT, tap_time=None) OS_LCTL_LSFT = KC.OS(KC.LCTL(OS_LSFT), tap_time=None) OS_LCTL_LALT = KC.OS(KC.LCTL(OS_LALT), tap_time=None) OS_LCTL_LGUI = KC.OS(KC.LCTL(OS_LGUI), tap_time=None) OS_LSFT_LALT = KC.OS(KC.LSFT(OS_LALT), tap_time=None) OS_LSFT_LGUI = KC.OS(KC.LSFT(OS_LGUI), tap_time=None) OS_LALT_LGUI = KC.OS(KC.LALT(OS_LGUI), tap_time=None) OS_LCTL_LSFT_LGUI = KC.OS(KC.LCTL(KC.LSFT(OS_LGUI)), tap_time=None) OS_LCTL_LSFT_LALT = KC.OS(KC.LCTL(KC.LSFT(OS_LALT)), tap_time=None) OS_LCTL_LALT_LGUI = KC.OS(KC.LCTL(KC.LALT(OS_LGUI)), tap_time=None) OS_LSFT_LALT_LGUI = KC.OS(KC.LSFT(KC.LALT(OS_LGUI)), tap_time=None) OS_LCTL_LSFT_LALT_LGUI = KC.OS(KC.LCTL(KC.LSFT(KC.LALT(OS_LGUI))), tap_time=None) # chord combos to allow for OneShot modifier key combinations combos.combos = [ Chord((OS_LCTL, OS_LSFT), OS_LCTL_LSFT, timeout=750), Chord((OS_LCTL, OS_LALT), OS_LCTL_LALT, timeout=750), Chord((OS_LCTL, OS_LGUI), OS_LCTL_LGUI, timeout=750), Chord((OS_LSFT, OS_LALT), OS_LSFT_LALT, timeout=750), Chord((OS_LSFT, OS_LGUI), OS_LSFT_LGUI, timeout=750), Chord((OS_LALT, OS_LGUI), OS_LALT_LGUI, timeout=750), Chord((OS_LCTL, OS_LSFT, OS_LGUI), OS_LCTL_LSFT_LGUI, timeout=750), Chord((OS_LCTL, OS_LSFT, OS_LALT), OS_LCTL_LSFT_LALT, timeout=750), Chord((OS_LCTL, OS_LALT, OS_LGUI), OS_LCTL_LALT_LGUI, timeout=750), Chord((OS_LSFT, OS_LALT, OS_LGUI), OS_LSFT_LALT_LGUI, timeout=750), Chord((OS_LCTL, OS_LSFT, OS_LALT, OS_LGUI), OS_LCTL_LSFT_LALT_LGUI, timeout=750), ] # NOQA # flake8: noqa keyboard.keymap = [ [ # WORKMAN #HERE-----#HERE-----#HERE-----#HERE-----#HERE-----#HERE-----#HERE-----#ENCODER--#ENCODER--#HERE-----#HERE-----#HERE-----#HERE-----#HERE-----#HERE-----#HERE----- KC.ESC, KC.N1, KC.N2, KC.N3, KC.N4, KC.N5, KC.N6, KC.N7, KC.N8, KC.N9, KC.N0, KC.MINS, KC.LPRN, KC.Q, KC.D, KC.R, KC.W, KC.B, KC.J, KC.F, KC.U, KC.P, KC.SCLN, KC.BSLS, KC.RPRN, KC.A, KC.S, KC.H, KC.T, KC.G, KC.Y, KC.N, KC.E, KC.O, KC.I, KC.QUOT, OS_LALT, KC.Z, KC.X, KC.M, KC.C, KC.V, KC.K, KC.L, KC.COMM, KC.DOT, KC.SLSH, KC.ENT, OS_LGUI, MO_FUNCT, OS_LSFT, KC.SPC, OS_LCTL, TO_NAV, TO_WORK, KC.DEL, KC.BSPC, MO_NUM, CTTAB, KC.TAB, KC.PGUP, KC.PGDN, KC.VOLD, KC.VOLU, #Encoder-Rotary-Directions----------------------------------#Counter--#Clockw--#Counter---#Clockw--------------------------------------------------------------- ], ] ```

This does work except as expected except that the modifiers apply to keys that they shouldn't. I'm using VSCode to test all of this. For example, tapping OS_CTRL then KC.N this opens a new tab (editor) but then if I press KC.W within the timeout period (which shouldn't be due to the interruption made by KC.N) it will close the tab (editor) meaning that the OS_CTRL is still being applied.

So I think that there is something not happening with the timeout and/or interruption related code. However, when not using the combos (chord or sequence) the OneShot keys work as expected.

Does that help?

ldsands commented 2 years ago

So I have an update on this. First off everything (except one item detailed below) seems to now be working with the modifier chaining. So whatever bug was affecting it seems to have been resolved. Something edited after May 6th fixed it. I don't think it was anything I did in my code since all I've done since then is to add OLED code to my kb.py and main.py.

ldsands commented 2 years ago

The issue/feature request that is still not resolved is that the OneShot keys all release upon change layers. I would be great to have an option to enable/disable OneShot modifier keys to apply to keys pressed after a layer change. I will probably take a stab at it but I don't know if I have the skills to figure it out. I'll give an update when/if I do make progress.

ldsands commented 2 years ago

I did find something of a workaround using chords, though it feels slower. Below is an example of me adding to the chords that I do/could use with a layer key.

for layer_key in [MO_FUNCT, MO_NUM]:
    combos.combos.append(Chord((OS_LCTL, layer_key), OS_LCTL, timeout=750))
    combos.combos.append(Chord((OS_LSFT, layer_key), OS_LSFT, timeout=750))
    combos.combos.append(Chord((OS_LGUI, layer_key), OS_LGUI, timeout=750))
    combos.combos.append(Chord((OS_LALT, layer_key), OS_LALT, timeout=750))

    combos.combos.append(Chord((OS_LCTL, OS_LSFT, layer_key), OS_LCTL_LSFT, timeout=750))
    combos.combos.append(Chord((OS_LCTL, OS_LALT, layer_key), OS_LCTL_LALT, timeout=750))
    combos.combos.append(Chord((OS_LCTL, OS_LGUI, layer_key), OS_LCTL_LGUI, timeout=750))
    combos.combos.append(Chord((OS_LSFT, OS_LALT, layer_key), OS_LSFT_LALT, timeout=750))
    combos.combos.append(Chord((OS_LSFT, OS_LGUI, layer_key), OS_LSFT_LGUI, timeout=750))
    combos.combos.append(Chord((OS_LALT, OS_LGUI, layer_key), OS_LALT_LGUI, timeout=750))

Ideally, I still think it would be best to have an option built into the OneShot module that could allow for chaining modifier keys without having to resort to using the combos module.

xs5871 commented 2 years ago

The "feels slow" is another issue / missing combo feature / on my to-do list (an option to resolve a combo as soon as it matches, instead of waiting for the timeout). Nevertheless, combos in this situation are verbose and tedious to setup. I agree that we should make this an option (or maybe default) within the oneshot module.

ldsands commented 2 years ago

The "feels slow" is another issue / missing combo feature / on my to-do list (an option to resolve a combo as soon as it matches, instead of waiting for the timeout).

Thank you! I think I may have said this elsewhere, so forgive me if this is repetitive, but OneShots (when I started using them with QMK) was probably one of the most effective things I did to reduce my wrist pain. Maybe I just use too many keyboard shortcuts :).

Nevertheless, combos in this situation are verbose and tedious to setup. I agree that we should make this an option (or maybe default) within the oneshot module.

Yes, using combos is verbose, and I do think that setting this as the default, at least to me, makes sense.

I have tried to see if I could implement something like this myself but I have very little experience with oop. I'm mostly self-taught and I only do data science so I don't have to worry about classes very often. Most of KMK goes way over my head, so again thank you for working on this!

xs5871 commented 2 years ago

@ldsands: from the QMK docs I couldn't figure out how the chaining "is supposed to" work (or even find a mention of it), and I don't feel like digging through the code right now. If you could provide me with some insight:

ldsands commented 1 year ago

@xs5871 Sorry I didn't see this I must have accidentally dismissed it without realizing.

To answer your questions:

I hope those thoughts help.

Also, just to remind you so you're aware, multiple OneShot modifiers with Chords which is the workaround to this issue I've been using since you made that possibile is currently broken. I'm pretty sure this issue is related.

Thanks again for all of your work on this! Apart from it being broken at the moment, it has been working very well overall.

ldsands commented 1 year ago

@xs5871

I got around to updating KMK (885359a) and OneShot modifier chaining using Combos still seems to be broken. I'm assuming it has something to do with the Combos refactor and/or some of the changes to holdtap that have happened since 2022-08-17 (6d6ccbc). There is no rush as older versions work well. Any of your thoughts are appreciated.

Also, thank you for all of your work on KMK!

xs5871 commented 1 year ago

Hm. It's probably better (and easier) to add a chaining option to the oneshot module anyway.

ldsands commented 1 year ago

Hm. It's probably better (and easier) to add a chaining option to the oneshot module anyway.

I wouldn't know if it is easier to do it this way but I do think that would better at least from an ease of use perspective.

ldsands commented 1 year ago

Just wanted to say thank you to @xs5871! This latest rewrite works great! It even allows me to use my mouse with the mod keys while I hold them down. In the past this wouldn't happen until after the timeout expired.

Again thank you, this really helps with my wrist pain!