New feature: DYNAMIC_TAPPING_TERM_ENABLE

[precondition]

Description

Taking inspiration from Auto Shift's special setup keys, I've created three new quantum keys:

Key	Description
DT_PRNT	"Dynamic Tapping Term Print": Types the current tapping term, in milliseconds
DT_UP	"Dynamic Tapping Term Up": Increases the current tapping term by 5ms
DT_DOWN	"Dynamic Tapping Term Down": Decreases the current tapping term by 5ms

(Ideally, I would have used KC_TAPRP, KC_TAPUP, KC_TAPDN like the auto shift keys but those names are unfortunately longer than 7 characters)

To use them, all the user needs to do is to enable DYNAMIC_TAPPING_TERM_ENABLE in their rules.mk (I wasn't sure whether to make it a config.h option or a rules.mk option).

In order for this feature to be effective for people who use per-key tapping terms, a few changes to the syntax of the get_tapping_term function are needed. All they need to do is replace every occurrence of TAPPING_TERM in the get_tapping_term function by lowercase g_tapping_term. I want to emphasize that nothing breaks if someone keeps the classic syntax and enables the new feature.

The reason for the necessity of that change is that TAPPING_TERM is a macro that expands to a constant integer and thus cannot be changed at runtime whereas tapping_term is a variable whose value can be changed at runtime.

The intended use is to temporarily enable DYNAMIC_TAPPING_TERM_ENABLE to find a suitable tapping term value and then disable that feature and revert back to using the classic syntax for per-key tapping term settings. In fact, if someone no longer needs the tap term keys and decide to disable the feature, they will have to revert back to using TAPPING_TERM because the variable g_tapping_term is only accessible at keymap-level if DYNAMIC_TAPPING_TERM_ENABLE is set to yes.

Exposing the tapping_term variable at a keymap level can also be used to do things like change the tapping term using a rotary encoder and display its value on an OLED screen.

As you probably know, subtracting 5 from a unsigned integer whose value is 0, will cycle and produce UINT16_MAX-5. It is entirely possible to go down in the quote-unquote "negative numbers" for the tapping term if you tap TK_DOWN enough times. While I could add a if (tapping_term >= 5) in the case of TK_DOWN in the quantum/process_keycode/process_dynamic_tapping_term.c file, it wouldn't help with per-key tapping term settings. If a user has set a per key tapping term of tapping_term - 60 for key X in uint16_t get_tapping_term(uint16_t keycode, keyrecord_t *record), the if-condition of TK_DOWN won't help. Regardless, I don't think it is so bad if it cycles through 0, 5, 10, ..., UINT16_MAX-5, 0, 5 so I just let it be.

On yet another note, can someone tell me where in the list of process_* functions in docs/understanding_qmk.md should I place my process_dynamic_tapping_term? I placed it after the auto shift one because that's what my PR is heavily based on.

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Issues Fixed or Closed by This PR

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[precondition]

The errors that happen related to the TAPPING_TERM on the kingly_keys/ave keymaps are unrelated to this PR and happen even on qmk:develop (they forgot to define the tapping term in config.h)

[precondition]

The get_tapping_term line has been removed from the process_space_cadet file in qmk:develop so I'm not sure whether the dynamically remapped tapping term actually affects space cadet as it should or not.

[tzarc]

Sorry about that, GitHub decided to delete the develop branch from upstream when we merged it, despite being told otherwise. Reopened.

[drashna]

This has some merge conflicts.

[precondition]

This has some merge conflicts.

Solved

[precondition]

What about renaming the feature "TAP_TERM_CHANGE_ENABLE" and renaming the keys to KC_TCRP, KC_TCUP, KC_TCDN?

[stale]

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

[filterpaper]

PS: I know that having TAP_TERM_KEYS_ENABLE and TAPPING_TERM_PER_KEY can be confusing but that's the best name I came up with. Originally, it was TAPPING_TERM_TWEAKS_ENABLE, which is awfully long. If you have ideas for a better name for this feature, I'm all ears.

This suggestion might be too close to the breaking change schedule, but TAPPING_TERM_TWEAKS_ENABLE is more distinct and self-explanatory, despite its length; along with TAPPING_TERM_VARY_ENABLE or TAP_TERM_VARY_ENABLE.

[precondition]

PS: I know that having TAP_TERM_KEYS_ENABLE and TAPPING_TERM_PER_KEY can be confusing but that's the best name I came up with. Originally, it was TAPPING_TERM_TWEAKS_ENABLE, which is awfully long. If you have ideas for a better name for this feature, I'm all ears.

This suggestion might be too close to the breaking change schedule, but TAPPING_TERM_TWEAKS_ENABLE is more distinct and self-explanatory, despite its length; along with TAPPING_TERM_VARY_ENABLE or TAP_TERM_VARY_ENABLE.

In a sense, I also prefer TAPPING_TERM_TWEAKS_ENABLE but then the TK_ prefix will need to change to TT_, which in turn can easily be confused with the TT() layer-tap toggle.

However, you did inspired me to rename it to DYNAMIC_TAPPING_TERM which is a lot less confusing.
Additionally, git grep " DT_" doesn't give any result in the qmk_firmware repo so no prefix "conflicts" :D

[filterpaper]

Tested with no anomalies on existing TAPPING_TERM use cases.

[precondition]

I rebased on top of latest develop and squashed my commits.

[precondition]

I get 1.3kb back with the following diff, but it types out the padding spaces in front of the tapping term when pressing DT_PRNT.

--- a/quantum/process_keycode/process_dynamic_tapping_term.c
+++ b/quantum/process_keycode/process_dynamic_tapping_term.c
@@ -14,7 +14,6 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <stdio.h>
 #include "quantum.h"
 #include "process_dynamic_tapping_term.h"
 
@@ -23,11 +22,7 @@
 #endif
 
 static void tapping_term_report(void) {
-    char display[8];
-
-    snprintf(display, sizeof(display), "%d", g_tapping_term);
-
-    send_string((const char *)display);
+    send_string(get_u16_str(g_tapping_term, ' '));
 }
 
 bool process_dynamic_tapping_term(uint16_t keycode, keyrecord_t *record) {

EDIT: tzarc pointed out that I could simply increment the char pointer returned by get_u16_str until it points to a non-padding character

[precondition]

Solved a small conflict in quantum/action_tapping.c

[precondition]

Missed opportunity to merge the PR on its 1 year birthday :p

[tzarc]