Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Cleanup and refactoring, mostly prepared for custom values #1

Merged
merged 5 commits into from
Aug 6, 2020
Merged

Cleanup and refactoring, mostly prepared for custom values #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
046bd60
Cleanup and refactoring, mostly prepared for custom values
IsaacElenbaas Jul 28, 2020
3c1dc57
snake_case
IsaacElenbaas Jul 28, 2020
54ea226
cformat, requested changes
IsaacElenbaas Jul 28, 2020
e65a377
Docs and changed defines to not be breaking change
IsaacElenbaas Jul 30, 2020
3491f86
Fixed shift not being released and removed unnecessary flush_shift.
IsaacElenbaas Jul 31, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions docs/feature_auto_shift.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ have not released the key after the `AUTO_SHIFT_TIMEOUT` period, then a shifted
version of the key is emitted. If the time is less than the `AUTO_SHIFT_TIMEOUT`
time, or you press another key, then the normal state is emitted.

If you hold the key down, it will repeat the shifted key. If you want to repeat
the normal key, then tap it once then immediately (within `TAPPING_TERM`) hold
it down again. The ability to repeat the normal key like this will be disabled
if `TAPPING_FORCE_HOLD` is set.
If `AUTO_SHIFT_REPEAT` is defined, there is keyrepeat support. Holding the key
down will repeat the shifted key, though this can be disabled with
`AUTO_SHIFT_NO_AUTO_REPEAT`. If you want to repeat the normal key, then tap it
once then immediately (within `TAPPING_TERM`) hold it down again (this works
with the shifted value as well if auto-repeat is disabled).

## Are There Limitations to Auto Shift?

Expand All @@ -35,6 +36,11 @@ practice. As we get in a hurry, we think we have hit the key long enough for a
shifted version, but we did not. On the other hand, we may think we are tapping
the keys, but really we have held it for a little longer than anticipated.

Additionally, with keyrepeat the desired shift state can get mixed up. It will
always 'belong' to the last key pressed. For example, keyrepeating a capital
and then tapping something lowercase (whether or not it's an Auto Shift key)
will result in the capital's *key* still being held, but shift not.

## How Do I Enable Auto Shift?

Add to your `rules.mk` in the keymap folder:
Expand Down Expand Up @@ -103,6 +109,14 @@ Do not Auto Shift numeric keys, zero through nine.

Do not Auto Shift alpha characters, which include A through Z.

### AUTO_SHIFT_REPEAT (simple define)

Enables keyrepeat.

### AUTO_SHIFT_NO_AUTO_REPEAT (simple define)

Disables automatically keyrepeating when `AUTO_SHIFT_TIMEOUT` is exceeded.

## Using Auto Shift Setup

This will enable you to define three keys temporarily to increase, decrease and report your `AUTO_SHIFT_TIMEOUT`.
Expand Down
220 changes: 91 additions & 129 deletions quantum/process_keycode/process_auto_shift.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,14 @@ static uint16_t autoshift_lastkey = KC_NO;
static struct {
// Whether autoshift is enabled.
bool enabled : 1;
// Whether the last autoshifted key was released after the timeout. This
// Whether the last auto-shifted key was released after the timeout. This
// is used to replicate the last key for a tap-then-hold.
bool lastshifted : 1;
// Whether an auto-shiftable key is currently being pressed.
// Whether an auto-shiftable key has been pressed but not processed.
bool in_progress : 1;
// Whether the auto-shifted keypress has been registered.
bool registered : 1;
// Whether autoshift is currently "holding" the shift key.
bool holding_shift : 1;
} autoshift_flags = {true, false, false, false, false};
} autoshift_flags = {true, false, false, false};

void autoshift_timer_report(void) {
char display[8];
Expand All @@ -46,30 +44,6 @@ void autoshift_timer_report(void) {
send_string((const char *)display);
}

/** \brief Resets the autoshift state */
static void autoshift_flush(uint16_t now) {
if (autoshift_flags.in_progress && !autoshift_flags.registered) {
// Register the key.
register_code(autoshift_lastkey);
# if TAP_CODE_DELAY > 0
wait_ms(TAP_CODE_DELAY);
# endif
}
// Roll the autoshift_time forward for detecting tap-and-hold.
autoshift_time = now;
autoshift_flags.in_progress = false;
autoshift_flags.registered = false;
}

/** \brief Releases the shift key if it was held by auto-shift */
static void autoshift_flush_shift(void) {
if (autoshift_flags.holding_shift) {
// Release the shift key if it was simulated.
unregister_code(KC_LSFT);
}
autoshift_flags.holding_shift = false;
}

/** \brief Record the press of an autoshiftable key
*
* \return Whether the record should be further processed.
Expand All @@ -79,24 +53,35 @@ static bool autoshift_press(uint16_t keycode, keyrecord_t *record) {
return true;
}

const uint16_t elapsed = TIMER_DIFF_16(record->event.time, autoshift_time);
autoshift_flush(record->event.time);

# ifndef AUTO_SHIFT_MODIFIERS
if (get_mods() ^ (autoshift_flags.holding_shift ? MOD_BIT(KC_LSFT) : 0)) {
if (get_mods() & (~MOD_BIT(KC_LSFT))) {
return true;
}
# endif
# ifndef TAPPING_FORCE_HOLD
if (elapsed < TAPPING_TERM && keycode == autoshift_lastkey && !autoshift_flags.lastshifted) {
// Allow a tap-then-hold to hold the unshifted key.
autoshift_lastkey = KC_NO;
return true;
# ifdef AUTO_SHIFT_REPEAT
const uint16_t elapsed = TIMER_DIFF_16(record->event.time, autoshift_time);
# ifndef AUTO_SHIFT_NO_AUTO_REPEAT
if (!autoshift_flags.lastshifted) {
# endif
if (elapsed < TAPPING_TERM && keycode == autoshift_lastkey) {
// Allow a tap-then-hold for keyrepeat.
if (!autoshift_flags.lastshifted) {
register_code(autoshift_lastkey);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good if we could return true here rather than registering the key ourselves. Though you'd miss updating autoshift_time below if you returned directly.

} else {
// Simulate pressing the shift key.
add_weak_mods(MOD_BIT(KC_LSFT));
register_code(autoshift_lastkey);
}
return false;
}
# ifndef AUTO_SHIFT_NO_AUTO_REPEAT
}
# endif
# endif

// Record the keycode so we can simulate it later.
autoshift_lastkey = keycode;
autoshift_time = record->event.time;
autoshift_flags.in_progress = true;

# if !defined(NO_ACTION_ONESHOT) && !defined(NO_ACTION_TAPPING)
Expand All @@ -107,114 +92,81 @@ static bool autoshift_press(uint16_t keycode, keyrecord_t *record) {

/** \brief Registers an autoshiftable key under the right conditions
*
* If the autoshift delay has elapsed and no shift has already been registered,
* register a shift and the key.
*
* \return True if the timeout had elapsed.
*/
void autoshift_check_timeout(uint16_t now) {
const uint16_t elapsed = TIMER_DIFF_16(now, autoshift_time);
if (elapsed >= autoshift_timeout) {
// The timeout has expired, so simulate the keypress.
if (!(get_mods() & (MOD_BIT(KC_LSFT) | MOD_BIT(KC_RSFT)))) {
// Simulate pressing the shift key.
register_code(KC_LSFT);
autoshift_flags.lastshifted = true;
autoshift_flags.holding_shift = true;
}
register_code(autoshift_lastkey);
autoshift_flags.registered = true;

# if TAP_CODE_DELAY > 0
wait_ms(TAP_CODE_DELAY);
# endif
}
}

/** \brief Registers an autoshiftable key under the right conditions
*
* If the autoshift delay has elapsed and no shift has already been registered,
* register a shift and the key.
* If the autoshift delay has elapsed, register a shift and the key.
*
* If the autoshift key is released before the delay has elapsed, register the
* key without a shift.
*
* If another key is pressed, register the key.
*/
static void autoshift_check_record(uint16_t keycode, keyrecord_t *record) {
if (!autoshift_flags.in_progress) {
return;
}
static void autoshift_end(uint16_t keycode, uint16_t now, bool matrix_trigger) {
// Called on key down with KC_NO, auto-shifted key up, and timeout.
if (autoshift_flags.in_progress) {
// Process the auto-shiftable key.
autoshift_flags.in_progress = false;

// Process the release of the autoshiftable key.
if (keycode == autoshift_lastkey && !record->event.pressed) {
autoshift_flush_shift();
// Time since the initial press was recorded.
const uint16_t elapsed = TIMER_DIFF_16(record->event.time, autoshift_time);
if (!autoshift_flags.registered && elapsed < autoshift_timeout) {
// Auto-shiftable key is being released before the shift timeout;
// simulate the original press then let the usual processing take
// care of the release.
register_code(keycode);
const uint16_t elapsed = TIMER_DIFF_16(now, autoshift_time);
if (elapsed < autoshift_timeout) {
register_code(autoshift_lastkey);
autoshift_flags.lastshifted = false;
autoshift_flags.registered = true;
# if TAP_CODE_DELAY > 0
wait_ms(TAP_CODE_DELAY);
} else {
// Simulate pressing the shift key.
add_weak_mods(MOD_BIT(KC_LSFT));
register_code(autoshift_lastkey);
autoshift_flags.lastshifted = true;
# if defined(AUTO_SHIFT_REPEAT) && !defined(AUTO_SHIFT_NO_AUTO_REPEAT)
if (matrix_trigger) {
// Prevents release.
return;
}
# endif
}
autoshift_flush(record->event.time);
} else if (record->event.pressed) {
if (keycode == KC_LSFT) {
// If the user presses shift, auto-shift will not hold shift for
// them.
autoshift_flags.holding_shift = false;
} else if (!autoshift_flags.registered) {
// If the key isn't registered yet, it means the timeout hasn't
// elapsed, so register the key without additional shifting.
autoshift_flush(0);
autoshift_lastkey = KC_NO;
} else {
// The key is registered; flush any shift state for the
// non-autoshiftable key.
autoshift_flush_shift();

# if TAP_CODE_DELAY > 0
wait_ms(TAP_CODE_DELAY);
# endif
unregister_code(autoshift_lastkey);
del_weak_mods(MOD_BIT(KC_LSFT));
} else {
// Release after keyrepeat.
unregister_code(keycode);
if (keycode == autoshift_lastkey) {
// This will only fire when the key was the last auto-shiftable
// pressed. That prevents aaaaBBBB then releasing a from unshifting
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: do you mind putting quotes around the keys here ("releasing a from" is a bit hard to parse eagerly).

// later Bs (if B wasn't auto-shiftable).
del_weak_mods(MOD_BIT(KC_LSFT));
}
}
send_keyboard_report(); // del_weak_mods doesn't send one.
// Roll the autoshift_time forward for detecting tap-and-hold.
autoshift_time = now;
}

/** \brief Simulates auto-shifted key presses
/** \brief Simulates auto-shifted key releases when timeout is hit
*
* Can be called from \c matrix_scan_user so that auto-shifted keys are sent
* immediately after the timeout has expired, rather than waiting for the key
* to be released.
*/
void autoshift_matrix_scan(void) {
if (!autoshift_flags.in_progress || autoshift_flags.registered) {
return;
if (autoshift_flags.in_progress) {
const uint16_t now = timer_read();
const uint16_t elapsed = TIMER_DIFF_16(now, autoshift_time);
if (elapsed >= autoshift_timeout) {
autoshift_end(autoshift_lastkey, now, true);
}
}

autoshift_check_timeout(timer_read());
}

void autoshift_enable(void) {
autoshift_flags.enabled = true;
autoshift_flush_shift();
autoshift_flush(0);
}
void autoshift_enable(void) { autoshift_flags.enabled = true; }

void autoshift_disable(void) {
autoshift_flags.enabled = false;
autoshift_flush_shift();
autoshift_flush(0);
del_weak_mods(MOD_BIT(KC_LSFT));
}

void autoshift_toggle(void) {
if (autoshift_flags.enabled) {
autoshift_flags.enabled = false;
autoshift_flush_shift();
autoshift_flush(0);
} else {
autoshift_flags.enabled = true;
}
autoshift_flags.enabled = !autoshift_flags.enabled;
del_weak_mods(MOD_BIT(KC_LSFT));
}

bool get_autoshift_state(void) { return autoshift_flags.enabled; }
Expand All @@ -225,6 +177,14 @@ void set_autoshift_timeout(uint16_t timeout) { autoshift_timeout = timeout; }

bool process_auto_shift(uint16_t keycode, keyrecord_t *record) {
if (record->event.pressed) {
if (autoshift_flags.in_progress) {
// Evaluate previous key if there is one. Doing this elsewhere is
// more complicated and easier to break.
autoshift_end(KC_NO, record->event.time, false);
IsaacElenbaas marked this conversation as resolved.
Show resolved Hide resolved
}
// For pressing another key while keyrepeating shifted autoshift.
del_weak_mods(MOD_BIT(KC_LSFT));

switch (keycode) {
case KC_ASUP:
autoshift_timeout += 5;
Expand All @@ -247,26 +207,28 @@ bool process_auto_shift(uint16_t keycode, keyrecord_t *record) {
case KC_ASOFF:
autoshift_disable();
return true;
}
}

switch (keycode) {
# ifndef NO_AUTO_SHIFT_ALPHA
case KC_A ... KC_Z:
case KC_A ... KC_Z:
# endif
# ifndef NO_AUTO_SHIFT_NUMERIC
case KC_1 ... KC_0:
case KC_1 ... KC_0:
# endif
# ifndef NO_AUTO_SHIFT_SPECIAL
case KC_TAB:
case KC_MINUS ... KC_SLASH:
case KC_NONUS_BSLASH:
case KC_TAB:
case KC_MINUS ... KC_SLASH:
case KC_NONUS_BSLASH:
# endif
if (record->event.pressed) {
return autoshift_press(keycode, record);

default:
break;
}
} else {
autoshift_end(keycode, record->event.time, false);
return false;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would it take to rewrite this so that we could return true here (at least for the non-custom case)?

I tried to avoid eating records since the behaviour becomes less transparent to things downstream (like process_record_user).

Copy link
Author

@IsaacElenbaas IsaacElenbaas Jul 31, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically just not mergng this. It's near impossible to return true and still have control of keyrepeat to make this not a breaking change, and this refactor relies heavily on the assumption that it will be the one pressing and releasing keys (as I was preparing for custom ones). Your original pull is simpler if you want to go with only-keyrepeat, though I would probably submit another cleanup pull to make it handle shift more like this.

IMO that is more of a QMK-wide issue. We need a way to let other processes know that a key was pressed but that it's been handled and they shouldn't press anything, avoiding return false; everywhere is a pain.

}
}

autoshift_check_record(keycode, record);
return true;
}

Expand Down