Implement double tap keys

[ezuk]

Tap ; once to send a semicolon. Tap twice to send a colon.

Based on #411 and the excellent code and discussion between @algernon and @piotr-dobrogost.

Package this into something like: TAP(KC_SCLN, KC_CLN) - first argument is the single tap, second arg is the double tap. And no, we're not going to support triple taps ;)

[algernon]

I have an idea how to implement this nicely, that also includes the ability to run a custom function a'la Fn. So TAP(KC_SCLN, KC_CLN) would do what you describe, TAPF(N) would call the function set in tap_fns[N] for every tap, so it can do its own thing: blink leds, change layer, whatever.

With TAPF(), one would even be able to implement triple taps, or some crazy combination like oo= resulting in ő.

[algernon]

I have the beginnings done on my f/tap-dance branch. Not quite there yet, and there are a number of things I don't like about it, but it's a start. It also supports any number of taps, and makes it possible to do custom stuff, too. So you can have a hundred-tap action, for the most persistent folk.

What it does, is that in process_record_quantum(), it calls the new process_tap_dance() function, which catches all keys between 0x7101 and 0x71ff, and treats them as tap keys, that behave as follows:

• If no tap key is active yet, we record the current keycode, a timer, and set the count to 1.
• If there is a tap key active, but a different one, we fire off the old one, and start the new (note that this part is buggy on my branch at the moment! it does not start the new one)
• If there is a tap key active, and the current key is the same, we increment the counter, and update the timer.
• If a non-tap key is hit, we fire off the tap key, and queue the new one.

There is another important part of the functionality: matrix_scan_tap_dance(). This needs to be called from matrix_scan_user() as early as possible. This implements the part of the feature where a tap key can time out, without another key being pressed.

I tried calling matrix_scan_tap_dance() from matrix_scan_quantum(), but for some odd reason, that did not work.

The next steps are:

• [x] Fix the case of two different tap keys being tapped after one another.
• [x] Figure out why matrix_scan_quantum() doesn't do the trick.
• [x] Figure out how to implement the TAP macro, so users of the feature don't have to implement their own tap_dance_user() function.

Any hints about the naming, or why the matrix scan stuff doesn't work as I expected it would be much appreciated.

I also made a branch for my own layout that uses this feature.

[algernon]

Expanding on the hundred-tap thing: imagine an easter egg, where if you tap a certain key 100 times, fast, you get some nice feedback from your keyboard: sounds, crazy led dance, or backlight breathing furiously, and so on.

Based on the above code, this is entirely possible with very little additional code.

[algernon]

Looking at how the other macros like LT are implemented, I think I'll leave the TAP() macro to someone else. My solution would be very different, and use a lot of pre-processor magic:

We'd have a single array of keycode pairs: tap_pair_t taps[TAPS_COUNT] = TAPS_DEF. The TAPS_COUNT would be a #defined value, each instance of TAP(kc1, kc2) would increment it by one, and append the pair to TAPS_DEF. This means some pre-processor abuse, as we'll have to use temporary defines, and undef the two main macros, in order to avoid infinite loops. The macro also needs to be well formatted, so it can appear in a keymap.

We will also need a third macro, that the user can paste into their keymap, after the keymap itself, so that the tap pairs be available. We can't do that in a header, because those get included before the macros run.

Then, in process_tap_dance(), we could run through this array, and see if our key fits it. The index would translate to QK_TAP + index.

This way, the preprocessor would do most of the work, and changes could be contained in a few places. But doing this is a tad boring, and I don't need it, so I have little reason to work on it, now that I wrote it down :)

[ezuk]

Very cool work, @algernon -- but from reading your description and looking at the commit, I am uncertain as to where you landed in terms of the actual "interface". What's the current syntax your feature uses? (can just link to the spot where you're using it if that's easier)

[algernon]

Myeah, the diff of the commit in my repo is hard to read, indeed.

Here's how to use it:

• Have a key with a keycode of 0x7100 or higher defined, like this
• Include that in the keymap
• Create a tap_dance_user function.
• Call matrix_scan_tap_dance() from matrix_scan_user(), like this

And that's it. Not the friendliest thing, but it's a start.

[algernon]

Hm, thinking further, I think I have an idea how to do the TAP macro nicely, if we compromise: make it TAP(n), and have an Fn-like array for it, where one can use, say, DOUBLE_TAP(kc1, kc2) as an action, or a custom function.

I'll go into details tomorrow (CEST), once I PoC'd it up. It's a fairly simple solution, friendlier than what I have now, but not as user-friendly as TAP(kc1, kc2) would be, but close enough, I'd say.

[ezuk]

sounds good! Thank you :)

[algernon]

Making progress... my issue currently is that register_code(KC_COLN) sends a semicolon, and ignores the shift, so this thing I have, will only work for modifier-less codes, which is a lot less useful.

Will push my code in a bit, and write a short guide on how to use it.

[algernon]

Updated my branch, the current syntax is:

• Use TD(n) in the keymap, to define a double-tap key.
• Have a const tap_dance_pair_t tap_dance_pairs[] variable in the keymap, similar to fn_actions: const tap_dance_pair_t tap_dance_pairs[] = { [CT_AB] = DOUBLE_TAP_PAIR (KC_A, KC_B) }
• Still have to call matrix_scan_tap_dance() from matrix_scan_user(), but this is something I want to get rid of.

I have an idea how to extend this further, so one can have custom functions as callbacks, but I ran out of keyboarding time for today, and will have to do some work.

The idea is to turn have an array of these:

typedef enum {
  QK_TAP_PAIR,
  QK_TAP_FN
} qk_tap_type;

typedef void (*qk_tap_user_t) (td_state_t *state);

typedef struct {
 qk_tap_type type;
 union {
  struct {
    uint16_t kc1;
    uint16_t kc2;
  } pair;
  qk_tap_user_t fn;
 };
} qk_tap_action_t;

If the type is pair, we call a function that does the right thing based on the tap count. If the type is fn, we call a user defined function with the current tap dance state, and that fn is free to do whatever it wants.

I may be able to get this done tomorrow morning. Meanwhile, suggestions on naming, or other issues would be very useful to me.

The next issue I have, is that for this to work, I need the array defined, and I can't easily do a default for that. This means that either we need to update all keymaps to have an empty array, or have the feature be disabled by default.

I'm leaning towards the latter, that's less noise accross the board.

[ezuk]

So, last things first: Disabling by default is the way to go, I agree. Then we'll put it in some default/experimental keymaps and in the docs, and in the upcoming graphical configurator, and in time it'll find its way into more and more keymaps.

As for the design of the code: I'm an MVP kinda guy. :) I see you're taking it one step further with QK_TAP_FN and if you feel that's the right thing to do, awesome. From my side, I would be very happy to see a lean and clean (in terms of end coder interface) implementation of "just" the double tap functionality, which is already miles beyond what anyone else can do today.

So I guess my only suggestion here would be to target double tap first and then extend once people have had a chance to actually use it -- but of course this is just a suggestion. :)

[algernon]

I need the fn stuff, for things like :/;, because I need to do the shifting from code, register_code(KC_COLN) appears to send a ;... and I have my eyes on some additional things, that require a way to do more than a key tap when triggered - think blinking leds, or the 100 tap easter egg I mentioned above.

So I'm focusing on making it possible for me to do what I want, while still having a reasonably easy interface for others to use this feature.

What do you think about ENABLE_TAP_DANCE = yes to turn it on? :)

[algernon]

Almost ready, but I'm working from home today, so I can't test if what I write actually works, only that it compiles. Will push the latest implementation to a branch, and do a detailed update / tour, in case someone is brave enough to test it over the weekend while I'm away :)

Teaser:

const qk_tap_dance_action_t tap_dance_actions[] = {
 [CT_CLN] = ACTION_TAP_DANCE_FN (ang_tap_dance)
,[CT_SE]  = ACTION_TAP_DANCE_DOUBLE(KC_SPC, KC_ENT)
};

It needs to be enablef with TAP_DANCE_ENABLE=yes, and if done so, adds a little over 1k to the firmware size. Working on making that smaller.

[algernon]

Pushed my latest changes, see algernon@852f95d85f9222aefeaa970ed0e931b8ca9135f1 for a detailed explanation, and an example. As I was not able to test this yet, there are likely bugs in it, I feel that some of the functionality will not work as one would expect, but this is as far as I can go without testing it on a real keyboard.

I'll come back to the topic on Monday, when I'm at work, at the keyboard.

If anyone wants to give it a go, there's an example in the commit message, and the f/e-experiment of my layout also uses this new feature. To test that, once flashed, press LEAD e to activate the experimental keymap. Then, tapping the rightmost key on the bottom row should send a : on single tap, ; on double tap. The rightmost 2u key on the right thumb cluster should send SPC on single tap, Enter on double.

[algernon]

Couple of things I want to be able to do, all of which are reasonably easy to implement, based on the code I have now:

• I'd like to support holding a tap key:
  • If pressed and held immediately, register the first keycode, but don't release it until the key is depressed.
  • If tapped and then held, do this for the second keycode too.

For this, we will need the pressed state, and a way to signal to the caller when to reset the counter (but we will need the latter anyway). Should be about half an hour of work to get this done, so will do that on monday.

This would allow me to have a CT_SPC_BS key, that triggers Space on tap, Backspace on double tap, and I would be able to hold the key to register a Space, and let the OS do its repeat magic; or tap once, then hold, to do the same for Backspace.

[djl]

Tap-and-hold support would be great! I've got multiple use cases for this. For example, I'd like my LGUI key to work like this:

• Single tap -> OSM(MYLAYER)
• Hold -> Works like LGUI
• Tap and hold -> MO(MYLAYER)

[algernon]

That scenario would be a bit harder to implement, I think, but with ACTION_TAP_DANCE_FN() only jut a bit. I think, at least. Will have to give it a try.

[ezuk]

That is super cool, @algernon! And I totally see why you need the Fn stuff now -- the : / ; is a big part of the appeal here for me as well.

I am also dreaming of a "hyperspace cadet shift" where the Shift is a Shift key -- single tap sends parens (like now), but double shift key tap sends brackets. Insane :) not sure if that would be feasible with your new hold implementation, just a potential use case I'm excited about.

[jackhumbert]

I haven't had a chance to catch-up and dive into the magic behind everything yet, but this is awesome - exactly what I was imagining with the process_record stuff :)

Just wanted to mention that I have the SAFE_RANGE variable at the end of the quantum keycode enum that you can use instead of the hard-coded value, like:

enum {
  KC_CUSTOM = SAFE_RANGE,
  KC_CUSTOM_2
};

for custom functions in the keymap. Obviously when we merge in the tap dance stuff, it'll have its own range defined in quantum.h, like:

#define TD(n) (QK_TAP_DANCE + n)

[algernon]

Updated my branch, the code is tested now, and after a few little fixes, it works like a charm. The last remaining issue is that I would like to hook the timeout stuff into matrix_scan_quantum, so that users do not have to call matrix_scan_tap_dance() from their own matrix_scan_user(). However, adding debug printfs to matrix_scan_quantum() shows that it is never triggered, for some reason.

I'll play with the implementation for a few more hours, and if all goes well, I'll open a PR.

[algernon]

Darn. It looks like some - if not all - keyboards override the matrix_scan function, and do not call matrix_scan_quantum. So to hook into here, I'll have to touch all such cases.

[algernon]

Branch updated again, this time matrix_scan_tap_dance() should me called automatically, and the users do not have to place a call to in into matrix_scan_user() anymore. I update all keyboards that had a custom matrix_scan() function, and matrix_scan_quantum() too. Works on the ErgoDox EZ, and I suspect it will work elsewhere too, but I can't test that.

PR coming soon!

[pvinis]

i guess this can be closed now, because of ACTION_TAP_DANCE_DOUBLE?

[ezuk]

@pvinis absolutely!