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

quantum: Add a tap dance feature #451

Merged
merged 4 commits into from Jun 28, 2016

Conversation

Projects
None yet
4 participants
@algernon
Contributor

algernon commented Jun 27, 2016

With this feature one can specify keys that behave differently, based on the amount of times they have been tapped, and when interrupted, they get handled before the interrupter.

To make it clear how this is different from ACTION_FUNCTION_TAP, lets explore a certain setup! We want one key to send Space on single tap, but Enter on double-tap.

With ACTION_FUNCTION_TAP, it is quite a rain-dance to set this up, and has the problem that when the sequence is interrupted, the interrupting key will be send first. Thus, SPC a will result in a SPC being sent, if they are typed within TAPPING_TERM. With the tap dance feature, that'll come out as SPC a, correctly.

The implementation hooks into two parts of the system, to achieve this: into process_record_quantum(), and the matrix scan. We need the latter to be able to time out a tap sequence even when a key is not being pressed, so SPC alone will time out and register after TAPPING_TERM time.

But lets start with how to use it, first!

First, you will need TAP_DANCE_ENABLE=yes in your Makefile, because the feature is disabled by default. This adds a little less than 1k to the firmware size. Next, you will want to define some tap-dance keys, which is easiest to do with the TD() macro, that - similar to F(), takes a number, which will later be used as an index into the tap_dance_actions array.

This array specifies what actions shall be taken when a tap-dance key is in action. Currently, there are two possible options:

  • ACTION_TAP_DANCE_DOUBLE(kc1, kc2): Sends the kc1 keycode when tapped once, kc2 otherwise.
  • ACTION_TAP_DANCE_FN(fn): Calls the specified function - defined in the user keymap - with the current state of the tap-dance action.

The first option is enough for a lot of cases, that just want dual roles. For example, ACTION_TAP_DANCE(KC_SPC, KC_ENT) will result in Space being sent on single-tap, Enter otherwise.

And that's the bulk of it!

Do note, however, that this implementation does have some consequences: keys do not register until either they reach the tapping ceiling, or they time out. This means that if you hold the key, nothing happens, no repeat, no nothing. It is possible to detect held state, and register an action then too, but that's not implemented yet. Keys also unregister immediately after being registered, so you can't even hold the second tap. This is intentional, to be consistent.

And now, on to the explanation of how it works!

The main entry point is process_tap_dance(), called from process_record_quantum(), which is run for every keypress, and our handler gets to run early. This function checks whether the key pressed is a tap-dance key. If it is not, and a tap-dance was in action, we handle that first, and enqueue the newly pressed key. If it is a tap-dance key, then we check if it is the same as the already active one (if there's one active, that is). If it is not, we fire off the old one first, then register the new one. If it was the same, we increment the counter and the timer.

This means that you have TAPPING_TERM time to tap the key again, you do not have to input all the taps within that timeframe. This allows for longer tap counts, with minimal impact on responsiveness.

Our next stop is matrix_scan_tap_dance(). This handles the timeout of tap-dance keys.

For the sake of flexibility, tap-dance actions can be either a pair of keycodes, or a user function. The latter allows one to handle higher tap counts, or do extra things, like blink the LEDs, fiddle with the backlighting, and so on. This is accomplished by using an union, and some clever macros.

In the end, lets see a full example!

enum {
 CT_SE = 0,
 CT_CLN,
 CT_EGG
};

/* Have the above three on the keymap, TD(CT_SE), etc... */

void dance_cln (qk_tap_dance_state_t *state) {
  if (state->count == 1) {
    register_code (KC_RSFT);
    register_code (KC_SCLN);
    unregister_code (KC_SCLN);
    unregister_code (KC_RSFT);
  } else {
    register_code (KC_SCLN);
    unregister_code (KC_SCLN);
    reset_tap_dance (state);
  }
}

void dance_egg (qk_tap_dance_state_t *state) {
  if (state->count >= 100) {
    SEND_STRING ("Safety dance!");
    reset_tap_dance (state);
  }
}

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

This addresses #426.

quantum: Add a tap dance feature
With this feature one can specify keys that behave differently, based on
the amount of times they have been tapped, and when interrupted, they
get handled before the interrupter.

To make it clear how this is different from `ACTION_FUNCTION_TAP`, lets
explore a certain setup! We want one key to send `Space` on single tap,
but `Enter` on double-tap.

With `ACTION_FUNCTION_TAP`, it is quite a rain-dance to set this up, and
has the problem that when the sequence is interrupted, the interrupting
key will be send first. Thus, `SPC a` will result in `a SPC` being sent,
if they are typed within `TAPPING_TERM`. With the tap dance feature,
that'll come out as `SPC a`, correctly.

The implementation hooks into two parts of the system, to achieve this:
into `process_record_quantum()`, and the matrix scan. We need the latter
to be able to time out a tap sequence even when a key is not being
pressed, so `SPC` alone will time out and register after `TAPPING_TERM`
time.

But lets start with how to use it, first!

First, you will need `TAP_DANCE_ENABLE=yes` in your `Makefile`, because
the feature is disabled by default. This adds a little less than 1k to
the firmware size. Next, you will want to define some tap-dance keys,
which is easiest to do with the `TD()` macro, that - similar to `F()`,
takes a number, which will later be used as an index into the
`tap_dance_actions` array.

This array specifies what actions shall be taken when a tap-dance key is
in action. Currently, there are two possible options:

* `ACTION_TAP_DANCE_DOUBLE(kc1, kc2)`: Sends the `kc1` keycode when
  tapped once, `kc2` otherwise.
* `ACTION_TAP_DANCE_FN(fn)`: Calls the specified function - defined in
  the user keymap - with the current state of the tap-dance action.

The first option is enough for a lot of cases, that just want dual
roles. For example, `ACTION_TAP_DANCE(KC_SPC, KC_ENT)` will result in
`Space` being sent on single-tap, `Enter` otherwise.

And that's the bulk of it!

Do note, however, that this implementation does have some consequences:
keys do not register until either they reach the tapping ceiling, or
they time out. This means that if you hold the key, nothing happens, no
repeat, no nothing. It is possible to detect held state, and register an
action then too, but that's not implemented yet. Keys also unregister
immediately after being registered, so you can't even hold the second
tap. This is intentional, to be consistent.

And now, on to the explanation of how it works!

The main entry point is `process_tap_dance()`, called from
`process_record_quantum()`, which is run for every keypress, and our
handler gets to run early. This function checks whether the key pressed
is a tap-dance key. If it is not, and a tap-dance was in action, we
handle that first, and enqueue the newly pressed key. If it is a
tap-dance key, then we check if it is the same as the already active
one (if there's one active, that is). If it is not, we fire off the old
one first, then register the new one. If it was the same, we increment
the counter and the timer.

This means that you have `TAPPING_TERM` time to tap the key again, you
do not have to input all the taps within that timeframe. This allows for
longer tap counts, with minimal impact on responsiveness.

Our next stop is `matrix_scan_tap_dance()`. This handles the timeout of
tap-dance keys.

For the sake of flexibility, tap-dance actions can be either a pair of
keycodes, or a user function. The latter allows one to handle higher tap
counts, or do extra things, like blink the LEDs, fiddle with the
backlighting, and so on. This is accomplished by using an union, and
some clever macros.

In the end, lets see a full example!

```c
enum {
 CT_SE = 0,
 CT_CLN,
 CT_EGG
};

/* Have the above three on the keymap, TD(CT_SE), etc... */

void dance_cln (qk_tap_dance_state_t *state) {
  if (state->count == 1) {
    register_code (KC_RSFT);
    register_code (KC_SCLN);
    unregister_code (KC_SCLN);
    unregister_code (KC_RSFT);
  } else {
    register_code (KC_SCLN);
    unregister_code (KC_SCLN);
    reset_tap_dance (state);
  }
}

void dance_egg (qk_tap_dance_state_t *state) {
  if (state->count >= 100) {
    SEND_STRING ("Safety dance!");
    reset_tap_dance (state);
  }
}

const qk_tap_dance_action_t tap_dance_actions[] = {
  [CT_SE]  = ACTION_TAP_DANCE_DOUBLE (KC_SPC, KC_ENT)
 ,[CT_CLN] = ACTION_TAP_DANCE_FN (dance_cln)
 ,[CT_EGG] = ACTION_TAP_DANCE_FN (dance_egg)
};
```

This addresses #426.

Signed-off-by: Gergely Nagy <algernon@madhouse-project.org>
@algernon

This comment has been minimized.

Show comment
Hide comment
@algernon

algernon Jun 27, 2016

Contributor

This was easier than expected. The changes compared to the previous PR:

  • Moved the tap dance files to a subdir, like the rest of the quantum features.
  • Fixed a bug where pressing two tap keys would register only the second.
  • Removed an unused parameter from process_tap_dance_action.

And as I see, I broke the build. Will fix that too, in a few minutes, hopefully.

Contributor

algernon commented Jun 27, 2016

This was easier than expected. The changes compared to the previous PR:

  • Moved the tap dance files to a subdir, like the rest of the quantum features.
  • Fixed a bug where pressing two tap keys would register only the second.
  • Removed an unused parameter from process_tap_dance_action.

And as I see, I broke the build. Will fix that too, in a few minutes, hopefully.

hhkb: Fix the build with the new tap-dance feature
Signed-off-by: Gergely Nagy <algernon@madhouse-project.org>
@algernon

This comment has been minimized.

Show comment
Hide comment
@algernon

algernon Jun 27, 2016

Contributor

crosses fingers

hhkb is fixed now, hopefully I didn't break anything else.

Contributor

algernon commented Jun 27, 2016

crosses fingers

hhkb is fixed now, hopefully I didn't break anything else.

@jackhumbert

This comment has been minimized.

Show comment
Hide comment
@jackhumbert

jackhumbert Jun 27, 2016

Member

Awesome :) on the order in the process_record_quantum - the music and midi should be called before anything else because they pull data from the row/col position, not keycodes, and I think we can conditionally call it with the TAP_DANCE_ENABLE call, right? We could do that with the Makefile as well.

Member

jackhumbert commented Jun 27, 2016

Awesome :) on the order in the process_record_quantum - the music and midi should be called before anything else because they pull data from the row/col position, not keycodes, and I think we can conditionally call it with the TAP_DANCE_ENABLE call, right? We could do that with the Makefile as well.

@algernon

This comment has been minimized.

Show comment
Hide comment
@algernon

algernon Jun 27, 2016

Contributor

Regarding order: sure, will patch that up in a bit. I didn't use the ifdef guards, because there are dummy functions for the disabled case that do nothing. The ifdef is hidden in process_tap_dance.c, instead of potentially all over the place. As far as I could tell, the compiler is smart enough to optimize these out, so it isn't even wasteful in the binary. If it would be a waste, then only a couple of bytes...

Same reason I'm using it unconditionally in the makefile.

The primary reason I went this route, is because I wanted to make it reasonably easy for a keymap author to disable this functionality. Now, all he needs to do is set TAP_DANCE_ENABLED=no, and done. If I used ifdefs, and no dummy functions, ifdefs may be needed in the keymap code as well, if it uses some of the advanced functionality.

Contributor

algernon commented Jun 27, 2016

Regarding order: sure, will patch that up in a bit. I didn't use the ifdef guards, because there are dummy functions for the disabled case that do nothing. The ifdef is hidden in process_tap_dance.c, instead of potentially all over the place. As far as I could tell, the compiler is smart enough to optimize these out, so it isn't even wasteful in the binary. If it would be a waste, then only a couple of bytes...

Same reason I'm using it unconditionally in the makefile.

The primary reason I went this route, is because I wanted to make it reasonably easy for a keymap author to disable this functionality. Now, all he needs to do is set TAP_DANCE_ENABLED=no, and done. If I used ifdefs, and no dummy functions, ifdefs may be needed in the keymap code as well, if it uses some of the advanced functionality.

tap_dance: Move process_tap_dance further down
Process the tap dance stuff after midi and audio, because those don't
process keycodes, but row/col positions.

Signed-off-by: Gergely Nagy <algernon@madhouse-project.org>
@jackhumbert

This comment has been minimized.

Show comment
Hide comment
@jackhumbert

jackhumbert Jun 27, 2016

Member

I think that's fair, but that's not how we're treating a lot of the other functionality - for now, let's keep it all the same (ifdefs, etc), and we can talk about moving things to the dummy functions in an issue. I do like that idea :)

Looking at the example, if your header file is always included (no need for an ifdef there), you shouldn't get any errors in the keymap if it's turned off, right?

Member

jackhumbert commented Jun 27, 2016

I think that's fair, but that's not how we're treating a lot of the other functionality - for now, let's keep it all the same (ifdefs, etc), and we can talk about moving things to the dummy functions in an issue. I do like that idea :)

Looking at the example, if your header file is always included (no need for an ifdef there), you shouldn't get any errors in the keymap if it's turned off, right?

@algernon

This comment has been minimized.

Show comment
Hide comment
@algernon

algernon Jun 27, 2016

Contributor

I think that's fair, but that's not how we're treating a lot of the other functionality - for now, let's keep it all the same (ifdefs, etc), and we can talk about moving things to the dummy functions in an issue. I do like that idea :)

Allright. I don't have time to migrate to such a setup, but I'll do that tomorrow morning CEST. I don't really mind either way, this was just more natural for me :)

Looking at the example, if your header file is always included (no need for an ifdef there), you shouldn't get any errors in the keymap if it's turned off, right?

Correct, you shouldn't. If you do, then that's an error in my code, and I'll fix it ASAP.

Contributor

algernon commented Jun 27, 2016

I think that's fair, but that's not how we're treating a lot of the other functionality - for now, let's keep it all the same (ifdefs, etc), and we can talk about moving things to the dummy functions in an issue. I do like that idea :)

Allright. I don't have time to migrate to such a setup, but I'll do that tomorrow morning CEST. I don't really mind either way, this was just more natural for me :)

Looking at the example, if your header file is always included (no need for an ifdef there), you shouldn't get any errors in the keymap if it's turned off, right?

Correct, you shouldn't. If you do, then that's an error in my code, and I'll fix it ASAP.

@jackhumbert

This comment has been minimized.

Show comment
Hide comment
@jackhumbert

jackhumbert Jun 27, 2016

Member

Awesome :) sounds good! I'll make a new issue about the ifdef/dummy stuff.

Member

jackhumbert commented Jun 27, 2016

Awesome :) sounds good! I'll make a new issue about the ifdef/dummy stuff.

tap_dance: Use conditionals instead of dummy functions
To be consistent with how the rest of the quantum features are
implemented, use ifdefs instead of dummy functions.

Signed-off-by: Gergely Nagy <algernon@madhouse-project.org>
@algernon

This comment has been minimized.

Show comment
Hide comment
@algernon

algernon Jun 28, 2016

Contributor

Updated the branch, now it uses ifdefs. There's one place where a "dummy" is still used: the TD() macro, which expands to KC_NO when tap dance is disabled. I did this because ifdefs within the keymap is incredibly ugly, and this is no extra code at all when the feature is disabled.

Contributor

algernon commented Jun 28, 2016

Updated the branch, now it uses ifdefs. There's one place where a "dummy" is still used: the TD() macro, which expands to KC_NO when tap dance is disabled. I did this because ifdefs within the keymap is incredibly ugly, and this is no extra code at all when the feature is disabled.

@jackhumbert

This comment has been minimized.

Show comment
Hide comment
@jackhumbert

jackhumbert Jun 28, 2016

Member

Perfect! Thanks :)

Member

jackhumbert commented Jun 28, 2016

Perfect! Thanks :)

@jackhumbert jackhumbert merged commit 5a17c77 into qmk:quantum-keypress-process Jun 28, 2016

jackhumbert added a commit that referenced this pull request Jun 29, 2016

Moves features to their own files (process_*), adds tap dance feature (
…#460)

* non-working commit

* working

* subprojects implemented for planck

* pass a subproject variable through to c

* consolidates clueboard revisions

* thanks for letting me know about conflicts..

* turn off audio for yang's

* corrects starting paths for subprojects

* messing around with travis

* semicolon

* travis script

* travis script

* script for travis

* correct directory (probably), amend files to commit

* remove origin before adding

* git pull, correct syntax

* git checkout

* git pull origin branch

* where are we?

* where are we?

* merging

* force things to happen

* adds commit message, adds add

* rebase, no commit message

* rebase branch

* idk!

* try just pull

* fetch - merge

* specify repo branch

* checkout

* goddammit

* merge? idk

* pls

* after all

* don't split up keyboards

* syntax

* adds quick for all-keyboards

* trying out new script

* script update

* lowercase

* all keyboards

* stop replacing compiled.hex automatically

* adds if statement

* skip automated build branches

* forces push to automated build branch

* throw an add in there

* upstream?

* adds AUTOGEN

* ignore all .hex files again

* testing out new repo

* global ident

* generate script, keyboard_keymap.hex

* skip generation for now, print pandoc info, submodule update

* try trusty

* and sudo

* try generate

* updates subprojects to keyboards

* no idea

* updates to keyboards

* cleans up clueboard stuff

* setup to use local readme

* updates cluepad, planck experimental

* remove extra led.c [ci skip]

* audio and midi moved over to separate files

* chording, leader, unicode separated

* consolidate each [skip ci]

* correct include

* quantum: Add a tap dance feature (#451)

* quantum: Add a tap dance feature

With this feature one can specify keys that behave differently, based on
the amount of times they have been tapped, and when interrupted, they
get handled before the interrupter.

To make it clear how this is different from `ACTION_FUNCTION_TAP`, lets
explore a certain setup! We want one key to send `Space` on single tap,
but `Enter` on double-tap.

With `ACTION_FUNCTION_TAP`, it is quite a rain-dance to set this up, and
has the problem that when the sequence is interrupted, the interrupting
key will be send first. Thus, `SPC a` will result in `a SPC` being sent,
if they are typed within `TAPPING_TERM`. With the tap dance feature,
that'll come out as `SPC a`, correctly.

The implementation hooks into two parts of the system, to achieve this:
into `process_record_quantum()`, and the matrix scan. We need the latter
to be able to time out a tap sequence even when a key is not being
pressed, so `SPC` alone will time out and register after `TAPPING_TERM`
time.

But lets start with how to use it, first!

First, you will need `TAP_DANCE_ENABLE=yes` in your `Makefile`, because
the feature is disabled by default. This adds a little less than 1k to
the firmware size. Next, you will want to define some tap-dance keys,
which is easiest to do with the `TD()` macro, that - similar to `F()`,
takes a number, which will later be used as an index into the
`tap_dance_actions` array.

This array specifies what actions shall be taken when a tap-dance key is
in action. Currently, there are two possible options:

* `ACTION_TAP_DANCE_DOUBLE(kc1, kc2)`: Sends the `kc1` keycode when
  tapped once, `kc2` otherwise.
* `ACTION_TAP_DANCE_FN(fn)`: Calls the specified function - defined in
  the user keymap - with the current state of the tap-dance action.

The first option is enough for a lot of cases, that just want dual
roles. For example, `ACTION_TAP_DANCE(KC_SPC, KC_ENT)` will result in
`Space` being sent on single-tap, `Enter` otherwise.

And that's the bulk of it!

Do note, however, that this implementation does have some consequences:
keys do not register until either they reach the tapping ceiling, or
they time out. This means that if you hold the key, nothing happens, no
repeat, no nothing. It is possible to detect held state, and register an
action then too, but that's not implemented yet. Keys also unregister
immediately after being registered, so you can't even hold the second
tap. This is intentional, to be consistent.

And now, on to the explanation of how it works!

The main entry point is `process_tap_dance()`, called from
`process_record_quantum()`, which is run for every keypress, and our
handler gets to run early. This function checks whether the key pressed
is a tap-dance key. If it is not, and a tap-dance was in action, we
handle that first, and enqueue the newly pressed key. If it is a
tap-dance key, then we check if it is the same as the already active
one (if there's one active, that is). If it is not, we fire off the old
one first, then register the new one. If it was the same, we increment
the counter and the timer.

This means that you have `TAPPING_TERM` time to tap the key again, you
do not have to input all the taps within that timeframe. This allows for
longer tap counts, with minimal impact on responsiveness.

Our next stop is `matrix_scan_tap_dance()`. This handles the timeout of
tap-dance keys.

For the sake of flexibility, tap-dance actions can be either a pair of
keycodes, or a user function. The latter allows one to handle higher tap
counts, or do extra things, like blink the LEDs, fiddle with the
backlighting, and so on. This is accomplished by using an union, and
some clever macros.

In the end, lets see a full example!

```c
enum {
 CT_SE = 0,
 CT_CLN,
 CT_EGG
};

/* Have the above three on the keymap, TD(CT_SE), etc... */

void dance_cln (qk_tap_dance_state_t *state) {
  if (state->count == 1) {
    register_code (KC_RSFT);
    register_code (KC_SCLN);
    unregister_code (KC_SCLN);
    unregister_code (KC_RSFT);
  } else {
    register_code (KC_SCLN);
    unregister_code (KC_SCLN);
    reset_tap_dance (state);
  }
}

void dance_egg (qk_tap_dance_state_t *state) {
  if (state->count >= 100) {
    SEND_STRING ("Safety dance!");
    reset_tap_dance (state);
  }
}

const qk_tap_dance_action_t tap_dance_actions[] = {
  [CT_SE]  = ACTION_TAP_DANCE_DOUBLE (KC_SPC, KC_ENT)
 ,[CT_CLN] = ACTION_TAP_DANCE_FN (dance_cln)
 ,[CT_EGG] = ACTION_TAP_DANCE_FN (dance_egg)
};
```

This addresses #426.

Signed-off-by: Gergely Nagy <algernon@madhouse-project.org>

* hhkb: Fix the build with the new tap-dance feature

Signed-off-by: Gergely Nagy <algernon@madhouse-project.org>

* tap_dance: Move process_tap_dance further down

Process the tap dance stuff after midi and audio, because those don't
process keycodes, but row/col positions.

Signed-off-by: Gergely Nagy <algernon@madhouse-project.org>

* tap_dance: Use conditionals instead of dummy functions

To be consistent with how the rest of the quantum features are
implemented, use ifdefs instead of dummy functions.

Signed-off-by: Gergely Nagy <algernon@madhouse-project.org>

* Merge branch 'master' into quantum-keypress-process

# Conflicts:
#	Makefile
#	keyboards/planck/rev3/config.h
#	keyboards/planck/rev4/config.h

* update build script
@moonrumble

This comment has been minimized.

Show comment
Hide comment
@moonrumble

moonrumble Jul 11, 2016

This feature doesn't compile. I literally copy pasted the example provided by @algernon Am I missing something obvious?

Compiling: ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c [ERRORS] | | ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c:332:3: error: unknown field 'pair' specified in initializer | ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c:332:3: warning: missing braces around initializer | ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c:332:3: warning: (near initialization for 'tap_dance_actions[0].<anonymous>.pair') | ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c:333:2: error: unknown field 'fn' specified in initializer | ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c:333:2: warning: initialization makes integer from pointer without a cast | ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c:334:2: error: unknown field 'fn' specified in initializer | ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c:334:2: warning: initialization makes integer from pointer without a cast

moonrumble commented Jul 11, 2016

This feature doesn't compile. I literally copy pasted the example provided by @algernon Am I missing something obvious?

Compiling: ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c [ERRORS] | | ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c:332:3: error: unknown field 'pair' specified in initializer | ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c:332:3: warning: missing braces around initializer | ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c:332:3: warning: (near initialization for 'tap_dance_actions[0].<anonymous>.pair') | ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c:333:2: error: unknown field 'fn' specified in initializer | ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c:333:2: warning: initialization makes integer from pointer without a cast | ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c:334:2: error: unknown field 'fn' specified in initializer | ../../keyboards/ergodox_ez/keymaps/apatel/keymap.c:334:2: warning: initialization makes integer from pointer without a cast

@algernon

This comment has been minimized.

Show comment
Hide comment
@algernon

algernon Jul 15, 2016

Contributor

@moonrumble what OS, and which compiler are you using? Also, do you have your keymap at a cloneable place, so I could have a look as well?

Contributor

algernon commented Jul 15, 2016

@moonrumble what OS, and which compiler are you using? Also, do you have your keymap at a cloneable place, so I could have a look as well?

@moonrumble

This comment has been minimized.

Show comment
Hide comment
@moonrumble

moonrumble Jul 15, 2016

I am using windows - cygwin with MHV AVR. It is not my keymap because I can't compile yours from your personal qmk repo. I will try OSX this weekend.

moonrumble commented Jul 15, 2016

I am using windows - cygwin with MHV AVR. It is not my keymap because I can't compile yours from your personal qmk repo. I will try OSX this weekend.

@fredizzimo

This comment has been minimized.

Show comment
Hide comment
@fredizzimo

fredizzimo Jul 15, 2016

Collaborator

What error message do you get when compiling Algernon's map? The problem would be easier to diagnose, if we knew which line it complains about.

Collaborator

fredizzimo commented Jul 15, 2016

What error message do you get when compiling Algernon's map? The problem would be easier to diagnose, if we knew which line it complains about.

@fredizzimo

This comment has been minimized.

Show comment
Hide comment
@fredizzimo

fredizzimo Jul 15, 2016

Collaborator

The problem is most likely the unnamed union, which is a C11 feature. So your MHV AVR compiler is probably too old. What version do you have? And what Gcc version does the make command say?

Collaborator

fredizzimo commented Jul 15, 2016

The problem is most likely the unnamed union, which is a C11 feature. So your MHV AVR compiler is probably too old. What version do you have? And what Gcc version does the make command say?

@moonrumble

This comment has been minimized.

Show comment
Hide comment
@moonrumble

moonrumble Jul 15, 2016

Yeah the error is with unnamed union, I am using following

COLLECT_GCC=C:\Program Files (x86)\MHV AVR Tools\bin\avr-gcc.exe
COLLECT_LTO_WRAPPER=c:/program files (x86)/mhv avr tools/bin/../libexec/gcc/avr/4.5.2/lto-wrapper.exe
Target: avr
Configured with: ../gcc-4.5.2/configure --prefix=c:/mhvavrtools --host=i686-pc-mingw32 --target=avr --enable-languages=c,c++ --with-dwarf2 -enable-win32-registry=MHV-AVR-Tools --disable-nls --with-gmp=c:/mhvavrtools --with-mpfr=c:/mhvavrtools --with-mpc=c:/mhvavrtools --disable-libssp
Thread model: single

moonrumble commented Jul 15, 2016

Yeah the error is with unnamed union, I am using following

COLLECT_GCC=C:\Program Files (x86)\MHV AVR Tools\bin\avr-gcc.exe
COLLECT_LTO_WRAPPER=c:/program files (x86)/mhv avr tools/bin/../libexec/gcc/avr/4.5.2/lto-wrapper.exe
Target: avr
Configured with: ../gcc-4.5.2/configure --prefix=c:/mhvavrtools --host=i686-pc-mingw32 --target=avr --enable-languages=c,c++ --with-dwarf2 -enable-win32-registry=MHV-AVR-Tools --disable-nls --with-gmp=c:/mhvavrtools --with-mpfr=c:/mhvavrtools --with-mpc=c:/mhvavrtools --disable-libssp
Thread model: single

@fredizzimo

This comment has been minimized.

Show comment
Hide comment
@fredizzimo

fredizzimo Jul 15, 2016

Collaborator

Yes, that's a very old version of Gcc, unnamed unions were added in 4.6. And the most recen 4 series is 4.9.x

Collaborator

fredizzimo commented Jul 15, 2016

Yes, that's a very old version of Gcc, unnamed unions were added in 4.6. And the most recen 4 series is 4.9.x

@moonrumble

This comment has been minimized.

Show comment
Hide comment
@moonrumble

moonrumble Jul 15, 2016

Where can I download latest one? I was getting one from the link which is in readme under windows section.

moonrumble commented Jul 15, 2016

Where can I download latest one? I was getting one from the link which is in readme under windows section.

@fredizzimo

This comment has been minimized.

Show comment
Hide comment
@fredizzimo

fredizzimo Jul 15, 2016

Collaborator

https://infernoembedded.com/products/avr-tools/release, you are using the oldest one from 2011

Edit: For me the link in the main readme downloads the newest one.

Collaborator

fredizzimo commented Jul 15, 2016

https://infernoembedded.com/products/avr-tools/release, you are using the oldest one from 2011

Edit: For me the link in the main readme downloads the newest one.

@moonrumble

This comment has been minimized.

Show comment
Hide comment
@moonrumble

moonrumble Jul 15, 2016

This worked. I feel dumb that I didn't follow the guide. You guys are awesome.

Should this work? it doesn't work. Probably because of KC_COLN?

ACTION_TAP_DANCE_DOUBLE (KC_SCLN, KC_COLN),

moonrumble commented Jul 15, 2016

This worked. I feel dumb that I didn't follow the guide. You guys are awesome.

Should this work? it doesn't work. Probably because of KC_COLN?

ACTION_TAP_DANCE_DOUBLE (KC_SCLN, KC_COLN),

@algernon

This comment has been minimized.

Show comment
Hide comment
@algernon

algernon Jul 16, 2016

Contributor

That does not work, because of the modifiers. register_code(), which is called when you use ACTION_TAP_DANCE_DOUBLE() does not register the modifier of KC_COLN, so you have to do this "by hand", using ACTION_TAP_DANCE_FN.

Contributor

algernon commented Jul 16, 2016

That does not work, because of the modifiers. register_code(), which is called when you use ACTION_TAP_DANCE_DOUBLE() does not register the modifier of KC_COLN, so you have to do this "by hand", using ACTION_TAP_DANCE_FN.

ryaninvents pushed a commit to ryaninvents/qmk_firmware that referenced this pull request Aug 12, 2016

Moves features to their own files (process_*), adds tap dance feature (
…qmk#460)

* non-working commit

* working

* subprojects implemented for planck

* pass a subproject variable through to c

* consolidates clueboard revisions

* thanks for letting me know about conflicts..

* turn off audio for yang's

* corrects starting paths for subprojects

* messing around with travis

* semicolon

* travis script

* travis script

* script for travis

* correct directory (probably), amend files to commit

* remove origin before adding

* git pull, correct syntax

* git checkout

* git pull origin branch

* where are we?

* where are we?

* merging

* force things to happen

* adds commit message, adds add

* rebase, no commit message

* rebase branch

* idk!

* try just pull

* fetch - merge

* specify repo branch

* checkout

* goddammit

* merge? idk

* pls

* after all

* don't split up keyboards

* syntax

* adds quick for all-keyboards

* trying out new script

* script update

* lowercase

* all keyboards

* stop replacing compiled.hex automatically

* adds if statement

* skip automated build branches

* forces push to automated build branch

* throw an add in there

* upstream?

* adds AUTOGEN

* ignore all .hex files again

* testing out new repo

* global ident

* generate script, keyboard_keymap.hex

* skip generation for now, print pandoc info, submodule update

* try trusty

* and sudo

* try generate

* updates subprojects to keyboards

* no idea

* updates to keyboards

* cleans up clueboard stuff

* setup to use local readme

* updates cluepad, planck experimental

* remove extra led.c [ci skip]

* audio and midi moved over to separate files

* chording, leader, unicode separated

* consolidate each [skip ci]

* correct include

* quantum: Add a tap dance feature (qmk#451)

* quantum: Add a tap dance feature

With this feature one can specify keys that behave differently, based on
the amount of times they have been tapped, and when interrupted, they
get handled before the interrupter.

To make it clear how this is different from `ACTION_FUNCTION_TAP`, lets
explore a certain setup! We want one key to send `Space` on single tap,
but `Enter` on double-tap.

With `ACTION_FUNCTION_TAP`, it is quite a rain-dance to set this up, and
has the problem that when the sequence is interrupted, the interrupting
key will be send first. Thus, `SPC a` will result in `a SPC` being sent,
if they are typed within `TAPPING_TERM`. With the tap dance feature,
that'll come out as `SPC a`, correctly.

The implementation hooks into two parts of the system, to achieve this:
into `process_record_quantum()`, and the matrix scan. We need the latter
to be able to time out a tap sequence even when a key is not being
pressed, so `SPC` alone will time out and register after `TAPPING_TERM`
time.

But lets start with how to use it, first!

First, you will need `TAP_DANCE_ENABLE=yes` in your `Makefile`, because
the feature is disabled by default. This adds a little less than 1k to
the firmware size. Next, you will want to define some tap-dance keys,
which is easiest to do with the `TD()` macro, that - similar to `F()`,
takes a number, which will later be used as an index into the
`tap_dance_actions` array.

This array specifies what actions shall be taken when a tap-dance key is
in action. Currently, there are two possible options:

* `ACTION_TAP_DANCE_DOUBLE(kc1, kc2)`: Sends the `kc1` keycode when
  tapped once, `kc2` otherwise.
* `ACTION_TAP_DANCE_FN(fn)`: Calls the specified function - defined in
  the user keymap - with the current state of the tap-dance action.

The first option is enough for a lot of cases, that just want dual
roles. For example, `ACTION_TAP_DANCE(KC_SPC, KC_ENT)` will result in
`Space` being sent on single-tap, `Enter` otherwise.

And that's the bulk of it!

Do note, however, that this implementation does have some consequences:
keys do not register until either they reach the tapping ceiling, or
they time out. This means that if you hold the key, nothing happens, no
repeat, no nothing. It is possible to detect held state, and register an
action then too, but that's not implemented yet. Keys also unregister
immediately after being registered, so you can't even hold the second
tap. This is intentional, to be consistent.

And now, on to the explanation of how it works!

The main entry point is `process_tap_dance()`, called from
`process_record_quantum()`, which is run for every keypress, and our
handler gets to run early. This function checks whether the key pressed
is a tap-dance key. If it is not, and a tap-dance was in action, we
handle that first, and enqueue the newly pressed key. If it is a
tap-dance key, then we check if it is the same as the already active
one (if there's one active, that is). If it is not, we fire off the old
one first, then register the new one. If it was the same, we increment
the counter and the timer.

This means that you have `TAPPING_TERM` time to tap the key again, you
do not have to input all the taps within that timeframe. This allows for
longer tap counts, with minimal impact on responsiveness.

Our next stop is `matrix_scan_tap_dance()`. This handles the timeout of
tap-dance keys.

For the sake of flexibility, tap-dance actions can be either a pair of
keycodes, or a user function. The latter allows one to handle higher tap
counts, or do extra things, like blink the LEDs, fiddle with the
backlighting, and so on. This is accomplished by using an union, and
some clever macros.

In the end, lets see a full example!

```c
enum {
 CT_SE = 0,
 CT_CLN,
 CT_EGG
};

/* Have the above three on the keymap, TD(CT_SE), etc... */

void dance_cln (qk_tap_dance_state_t *state) {
  if (state->count == 1) {
    register_code (KC_RSFT);
    register_code (KC_SCLN);
    unregister_code (KC_SCLN);
    unregister_code (KC_RSFT);
  } else {
    register_code (KC_SCLN);
    unregister_code (KC_SCLN);
    reset_tap_dance (state);
  }
}

void dance_egg (qk_tap_dance_state_t *state) {
  if (state->count >= 100) {
    SEND_STRING ("Safety dance!");
    reset_tap_dance (state);
  }
}

const qk_tap_dance_action_t tap_dance_actions[] = {
  [CT_SE]  = ACTION_TAP_DANCE_DOUBLE (KC_SPC, KC_ENT)
 ,[CT_CLN] = ACTION_TAP_DANCE_FN (dance_cln)
 ,[CT_EGG] = ACTION_TAP_DANCE_FN (dance_egg)
};
```

This addresses qmk#426.

Signed-off-by: Gergely Nagy <algernon@madhouse-project.org>

* hhkb: Fix the build with the new tap-dance feature

Signed-off-by: Gergely Nagy <algernon@madhouse-project.org>

* tap_dance: Move process_tap_dance further down

Process the tap dance stuff after midi and audio, because those don't
process keycodes, but row/col positions.

Signed-off-by: Gergely Nagy <algernon@madhouse-project.org>

* tap_dance: Use conditionals instead of dummy functions

To be consistent with how the rest of the quantum features are
implemented, use ifdefs instead of dummy functions.

Signed-off-by: Gergely Nagy <algernon@madhouse-project.org>

* Merge branch 'master' into quantum-keypress-process

# Conflicts:
#	Makefile
#	keyboards/planck/rev3/config.h
#	keyboards/planck/rev4/config.h

* update build script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment