From f3245ecfb7e16eb06bbf4624e71874b3386f4f81 Mon Sep 17 00:00:00 2001 From: Stephen Ostermiller Date: Fri, 27 Jun 2025 05:37:01 -0400 Subject: [PATCH 1/8] Use less tap dance memory. Use dynamically allocated sparse array for tap dance state, dynamically allocate tap dance state when needed and free it when the tap dance is done. --- docs/ChangeLog/20250824.md | 23 +++++ docs/features/tap_dance.md | 8 +- quantum/process_keycode/process_tap_dance.c | 101 +++++++++++++------- quantum/process_keycode/process_tap_dance.h | 10 +- tests/tap_dance/examples.c | 4 +- 5 files changed, 104 insertions(+), 42 deletions(-) create mode 100644 docs/ChangeLog/20250824.md diff --git a/docs/ChangeLog/20250824.md b/docs/ChangeLog/20250824.md new file mode 100644 index 00000000000..d8a9b0ecd83 --- /dev/null +++ b/docs/ChangeLog/20250824.md @@ -0,0 +1,23 @@ +# QMK Breaking Changes - 2025 August 24 Changelog + +## Notable Features + +### Tap Dance ([#25415](https://github.com/qmk/qmk_firmware/pull/25415)) + +Tap dance state has been removed from `tap_dance_action_t`. Instead, tap dance +state can now be obtained from a method `tap_dance_get_state(int tap_dance_idx)`. + +`action.state->xxxxx` should generally be replaced with: + +```c +tap_dance_state_t* state; +... +state = tap_dance_get_state(QK_TAP_DANCE_GET_INDEX(keycode)); +... +state->xxxxx +``` + +Eg. [Example 3 in the tap dance documentation](../tap_dance#example-3). + +Tap dance functions that obtain the state as a parameter to one of the callback +methods are unaffected. They will continue to work without changes. diff --git a/docs/features/tap_dance.md b/docs/features/tap_dance.md index d533e41aaaf..8f6887c96e4 100644 --- a/docs/features/tap_dance.md +++ b/docs/features/tap_dance.md @@ -209,11 +209,13 @@ tap_dance_action_t tap_dance_actions[] = { bool process_record_user(uint16_t keycode, keyrecord_t *record) { tap_dance_action_t *action; + tap_dance_state_t* state; switch (keycode) { - case TD(CT_CLN): // list all tap dance keycodes with tap-hold configurations - action = &tap_dance_actions[QK_TAP_DANCE_GET_INDEX(keycode)]; - if (!record->event.pressed && action->state.count && !action->state.finished) { + case TD(CT_CLN): + action = tap_dance_get(QK_TAP_DANCE_GET_INDEX(keycode)); + state = tap_dance_get_state(QK_TAP_DANCE_GET_INDEX(keycode)); + if (!record->event.pressed && state->count && !state->finished) { tap_dance_tap_hold_t *tap_hold = (tap_dance_tap_hold_t *)action->user_data; tap_code16(tap_hold->tap); } diff --git a/quantum/process_keycode/process_tap_dance.c b/quantum/process_keycode/process_tap_dance.c index 11df62763dd..c657ddfaa16 100644 --- a/quantum/process_keycode/process_tap_dance.c +++ b/quantum/process_keycode/process_tap_dance.c @@ -24,8 +24,33 @@ #include "keymap_introspection.h" static uint16_t active_td; + +// Pointer to array of state pointers. Because the size of the +// array depends on the number of tap dances, it is discovered +// via introspection and is unknown at compile time. It will +// be allocated when first used. +static tap_dance_state_t **tap_dance_states = NULL; + static uint16_t last_tap_time; +tap_dance_state_t *tap_dance_get_state(int tap_dance_idx) { + if (tap_dance_idx >= tap_dance_count()) { + return NULL; + } + if (tap_dance_states == NULL) { + // Dynamic allocation of array of NULL pointers to tap dance states + // This is never freed, this array is initialized once and used forever + tap_dance_states = calloc(tap_dance_count(), sizeof(tap_dance_state_t *)); + } + if (tap_dance_states[tap_dance_idx] == NULL) { + // Dynamic allocation of struct for the tap dance state. + // This memory will be freed when the tap dance is complete + tap_dance_states[tap_dance_idx] = calloc(1, sizeof(tap_dance_state_t)); + tap_dance_states[tap_dance_idx]->index = tap_dance_idx; + } + return tap_dance_states[tap_dance_idx]; +} + void tap_dance_pair_on_each_tap(tap_dance_state_t *state, void *user_data) { tap_dance_pair_t *pair = (tap_dance_pair_t *)user_data; @@ -86,58 +111,62 @@ static inline void _process_tap_dance_action_fn(tap_dance_state_t *state, void * } } -static inline void process_tap_dance_action_on_each_tap(tap_dance_action_t *action) { - action->state.count++; - action->state.weak_mods = get_mods(); - action->state.weak_mods |= get_weak_mods(); +static inline void process_tap_dance_action_on_each_tap(tap_dance_action_t *action, tap_dance_state_t *state) { + state->count++; + state->weak_mods = get_mods(); + state->weak_mods |= get_weak_mods(); #ifndef NO_ACTION_ONESHOT - action->state.oneshot_mods = get_oneshot_mods(); + state->oneshot_mods = get_oneshot_mods(); #endif - _process_tap_dance_action_fn(&action->state, action->user_data, action->fn.on_each_tap); + _process_tap_dance_action_fn(state, action->user_data, action->fn.on_each_tap); } -static inline void process_tap_dance_action_on_each_release(tap_dance_action_t *action) { - _process_tap_dance_action_fn(&action->state, action->user_data, action->fn.on_each_release); +static inline void process_tap_dance_action_on_each_release(tap_dance_action_t *action, tap_dance_state_t *state) { + _process_tap_dance_action_fn(state, action->user_data, action->fn.on_each_release); } -static inline void process_tap_dance_action_on_reset(tap_dance_action_t *action) { - _process_tap_dance_action_fn(&action->state, action->user_data, action->fn.on_reset); - del_weak_mods(action->state.weak_mods); +static inline void process_tap_dance_action_on_reset(tap_dance_action_t *action, tap_dance_state_t *state) { + _process_tap_dance_action_fn(state, action->user_data, action->fn.on_reset); + del_weak_mods(state->weak_mods); #ifndef NO_ACTION_ONESHOT - del_mods(action->state.oneshot_mods); + del_mods(state->oneshot_mods); #endif send_keyboard_report(); - action->state = (const tap_dance_state_t){0}; + uint8_t tap_dance_idx = state->index; + free(tap_dance_states[tap_dance_idx]); + tap_dance_states[tap_dance_idx] = NULL; } -static inline void process_tap_dance_action_on_dance_finished(tap_dance_action_t *action) { - if (!action->state.finished) { - action->state.finished = true; - add_weak_mods(action->state.weak_mods); +static inline void process_tap_dance_action_on_dance_finished(tap_dance_action_t *action, tap_dance_state_t *state) { + if (!state->finished) { + state->finished = true; + add_weak_mods(state->weak_mods); #ifndef NO_ACTION_ONESHOT - add_mods(action->state.oneshot_mods); + add_mods(state->oneshot_mods); #endif send_keyboard_report(); - _process_tap_dance_action_fn(&action->state, action->user_data, action->fn.on_dance_finished); + _process_tap_dance_action_fn(state, action->user_data, action->fn.on_dance_finished); } active_td = 0; - if (!action->state.pressed) { + if (!state->pressed) { // There will not be a key release event, so reset now. - process_tap_dance_action_on_reset(action); + process_tap_dance_action_on_reset(action, state); } } bool preprocess_tap_dance(uint16_t keycode, keyrecord_t *record) { tap_dance_action_t *action; + tap_dance_state_t *state; if (!record->event.pressed) return false; if (!active_td || keycode == active_td) return false; - action = tap_dance_get(QK_TAP_DANCE_GET_INDEX(active_td)); - action->state.interrupted = true; - action->state.interrupting_keycode = keycode; - process_tap_dance_action_on_dance_finished(action); + action = tap_dance_get(QK_TAP_DANCE_GET_INDEX(active_td)); + state = tap_dance_get_state(QK_TAP_DANCE_GET_INDEX(active_td)); + state->interrupted = true; + state->interrupting_keycode = keycode; + process_tap_dance_action_on_dance_finished(action, state); // Tap dance actions can leave some weak mods active (e.g., if the tap dance is mapped to a keycode with // modifiers), but these weak mods should not affect the keypress which interrupted the tap dance. @@ -153,6 +182,7 @@ bool preprocess_tap_dance(uint16_t keycode, keyrecord_t *record) { bool process_tap_dance(uint16_t keycode, keyrecord_t *record) { int td_index; tap_dance_action_t *action; + tap_dance_state_t *state; switch (keycode) { case QK_TAP_DANCE ... QK_TAP_DANCE_MAX: @@ -161,16 +191,17 @@ bool process_tap_dance(uint16_t keycode, keyrecord_t *record) { return false; } action = tap_dance_get(td_index); + state = tap_dance_get_state(td_index); - action->state.pressed = record->event.pressed; + state->pressed = record->event.pressed; if (record->event.pressed) { last_tap_time = timer_read(); - process_tap_dance_action_on_each_tap(action); - active_td = action->state.finished ? 0 : keycode; + process_tap_dance_action_on_each_tap(action, state); + active_td = state->finished ? 0 : keycode; } else { - process_tap_dance_action_on_each_release(action); - if (action->state.finished) { - process_tap_dance_action_on_reset(action); + process_tap_dance_action_on_each_release(action, state); + if (state->finished) { + process_tap_dance_action_on_reset(action, state); if (active_td == keycode) { active_td = 0; } @@ -185,16 +216,18 @@ bool process_tap_dance(uint16_t keycode, keyrecord_t *record) { void tap_dance_task(void) { tap_dance_action_t *action; + tap_dance_state_t *state; if (!active_td || timer_elapsed(last_tap_time) <= GET_TAPPING_TERM(active_td, &(keyrecord_t){})) return; action = tap_dance_get(QK_TAP_DANCE_GET_INDEX(active_td)); - if (!action->state.interrupted) { - process_tap_dance_action_on_dance_finished(action); + state = tap_dance_get_state(QK_TAP_DANCE_GET_INDEX(active_td)); + if (!state->interrupted) { + process_tap_dance_action_on_dance_finished(action, state); } } void reset_tap_dance(tap_dance_state_t *state) { active_td = 0; - process_tap_dance_action_on_reset((tap_dance_action_t *)state); + process_tap_dance_action_on_reset(tap_dance_get(state->index), state); } diff --git a/quantum/process_keycode/process_tap_dance.h b/quantum/process_keycode/process_tap_dance.h index 5cccbdf439a..2d04fa985a5 100644 --- a/quantum/process_keycode/process_tap_dance.h +++ b/quantum/process_keycode/process_tap_dance.h @@ -28,15 +28,15 @@ typedef struct { #ifndef NO_ACTION_ONESHOT uint8_t oneshot_mods; #endif - bool pressed : 1; - bool finished : 1; - bool interrupted : 1; + bool pressed : 1; + bool finished : 1; + bool interrupted : 1; + uint8_t index; } tap_dance_state_t; typedef void (*tap_dance_user_fn_t)(tap_dance_state_t *state, void *user_data); typedef struct tap_dance_action_t { - tap_dance_state_t state; struct { tap_dance_user_fn_t on_each_tap; tap_dance_user_fn_t on_dance_finished; @@ -80,6 +80,8 @@ typedef struct { void reset_tap_dance(tap_dance_state_t *state); +tap_dance_state_t *tap_dance_get_state(int tap_dance_idx); + /* To be used internally */ bool preprocess_tap_dance(uint16_t keycode, keyrecord_t *record); diff --git a/tests/tap_dance/examples.c b/tests/tap_dance/examples.c index 4b6bdb20908..1d28ba24d90 100644 --- a/tests/tap_dance/examples.c +++ b/tests/tap_dance/examples.c @@ -81,11 +81,13 @@ typedef struct { bool process_record_user(uint16_t keycode, keyrecord_t *record) { tap_dance_action_t *action; + tap_dance_state_t* state; switch (keycode) { case TD(CT_CLN): action = tap_dance_get(QK_TAP_DANCE_GET_INDEX(keycode)); - if (!record->event.pressed && action->state.count && !action->state.finished) { + state = tap_dance_get_state(QK_TAP_DANCE_GET_INDEX(keycode)); + if (!record->event.pressed && state->count && !state->finished) { tap_dance_tap_hold_t *tap_hold = (tap_dance_tap_hold_t *)action->user_data; tap_code16(tap_hold->tap); } From 2809c68d030b5b73ad4a5fb5e59ceeb2333bfdec Mon Sep 17 00:00:00 2001 From: Stephen Ostermiller Date: Fri, 27 Jun 2025 16:08:08 -0400 Subject: [PATCH 2/8] new approach --- docs/ChangeLog/20250824.md | 23 ---------- docs/ChangeLog/20250831/pr25415.md | 3 ++ quantum/process_keycode/process_tap_dance.c | 51 ++++++++++++--------- quantum/process_keycode/process_tap_dance.h | 10 ++-- 4 files changed, 38 insertions(+), 49 deletions(-) delete mode 100644 docs/ChangeLog/20250824.md create mode 100644 docs/ChangeLog/20250831/pr25415.md diff --git a/docs/ChangeLog/20250824.md b/docs/ChangeLog/20250824.md deleted file mode 100644 index d8a9b0ecd83..00000000000 --- a/docs/ChangeLog/20250824.md +++ /dev/null @@ -1,23 +0,0 @@ -# QMK Breaking Changes - 2025 August 24 Changelog - -## Notable Features - -### Tap Dance ([#25415](https://github.com/qmk/qmk_firmware/pull/25415)) - -Tap dance state has been removed from `tap_dance_action_t`. Instead, tap dance -state can now be obtained from a method `tap_dance_get_state(int tap_dance_idx)`. - -`action.state->xxxxx` should generally be replaced with: - -```c -tap_dance_state_t* state; -... -state = tap_dance_get_state(QK_TAP_DANCE_GET_INDEX(keycode)); -... -state->xxxxx -``` - -Eg. [Example 3 in the tap dance documentation](../tap_dance#example-3). - -Tap dance functions that obtain the state as a parameter to one of the callback -methods are unaffected. They will continue to work without changes. diff --git a/docs/ChangeLog/20250831/pr25415.md b/docs/ChangeLog/20250831/pr25415.md new file mode 100644 index 00000000000..e2cac6cc8ca --- /dev/null +++ b/docs/ChangeLog/20250831/pr25415.md @@ -0,0 +1,3 @@ +# Tap dance state removed from `tap_dance_action_t` + +Code that accessed the tap dance state as a field in the tap dance action should now call `tap_dance_get_state(int tap_dance_idx)` instead. diff --git a/quantum/process_keycode/process_tap_dance.c b/quantum/process_keycode/process_tap_dance.c index c657ddfaa16..04b710a2e89 100644 --- a/quantum/process_keycode/process_tap_dance.c +++ b/quantum/process_keycode/process_tap_dance.c @@ -25,30 +25,40 @@ static uint16_t active_td; -// Pointer to array of state pointers. Because the size of the -// array depends on the number of tap dances, it is discovered -// via introspection and is unknown at compile time. It will -// be allocated when first used. -static tap_dance_state_t **tap_dance_states = NULL; +#ifndef TAP_DANCE_MAX_SIMULTANEOUS +# define TAP_DANCE_MAX_SIMULTANEOUS 3 +#endif + +static tap_dance_state_t tap_dance_states[TAP_DANCE_MAX_SIMULTANEOUS]; static uint16_t last_tap_time; -tap_dance_state_t *tap_dance_get_state(int tap_dance_idx) { +tap_dance_state_t *tap_dance_get_state(uint8_t tap_dance_idx) { + uint8_t i; + uint16_t keycode = QK_TAP_DANCE + tap_dance_idx; if (tap_dance_idx >= tap_dance_count()) { return NULL; } - if (tap_dance_states == NULL) { - // Dynamic allocation of array of NULL pointers to tap dance states - // This is never freed, this array is initialized once and used forever - tap_dance_states = calloc(tap_dance_count(), sizeof(tap_dance_state_t *)); + // Search for a state already used for this case + for (i = 0; i < TAP_DANCE_MAX_SIMULTANEOUS; i++) { + if (tap_dance_states[i].keycode == keycode) { + return &tap_dance_states[i]; + } } - if (tap_dance_states[tap_dance_idx] == NULL) { - // Dynamic allocation of struct for the tap dance state. - // This memory will be freed when the tap dance is complete - tap_dance_states[tap_dance_idx] = calloc(1, sizeof(tap_dance_state_t)); - tap_dance_states[tap_dance_idx]->index = tap_dance_idx; + // Search for the first free state + for (i = 0; i < TAP_DANCE_MAX_SIMULTANEOUS; i++) { + if (tap_dance_states[i].keycode == 0) { + tap_dance_states[i].keycode = keycode; + return &tap_dance_states[i]; + } } - return tap_dance_states[tap_dance_idx]; + // No states are free, reuse the last state. + // The undefined behavior that causes is + // better than returning null. All the tap dances + // don't get processed correctly, but the keyboard + // doesn't crash when more than the max tap dance + // keys are held at the same time. + return &tap_dance_states[TAP_DANCE_MAX_SIMULTANEOUS - 1]; } void tap_dance_pair_on_each_tap(tap_dance_state_t *state, void *user_data) { @@ -132,9 +142,8 @@ static inline void process_tap_dance_action_on_reset(tap_dance_action_t *action, del_mods(state->oneshot_mods); #endif send_keyboard_report(); - uint8_t tap_dance_idx = state->index; - free(tap_dance_states[tap_dance_idx]); - tap_dance_states[tap_dance_idx] = NULL; + // Clear the tap dance state and mark it as unused + memset(state, 0, sizeof(tap_dance_state_t)); } static inline void process_tap_dance_action_on_dance_finished(tap_dance_action_t *action, tap_dance_state_t *state) { @@ -180,7 +189,7 @@ bool preprocess_tap_dance(uint16_t keycode, keyrecord_t *record) { } bool process_tap_dance(uint16_t keycode, keyrecord_t *record) { - int td_index; + uint8_t td_index; tap_dance_action_t *action; tap_dance_state_t *state; @@ -229,5 +238,5 @@ void tap_dance_task(void) { void reset_tap_dance(tap_dance_state_t *state) { active_td = 0; - process_tap_dance_action_on_reset(tap_dance_get(state->index), state); + process_tap_dance_action_on_reset(tap_dance_get(QK_TAP_DANCE_GET_INDEX(state->keycode)), state); } diff --git a/quantum/process_keycode/process_tap_dance.h b/quantum/process_keycode/process_tap_dance.h index 2d04fa985a5..2fd0d1d2e63 100644 --- a/quantum/process_keycode/process_tap_dance.h +++ b/quantum/process_keycode/process_tap_dance.h @@ -28,10 +28,10 @@ typedef struct { #ifndef NO_ACTION_ONESHOT uint8_t oneshot_mods; #endif - bool pressed : 1; - bool finished : 1; - bool interrupted : 1; - uint8_t index; + bool pressed : 1; + bool finished : 1; + bool interrupted : 1; + uint16_t keycode; } tap_dance_state_t; typedef void (*tap_dance_user_fn_t)(tap_dance_state_t *state, void *user_data); @@ -80,7 +80,7 @@ typedef struct { void reset_tap_dance(tap_dance_state_t *state); -tap_dance_state_t *tap_dance_get_state(int tap_dance_idx); +tap_dance_state_t *tap_dance_get_state(uint8_t tap_dance_idx); /* To be used internally */ From a10feeba4d47399ebbd07b0f86047c3230f8b75c Mon Sep 17 00:00:00 2001 From: Stephen Ostermiller Date: Fri, 27 Jun 2025 18:10:27 -0400 Subject: [PATCH 3/8] Use null, check for null --- quantum/process_keycode/process_tap_dance.c | 26 ++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/quantum/process_keycode/process_tap_dance.c b/quantum/process_keycode/process_tap_dance.c index 04b710a2e89..260d1a246ea 100644 --- a/quantum/process_keycode/process_tap_dance.c +++ b/quantum/process_keycode/process_tap_dance.c @@ -39,26 +39,21 @@ tap_dance_state_t *tap_dance_get_state(uint8_t tap_dance_idx) { if (tap_dance_idx >= tap_dance_count()) { return NULL; } - // Search for a state already used for this case + // Search for a state already used for this keycode for (i = 0; i < TAP_DANCE_MAX_SIMULTANEOUS; i++) { if (tap_dance_states[i].keycode == keycode) { return &tap_dance_states[i]; } } - // Search for the first free state + // Search for the first available state for (i = 0; i < TAP_DANCE_MAX_SIMULTANEOUS; i++) { if (tap_dance_states[i].keycode == 0) { tap_dance_states[i].keycode = keycode; return &tap_dance_states[i]; } } - // No states are free, reuse the last state. - // The undefined behavior that causes is - // better than returning null. All the tap dances - // don't get processed correctly, but the keyboard - // doesn't crash when more than the max tap dance - // keys are held at the same time. - return &tap_dance_states[TAP_DANCE_MAX_SIMULTANEOUS - 1]; + // No states are available, tap dance won't happen + return NULL; } void tap_dance_pair_on_each_tap(tap_dance_state_t *state, void *user_data) { @@ -171,8 +166,11 @@ bool preprocess_tap_dance(uint16_t keycode, keyrecord_t *record) { if (!active_td || keycode == active_td) return false; - action = tap_dance_get(QK_TAP_DANCE_GET_INDEX(active_td)); - state = tap_dance_get_state(QK_TAP_DANCE_GET_INDEX(active_td)); + action = tap_dance_get(QK_TAP_DANCE_GET_INDEX(active_td)); + state = tap_dance_get_state(QK_TAP_DANCE_GET_INDEX(active_td)); + if (state == NULL) { + return false; + } state->interrupted = true; state->interrupting_keycode = keycode; process_tap_dance_action_on_dance_finished(action, state); @@ -201,7 +199,9 @@ bool process_tap_dance(uint16_t keycode, keyrecord_t *record) { } action = tap_dance_get(td_index); state = tap_dance_get_state(td_index); - + if (state == NULL) { + return false; + } state->pressed = record->event.pressed; if (record->event.pressed) { last_tap_time = timer_read(); @@ -231,7 +231,7 @@ void tap_dance_task(void) { action = tap_dance_get(QK_TAP_DANCE_GET_INDEX(active_td)); state = tap_dance_get_state(QK_TAP_DANCE_GET_INDEX(active_td)); - if (!state->interrupted) { + if (state != NULL && !state->interrupted) { process_tap_dance_action_on_dance_finished(action, state); } } From 39ab15a52fcc0758a693bc0405bdcf5304830250 Mon Sep 17 00:00:00 2001 From: Stephen Ostermiller Date: Fri, 27 Jun 2025 18:26:20 -0400 Subject: [PATCH 4/8] Reformat with docker --- quantum/process_keycode/process_tap_dance.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/quantum/process_keycode/process_tap_dance.c b/quantum/process_keycode/process_tap_dance.c index 260d1a246ea..d4417924544 100644 --- a/quantum/process_keycode/process_tap_dance.c +++ b/quantum/process_keycode/process_tap_dance.c @@ -160,7 +160,7 @@ static inline void process_tap_dance_action_on_dance_finished(tap_dance_action_t bool preprocess_tap_dance(uint16_t keycode, keyrecord_t *record) { tap_dance_action_t *action; - tap_dance_state_t *state; + tap_dance_state_t * state; if (!record->event.pressed) return false; @@ -189,7 +189,7 @@ bool preprocess_tap_dance(uint16_t keycode, keyrecord_t *record) { bool process_tap_dance(uint16_t keycode, keyrecord_t *record) { uint8_t td_index; tap_dance_action_t *action; - tap_dance_state_t *state; + tap_dance_state_t * state; switch (keycode) { case QK_TAP_DANCE ... QK_TAP_DANCE_MAX: @@ -225,7 +225,7 @@ bool process_tap_dance(uint16_t keycode, keyrecord_t *record) { void tap_dance_task(void) { tap_dance_action_t *action; - tap_dance_state_t *state; + tap_dance_state_t * state; if (!active_td || timer_elapsed(last_tap_time) <= GET_TAPPING_TERM(active_td, &(keyrecord_t){})) return; From 425a4007ce645c2b3a7d1dc275983a973bd04c06 Mon Sep 17 00:00:00 2001 From: Stephen Ostermiller Date: Fri, 27 Jun 2025 18:58:49 -0400 Subject: [PATCH 5/8] Use uint8 with idx rather than uint16 with keycode in state --- docs/features/tap_dance.md | 2 +- quantum/process_keycode/process_tap_dance.c | 10 +++++----- quantum/process_keycode/process_tap_dance.h | 9 +++++---- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/docs/features/tap_dance.md b/docs/features/tap_dance.md index 8f6887c96e4..1841fd95c65 100644 --- a/docs/features/tap_dance.md +++ b/docs/features/tap_dance.md @@ -46,7 +46,7 @@ Well, that's the bulk of it! You should now be able to work through the examples Let's go over the three functions mentioned in `ACTION_TAP_DANCE_FN_ADVANCED` in a little more detail. They all receive the same two arguments: a pointer to a structure that holds all dance related state information, and a pointer to a use case specific state variable. The three functions differ in when they are called. The first, `on_each_tap_fn()`, is called every time the tap dance key is *pressed*. Before it is called, the counter is incremented and the timer is reset. The second function, `on_dance_finished_fn()`, is called when the tap dance is interrupted or ends because `TAPPING_TERM` milliseconds have passed since the last tap. When the `finished` field of the dance state structure is set to `true`, the `on_dance_finished_fn()` is skipped. After `on_dance_finished_fn()` was called or would have been called, but no sooner than when the tap dance key is *released*, `on_dance_reset_fn()` is called. It is possible to end a tap dance immediately, skipping `on_dance_finished_fn()`, but not `on_dance_reset_fn`, by calling `reset_tap_dance(state)`. -To accomplish this logic, the tap dance mechanics use three entry points. The main entry point is `process_tap_dance()`, called from `process_record_quantum()` *after* `process_record_kb()` and `process_record_user()`. This function is responsible for calling `on_each_tap_fn()` and `on_dance_reset_fn()`. In order to handle interruptions of a tap dance, another entry point, `preprocess_tap_dance()` is run right at the beginning of `process_record_quantum()`. 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. Finally, `tap_dance_task()` periodically checks whether `TAPPING_TERM` has passed since the last key press and finishes a tap dance if that is the case. +To accomplish this logic, the tap dance mechanics use three entry points. The main entry point is `process_tap_dance()`, called from `process_record_quantum()` *after* `process_record_kb()` and `process_record_user()`. This function is responsible for calling `on_each_tap_fn()` and `on_dance_reset_fn()`. In order to handle interruptions of a tap dance, another entry point, `preprocess_tap_dance()` is run right at the beginning of `process_record_quantum()`. 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. Finally, `½()` periodically checks whether `TAPPING_TERM` has passed since the last key press and finishes a tap dance if that is the case. This means that you have `TAPPING_TERM` time to tap the key again; you do not have to input all the taps within a single `TAPPING_TERM` timeframe. This allows for longer tap counts, with minimal impact on responsiveness. diff --git a/quantum/process_keycode/process_tap_dance.c b/quantum/process_keycode/process_tap_dance.c index d4417924544..0899ce459cc 100644 --- a/quantum/process_keycode/process_tap_dance.c +++ b/quantum/process_keycode/process_tap_dance.c @@ -35,20 +35,20 @@ static uint16_t last_tap_time; tap_dance_state_t *tap_dance_get_state(uint8_t tap_dance_idx) { uint8_t i; - uint16_t keycode = QK_TAP_DANCE + tap_dance_idx; if (tap_dance_idx >= tap_dance_count()) { return NULL; } // Search for a state already used for this keycode for (i = 0; i < TAP_DANCE_MAX_SIMULTANEOUS; i++) { - if (tap_dance_states[i].keycode == keycode) { + if (tap_dance_states[i].in_use && tap_dance_states[i].index == tap_dance_idx) { return &tap_dance_states[i]; } } // Search for the first available state for (i = 0; i < TAP_DANCE_MAX_SIMULTANEOUS; i++) { - if (tap_dance_states[i].keycode == 0) { - tap_dance_states[i].keycode = keycode; + if (!tap_dance_states[i].in_use) { + tap_dance_states[i].index = tap_dance_idx; + tap_dance_states[i].in_use = true; return &tap_dance_states[i]; } } @@ -238,5 +238,5 @@ void tap_dance_task(void) { void reset_tap_dance(tap_dance_state_t *state) { active_td = 0; - process_tap_dance_action_on_reset(tap_dance_get(QK_TAP_DANCE_GET_INDEX(state->keycode)), state); + process_tap_dance_action_on_reset(tap_dance_get(state->index), state); } diff --git a/quantum/process_keycode/process_tap_dance.h b/quantum/process_keycode/process_tap_dance.h index 2fd0d1d2e63..5a972cee5ab 100644 --- a/quantum/process_keycode/process_tap_dance.h +++ b/quantum/process_keycode/process_tap_dance.h @@ -28,10 +28,11 @@ typedef struct { #ifndef NO_ACTION_ONESHOT uint8_t oneshot_mods; #endif - bool pressed : 1; - bool finished : 1; - bool interrupted : 1; - uint16_t keycode; + bool pressed : 1; + bool finished : 1; + bool interrupted : 1; + bool in_use : 1; + uint8_t index; } tap_dance_state_t; typedef void (*tap_dance_user_fn_t)(tap_dance_state_t *state, void *user_data); From d5ee94e0b931a0a6f74e2d25b23dfb831c4a03da Mon Sep 17 00:00:00 2001 From: Stephen Ostermiller Date: Fri, 27 Jun 2025 19:01:00 -0400 Subject: [PATCH 6/8] fix accidental change --- docs/features/tap_dance.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/features/tap_dance.md b/docs/features/tap_dance.md index 1841fd95c65..8f6887c96e4 100644 --- a/docs/features/tap_dance.md +++ b/docs/features/tap_dance.md @@ -46,7 +46,7 @@ Well, that's the bulk of it! You should now be able to work through the examples Let's go over the three functions mentioned in `ACTION_TAP_DANCE_FN_ADVANCED` in a little more detail. They all receive the same two arguments: a pointer to a structure that holds all dance related state information, and a pointer to a use case specific state variable. The three functions differ in when they are called. The first, `on_each_tap_fn()`, is called every time the tap dance key is *pressed*. Before it is called, the counter is incremented and the timer is reset. The second function, `on_dance_finished_fn()`, is called when the tap dance is interrupted or ends because `TAPPING_TERM` milliseconds have passed since the last tap. When the `finished` field of the dance state structure is set to `true`, the `on_dance_finished_fn()` is skipped. After `on_dance_finished_fn()` was called or would have been called, but no sooner than when the tap dance key is *released*, `on_dance_reset_fn()` is called. It is possible to end a tap dance immediately, skipping `on_dance_finished_fn()`, but not `on_dance_reset_fn`, by calling `reset_tap_dance(state)`. -To accomplish this logic, the tap dance mechanics use three entry points. The main entry point is `process_tap_dance()`, called from `process_record_quantum()` *after* `process_record_kb()` and `process_record_user()`. This function is responsible for calling `on_each_tap_fn()` and `on_dance_reset_fn()`. In order to handle interruptions of a tap dance, another entry point, `preprocess_tap_dance()` is run right at the beginning of `process_record_quantum()`. 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. Finally, `½()` periodically checks whether `TAPPING_TERM` has passed since the last key press and finishes a tap dance if that is the case. +To accomplish this logic, the tap dance mechanics use three entry points. The main entry point is `process_tap_dance()`, called from `process_record_quantum()` *after* `process_record_kb()` and `process_record_user()`. This function is responsible for calling `on_each_tap_fn()` and `on_dance_reset_fn()`. In order to handle interruptions of a tap dance, another entry point, `preprocess_tap_dance()` is run right at the beginning of `process_record_quantum()`. 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. Finally, `tap_dance_task()` periodically checks whether `TAPPING_TERM` has passed since the last key press and finishes a tap dance if that is the case. This means that you have `TAPPING_TERM` time to tap the key again; you do not have to input all the taps within a single `TAPPING_TERM` timeframe. This allows for longer tap counts, with minimal impact on responsiveness. From 693507d759c0596b08537374cb7b3758a1ec349e Mon Sep 17 00:00:00 2001 From: Stephen Ostermiller Date: Fri, 27 Jun 2025 19:05:26 -0400 Subject: [PATCH 7/8] reformat --- quantum/process_keycode/process_tap_dance.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/quantum/process_keycode/process_tap_dance.c b/quantum/process_keycode/process_tap_dance.c index 0899ce459cc..3b5c9576800 100644 --- a/quantum/process_keycode/process_tap_dance.c +++ b/quantum/process_keycode/process_tap_dance.c @@ -34,7 +34,7 @@ static tap_dance_state_t tap_dance_states[TAP_DANCE_MAX_SIMULTANEOUS]; static uint16_t last_tap_time; tap_dance_state_t *tap_dance_get_state(uint8_t tap_dance_idx) { - uint8_t i; + uint8_t i; if (tap_dance_idx >= tap_dance_count()) { return NULL; } @@ -47,7 +47,7 @@ tap_dance_state_t *tap_dance_get_state(uint8_t tap_dance_idx) { // Search for the first available state for (i = 0; i < TAP_DANCE_MAX_SIMULTANEOUS; i++) { if (!tap_dance_states[i].in_use) { - tap_dance_states[i].index = tap_dance_idx; + tap_dance_states[i].index = tap_dance_idx; tap_dance_states[i].in_use = true; return &tap_dance_states[i]; } From 24fae67788cf798ed3916fc6dc6ee01822bb8100 Mon Sep 17 00:00:00 2001 From: Stephen Ostermiller Date: Sat, 28 Jun 2025 05:46:41 -0400 Subject: [PATCH 8/8] Add null check --- docs/ChangeLog/20250831/pr25415.md | 2 +- docs/features/tap_dance.md | 2 +- tests/tap_dance/examples.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/ChangeLog/20250831/pr25415.md b/docs/ChangeLog/20250831/pr25415.md index e2cac6cc8ca..ffa29e2ec4b 100644 --- a/docs/ChangeLog/20250831/pr25415.md +++ b/docs/ChangeLog/20250831/pr25415.md @@ -1,3 +1,3 @@ # Tap dance state removed from `tap_dance_action_t` -Code that accessed the tap dance state as a field in the tap dance action should now call `tap_dance_get_state(int tap_dance_idx)` instead. +Code that accessed the tap dance state as a field in the tap dance action should now call `tap_dance_get_state(int tap_dance_idx)` instead. That function may return `NULL` if many tap dance keys are held together. Add a `NULL` check before using the returned state. diff --git a/docs/features/tap_dance.md b/docs/features/tap_dance.md index 8f6887c96e4..f0e5ddb1742 100644 --- a/docs/features/tap_dance.md +++ b/docs/features/tap_dance.md @@ -215,7 +215,7 @@ bool process_record_user(uint16_t keycode, keyrecord_t *record) { case TD(CT_CLN): action = tap_dance_get(QK_TAP_DANCE_GET_INDEX(keycode)); state = tap_dance_get_state(QK_TAP_DANCE_GET_INDEX(keycode)); - if (!record->event.pressed && state->count && !state->finished) { + if (!record->event.pressed && state != NULL && state->count && !state->finished) { tap_dance_tap_hold_t *tap_hold = (tap_dance_tap_hold_t *)action->user_data; tap_code16(tap_hold->tap); } diff --git a/tests/tap_dance/examples.c b/tests/tap_dance/examples.c index 1d28ba24d90..6aaf0082323 100644 --- a/tests/tap_dance/examples.c +++ b/tests/tap_dance/examples.c @@ -87,7 +87,7 @@ bool process_record_user(uint16_t keycode, keyrecord_t *record) { case TD(CT_CLN): action = tap_dance_get(QK_TAP_DANCE_GET_INDEX(keycode)); state = tap_dance_get_state(QK_TAP_DANCE_GET_INDEX(keycode)); - if (!record->event.pressed && state->count && !state->finished) { + if (!record->event.pressed && state != NULL && state->count && !state->finished) { tap_dance_tap_hold_t *tap_hold = (tap_dance_tap_hold_t *)action->user_data; tap_code16(tap_hold->tap); }