Skip to content

Commit

Permalink
Simplify action_tapping logic
Browse files Browse the repository at this point in the history
This commit simplifies the logic around the tapping state machine in
action_tapping by reordering the checks and taking invariants in the
state into account.
  • Loading branch information
KarlK90 committed Mar 3, 2023
1 parent c5d0918 commit bcccd21
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 59 deletions.
2 changes: 1 addition & 1 deletion docs/feature_stenography.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ bool post_process_steno_user(uint16_t keycode, keyrecord_t *record, steno_mode_t
This function is called after a key has been processed, but before any decision about whether or not to send a chord. This is where to put hooks for things like, say, live displays of steno chords or keys.
If `IS_PRESSED(record->event)` is false, and `n_pressed_keys` is 0 or 1, the chord will be sent shortly, but has not yet been sent. This relieves you of the need of keeping track of where a packet ends and another begins.
If `record->event.pressed` is false, and `n_pressed_keys` is 0 or 1, the chord will be sent shortly, but has not yet been sent. This relieves you of the need of keeping track of where a packet ends and another begins.
The `chord` argument contains the packet of the current chord as specified by the protocol in use. This is *NOT* simply a list of chorded steno keys of the form `[STN_E, STN_U, STN_BR, STN_GR]`. Refer to the appropriate protocol section of this document to learn more about the format of the packets in your steno protocol/mode of choice.
Expand Down
2 changes: 1 addition & 1 deletion docs/ja/feature_stenography.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ bool process_steno_user(uint16_t keycode, keyrecord_t *record) { return true; }
bool post_process_steno_user(uint16_t keycode, keyrecord_t *record, steno_mode_t mode, uint8_t chord[6], int8_t pressed);
```
この関数はキーが処理された後、ただしコードを送信するかどうかを決める前に呼び出されます。`IS_PRESSED(record->event)` が false で、`pressed` が 0 または 1 の場合は、コードはまもなく送信されますが、まだ送信されてはいません。ここが速記コードあるいはキーのライブ表示などのフックを配置する場所です。
この関数はキーが処理された後、ただしコードを送信するかどうかを決める前に呼び出されます。`record->event.pressed` が false で、`pressed` が 0 または 1 の場合は、コードはまもなく送信されますが、まだ送信されてはいません。ここが速記コードあるいはキーのライブ表示などのフックを配置する場所です。
## キーコードリファレンス :id=keycode-reference
Expand Down
109 changes: 61 additions & 48 deletions quantum/action_tapping.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,10 @@
# endif
# endif

# define IS_TAPPING() IS_EVENT(tapping_key.event)
# define IS_TAPPING_PRESSED() (IS_TAPPING() && tapping_key.event.pressed)
# define IS_TAPPING_RELEASED() (IS_TAPPING() && !tapping_key.event.pressed)
# define IS_TAPPING_KEY(k) (IS_TAPPING() && KEYEQ(tapping_key.event.key, (k)))
# ifndef COMBO_ENABLE
# define IS_TAPPING_RECORD(r) (IS_EVENT(r->event) && IS_TAPPING() && KEYEQ(tapping_key.event.key, (r->event.key)))
# define IS_TAPPING_RECORD(r) (KEYEQ(tapping_key.event.key, (r->event.key)))
# else
# define IS_TAPPING_RECORD(r) (IS_EVENT(r->event) && IS_TAPPING() && KEYEQ(tapping_key.event.key, (r->event.key)) && tapping_key.keycode == r->keycode)
# define IS_TAPPING_RECORD(r) (KEYEQ(tapping_key.event.key, (r->event.key)) && tapping_key.keycode == r->keycode)
# endif
# define WITHIN_TAPPING_TERM(e) (TIMER_DIFF_16(e.time, tapping_key.event.time) < GET_TAPPING_TERM(get_record_keycode(&tapping_key, false), &tapping_key))
# define WITHIN_QUICK_TAP_TERM(e) (TIMER_DIFF_16(e.time, tapping_key.event.time) < GET_QUICK_TAP_TERM(get_record_keycode(&tapping_key, false), &tapping_key))
Expand Down Expand Up @@ -96,7 +92,7 @@ void action_tapping_process(keyrecord_t record) {
ac_dprintf("OVERFLOW: CLEAR ALL STATES\n");
clear_keyboard();
waiting_buffer_clear();
tapping_key = (keyrecord_t){};
tapping_key = (keyrecord_t){0};
}
}

Expand All @@ -123,7 +119,7 @@ void action_tapping_process(keyrecord_t record) {
* conditional uses of it are hidden inside macros named TAP_...
*/
# if (defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT)) || defined(PERMISSIVE_HOLD_PER_KEY) || defined(HOLD_ON_OTHER_KEY_PRESS_PER_KEY)
# define TAP_DEFINE_KEYCODE uint16_t tapping_keycode = get_record_keycode(&tapping_key, false)
# define TAP_DEFINE_KEYCODE const uint16_t tapping_keycode = get_record_keycode(&tapping_key, false)
# else
# define TAP_DEFINE_KEYCODE
# endif
Expand Down Expand Up @@ -175,12 +171,37 @@ void action_tapping_process(keyrecord_t record) {
*/
/* return true when key event is processed or consumed. */
bool process_tapping(keyrecord_t *keyp) {
keyevent_t event = keyp->event;
const keyevent_t event = keyp->event;

// state machine is in the "reset" state, no tapping key is to be
// processed
if (IS_NOEVENT(tapping_key.event) && IS_EVENT(event)) {
if (event.pressed && is_tap_record(keyp)) {
// the currently pressed key is a tapping key, therefore transition
// into the "pressed" tapping key state
ac_dprintf("Tapping: Start(Press tap key).\n");
tapping_key = *keyp;
process_record_tap_hint(&tapping_key);
waiting_buffer_scan_tap();
debug_tapping_key();
return true;
} else {
// the current key is just a regular key, pass it on for regular
// processing
process_record(keyp);
return true;
}
}

TAP_DEFINE_KEYCODE;

// if tapping
if (IS_TAPPING_PRESSED()) {
// process "pressed" tapping key state
if (tapping_key.event.pressed) {
if (WITHIN_TAPPING_TERM(event) || MAYBE_RETRO_SHIFTING(event)) {
if (IS_NOEVENT(event)) {
// early return for tick events
return true;
}
if (tapping_key.tap.count == 0) {
if (IS_TAPPING_RECORD(keyp) && !event.pressed) {
# if defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT)
Expand All @@ -204,7 +225,7 @@ bool process_tapping(keyrecord_t *keyp) {
// clang-format off
else if (
(
IS_RELEASED(event) && waiting_buffer_typed(event) &&
!event.pressed && waiting_buffer_typed(event) &&
TAP_GET_PERMISSIVE_HOLD
)
// Causes nested taps to not wait past TAPPING_TERM/RETRO_SHIFT
Expand All @@ -223,15 +244,15 @@ bool process_tapping(keyrecord_t *keyp) {
|| (
TAP_IS_RETRO
&& (event.key.col != tapping_key.event.key.col || event.key.row != tapping_key.event.key.row)
&& IS_RELEASED(event) && waiting_buffer_typed(event)
&& !event.pressed && waiting_buffer_typed(event)
)
)
)
) {
// clang-format on
ac_dprintf("Tapping: End. No tap. Interfered by typing key\n");
process_record(&tapping_key);
tapping_key = (keyrecord_t){};
tapping_key = (keyrecord_t){0};
debug_tapping_key();
// enqueue
return false;
Expand All @@ -240,7 +261,7 @@ bool process_tapping(keyrecord_t *keyp) {
* Without this unexpected repeating will occur with having fast repeating setting
* https://github.com/tmk/tmk_keyboard/issues/60
*/
else if (IS_RELEASED(event) && !waiting_buffer_typed(event)) {
else if (!event.pressed && !waiting_buffer_typed(event)) {
// Modifier/Layer should be retained till end of this tapping.
action_t action = layer_switch_get_action(event.key);
switch (action.kind.id) {
Expand Down Expand Up @@ -276,7 +297,7 @@ bool process_tapping(keyrecord_t *keyp) {
if (TAP_GET_HOLD_ON_OTHER_KEY_PRESS) {
ac_dprintf("Tapping: End. No tap. Interfered by pressed key\n");
process_record(&tapping_key);
tapping_key = (keyrecord_t){};
tapping_key = (keyrecord_t){0};
debug_tapping_key();
// enqueue
return false;
Expand Down Expand Up @@ -317,9 +338,7 @@ bool process_tapping(keyrecord_t *keyp) {
debug_tapping_key();
return true;
} else {
if (IS_EVENT(event)) {
ac_dprintf("Tapping: key event while last tap(>0).\n");
}
ac_dprintf("Tapping: key event while last tap(>0).\n");
process_record(keyp);
return true;
}
Expand All @@ -332,15 +351,18 @@ bool process_tapping(keyrecord_t *keyp) {
debug_event(event);
ac_dprintf("\n");
process_record(&tapping_key);
tapping_key = (keyrecord_t){};
tapping_key = (keyrecord_t){0};
debug_tapping_key();
return false;
} else {
if (IS_NOEVENT(event)) {
return true;
}
if (IS_TAPPING_RECORD(keyp) && !event.pressed) {
ac_dprintf("Tapping: End. last timeout tap release(>0).");
keyp->tap = tapping_key.tap;
process_record(keyp);
tapping_key = (keyrecord_t){};
tapping_key = (keyrecord_t){0};
return true;
} else if (is_tap_record(keyp) && event.pressed) {
if (tapping_key.tap.count > 1) {
Expand All @@ -364,16 +386,20 @@ bool process_tapping(keyrecord_t *keyp) {
debug_tapping_key();
return true;
} else {
if (IS_EVENT(event)) {
ac_dprintf("Tapping: key event while last timeout tap(>0).\n");
}
ac_dprintf("Tapping: key event while last timeout tap(>0).\n");
process_record(keyp);
return true;
}
}
}
} else if (IS_TAPPING_RELEASED()) {
}
// process "released" tapping key state
else {
if (WITHIN_TAPPING_TERM(event) || MAYBE_RETRO_SHIFTING(event)) {
if (IS_NOEVENT(event)) {
// early return for tick events
return true;
}
if (event.pressed) {
if (IS_TAPPING_RECORD(keyp)) {
if (WITHIN_QUICK_TAP_TERM(event) && !tapping_key.tap.interrupted && tapping_key.tap.count > 0) {
Expand Down Expand Up @@ -404,35 +430,20 @@ bool process_tapping(keyrecord_t *keyp) {
return true;
}
} else {
if (IS_EVENT(event)) ac_dprintf("Tapping: other key just after tap.\n");
ac_dprintf("Tapping: other key just after tap.\n");
process_record(keyp);
return true;
}
} else {
// FIX: process_action here?
// timeout. no sequential tap.
// Timeout - reset state machine.
ac_dprintf("Tapping: End(Timeout after releasing last tap): ");
debug_event(event);
ac_dprintf("\n");
tapping_key = (keyrecord_t){};
tapping_key = (keyrecord_t){0};
debug_tapping_key();
return false;
}
}
// not tapping state
else {
if (event.pressed && is_tap_record(keyp)) {
ac_dprintf("Tapping: Start(Press tap key).\n");
tapping_key = *keyp;
process_record_tap_hint(&tapping_key);
waiting_buffer_scan_tap();
debug_tapping_key();
return true;
} else {
process_record(keyp);
return true;
}
}
}

/** \brief Waiting buffer enq
Expand Down Expand Up @@ -495,14 +506,16 @@ __attribute__((unused)) bool waiting_buffer_has_anykey_pressed(void) {
* FIXME: Needs docs
*/
void waiting_buffer_scan_tap(void) {
// tapping already is settled
if (tapping_key.tap.count > 0) return;
// invalid state: tapping_key released && tap.count == 0
if (!tapping_key.event.pressed) return;
// early return if:
// - tapping already is settled
// - invalid state: tapping_key released && tap.count == 0
if ((tapping_key.tap.count > 0) || !tapping_key.event.pressed) {
return;
}

for (uint8_t i = waiting_buffer_tail; i != waiting_buffer_head; i = (i + 1) % WAITING_BUFFER_SIZE) {
keyrecord_t *candidate = &waiting_buffer[i];
if (IS_EVENT(candidate->event) && IS_TAPPING_KEY(candidate->event.key) && !candidate->event.pressed && WITHIN_TAPPING_TERM(candidate->event)) {
if (IS_EVENT(candidate->event) && KEYEQ(candidate->event.key, tapping_key.event.key) && !candidate->event.pressed && WITHIN_TAPPING_TERM(candidate->event)) {
tapping_key.tap.count = 1;
candidate->tap.count = 1;
process_record(&tapping_key);
Expand Down
6 changes: 0 additions & 6 deletions quantum/keyboard.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,6 @@ static inline bool IS_COMBOEVENT(const keyevent_t event) {
static inline bool IS_ENCODEREVENT(const keyevent_t event) {
return event.type == ENCODER_CW_EVENT || event.type == ENCODER_CCW_EVENT;
}
static inline bool IS_PRESSED(const keyevent_t event) {
return IS_EVENT(event) && event.pressed;
}
static inline bool IS_RELEASED(const keyevent_t event) {
return IS_EVENT(event) && !event.pressed;
}

/* Common keypos_t object factory */
#define MAKE_KEYPOS(row_num, col_num) ((keypos_t){.row = (row_num), .col = (col_num)})
Expand Down
6 changes: 3 additions & 3 deletions quantum/process_keycode/process_steno.c
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,13 @@ bool process_steno(uint16_t keycode, keyrecord_t *record) {
switch (keycode) {
#ifdef STENO_ENABLE_ALL
case QK_STENO_BOLT:
if (IS_PRESSED(record->event)) {
if (record->event.pressed) {
steno_set_mode(STENO_MODE_BOLT);
}
return false;

case QK_STENO_GEMINI:
if (IS_PRESSED(record->event)) {
if (record->event.pressed) {
steno_set_mode(STENO_MODE_GEMINI);
}
return false;
Expand All @@ -193,7 +193,7 @@ bool process_steno(uint16_t keycode, keyrecord_t *record) {
}
#endif // STENO_COMBINEDMAP
case STN__MIN ... STN__MAX:
if (IS_PRESSED(record->event)) {
if (record->event.pressed) {
n_pressed_keys++;
switch (mode) {
#ifdef STENO_ENABLE_BOLT
Expand Down

0 comments on commit bcccd21

Please sign in to comment.