qmk / qmk_firmware

Open-source keyboard firmware for Atmel AVR and Arm USB families
https://qmk.fm
GNU General Public License v2.0
17.81k stars 38.16k forks source link

chaining one shot mods #2796

Closed manogna4 closed 6 years ago

manogna4 commented 6 years ago

I would like to tap 'ctrl', tap 'shift' and then tap 'a' to get 'Ctrl+Shift+a'. At present this can be done with only one modifier. Tapping the second modifier cancels the first one. The above combination produces 'Shift+a' i.e. 'A'

This is a duplicate of 856 and of 1580 but has been opened because @algernon asked for it on reddit/olkb. Both issues were closed because they were too difficult to do.

algernon commented 6 years ago

It shouldn't be too difficult, we're doing it in Kaleidoscope. In QMK, it could be even easier, because the report isn't cleared each cycle (as far as I remember), so all it would take is to not treat other oneshot keys as interrupts.

On the other hand, having looked at the code that implements oneshot in QMK... I can see why adding this is non-trivial... or, perhaps it is:

diff --git a/tmk_core/common/action.c b/tmk_core/common/action.c
index dd3a5b3ee..91577228c 100644
--- a/tmk_core/common/action.c
+++ b/tmk_core/common/action.c
@@ -217,10 +217,10 @@ void process_action(keyrecord_t *record, action_t action)
                         if (event.pressed) {
                             if (tap_count == 0) {
                                 dprint("MODS_TAP: Oneshot: 0\n");
-                                register_mods(mods);
+                                register_mods(mods | get_oneshot_mods());
                             } else if (tap_count == 1) {
                                 dprint("MODS_TAP: Oneshot: start\n");
-                                set_oneshot_mods(mods);
+                                set_oneshot_mods(mods | get_oneshot_mods());
                     #if defined(ONESHOT_TAP_TOGGLE) && ONESHOT_TAP_TOGGLE > 1
                             } else if (tap_count == ONESHOT_TAP_TOGGLE) {
                                 dprint("MODS_TAP: Toggling oneshot");
@@ -229,7 +229,7 @@ void process_action(keyrecord_t *record, action_t action)
                                 register_mods(mods);
                     #endif
                             } else {
-                                register_mods(mods);
+                              register_mods(mods | get_oneshot_mods());
                             }
                         } else {
                             if (tap_count == 0) {

Can you give this a go?

It is probably not perfect, and may have a few weird interactions here and there, but its a start. Let me know how it works for you, and if you find any problematic cases, and I'll try to find some time to improve the patch (and eventually turn it into a pull request).

manogna4 commented 6 years ago

it works 😃 This is brilliant. Thanks. Such a simple change too. I wonder why the the comments on the previous issues said it was too hard...

I'll let you know if it leads to any problems. I ran some simple tests, and it didn't cause any issues. I do not use any tapping toggles so I won't be able to tell you if it works with them.

algernon commented 6 years ago

It may look simple, but it took me about four hours to find the right places to modify =) It is almost always simple in hindsight.

manogna4 commented 6 years ago

whoa! Thanks a lot for your effort then. This frees up several keys on my layer.

drashna commented 6 years ago

Awesome!

DanielGGordon commented 6 years ago

Woah cool - can we make a PR for this?

algernon commented 6 years ago

There is a PR for this, #2799.