Refactor steno and add STENO_PROTOCOL = [all|txbolt|geminipr]
#17065
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This breaks some conventions and assumptions the CLI makes about STENO_ENABLE = yes|no
STENO_DRIVER = gemini|bolt|etc |
|
Is PS: just to be clear, the rules.mk entries of the current state of the PR are not: STENO_ENABLE = GEMINI|BOLT|ALL Each protocol is a separate yes|no |
precondition
force-pushed
the
steno_refactor
branch
3 times, most recently
from
3c9c694 to
7956242
Compare
|
|
precondition
changed the title
STENO_ENABLE_[ALL|GEMINI|BOLT]STENO_PROTOCOL = [all|txbolt|geminipr]
|
The |
precondition
force-pushed
the
steno_refactor
branch
from
a8039b5 to
2c6e4e5
Compare
Closed
TXBOLT is a better name but BOLT is more consistent with the pre-existing TX Bolt related constants, which all drop the "TX " prefix
This is done so that existing keymaps invoking `steno_set_mode` don't all suddenly break
precondition
force-pushed
the
steno_refactor
branch
from
d73d50a to
7600b65
Compare
|
(Last force push was to simply rebase the PR on top of latest develop) |
drashna
approved these changes
Jun 22, 2022
KarlK90
approved these changes
Jun 22, 2022
There was a problem hiding this comment.
Thanks for the refactor and LGTM!
A additional request, it would be great if this PR could add the data driver equivalents for STENO_ENABLE and STENO_PROTOCOL to info_rules.json along with the json schema validations in keyboard.jsonschema. Would you mind adding these?
precondition
force-pushed
the
steno_refactor
branch
from
c39ef31 to
d2bd890
Compare
(Thanks dnaq)
precondition
force-pushed
the
steno_refactor
branch
from
d2bd890 to
16f19cd
Compare
KarlK90
reviewed
Jun 23, 2022
0xcharly
pushed a commit
to Bastardkb/bastardkb-qmk
that referenced
this pull request
Jul 4, 2022
…17065) * Refactor steno into STENO_ENABLE_[ALL|GEMINI|BOLT] * Update stenography documentation * STENO_ENABLE_TXBOLT → STENO_ENABLE_BOLT TXBOLT is a better name but BOLT is more consistent with the pre-existing TX Bolt related constants, which all drop the "TX " prefix * Comments * STENO_ENABLE_[GEMINI|BOLT|ALL] → STENO_PROTOCOL = [geminipr|txbolt|all] * Add note on lacking V-USB support * Clear chord at the end of the switch(mode){send_steno_chord} block * Return true if NOEVENT * update_chord_xxx → add_xxx_key_to_chord * Enable the defines for all the protocols if STENO_PROTOCOL = all * Mention how to use `steno_set_mode` * Set the default steno protocol to "all" This is done so that existing keymaps invoking `steno_set_mode` don't all suddenly break * Add data driver equivalents for stenography feature * Document format of serial steno packets (Thanks dnaq) * Add missing comma
nolanseaton
pushed a commit
to nolanseaton/qmk_firmware
that referenced
this pull request
Jan 23, 2023
…17065) * Refactor steno into STENO_ENABLE_[ALL|GEMINI|BOLT] * Update stenography documentation * STENO_ENABLE_TXBOLT → STENO_ENABLE_BOLT TXBOLT is a better name but BOLT is more consistent with the pre-existing TX Bolt related constants, which all drop the "TX " prefix * Comments * STENO_ENABLE_[GEMINI|BOLT|ALL] → STENO_PROTOCOL = [geminipr|txbolt|all] * Add note on lacking V-USB support * Clear chord at the end of the switch(mode){send_steno_chord} block * Return true if NOEVENT * update_chord_xxx → add_xxx_key_to_chord * Enable the defines for all the protocols if STENO_PROTOCOL = all * Mention how to use `steno_set_mode` * Set the default steno protocol to "all" This is done so that existing keymaps invoking `steno_set_mode` don't all suddenly break * Add data driver equivalents for stenography feature * Document format of serial steno packets (Thanks dnaq) * Add missing comma
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Refactor
quantum/process_keycode/process_steno.*so that it's easier to read and more firmware space efficient (cut the compile size by ~15% if compiling all protocols and by ~50% if compiling only a single protocol) and give the possibility to users to compile only a single protocol because it's wasteful to compile all available steno protocols even if you plan to use only one — after discussing with some folks on the Plover discord, I came to the conclusion that there are little to no use cases to the ability of switching serial protocols on the fly so using only a single serial protocol is by far the most common use case.I also fixed a bug wherein the
n_pressed_keysvalue given to thepostprocess_steno_userhook was off by 1. Speaking ofpostprocess_steno_user, shouldn't we rename this topost_process_steno_user? I avoided doing it here as that would be a breaking change but that'd be more consistent with all the other availablepost_process_*hooks.Types of Changes
Issues Fixed or Closed by This PR
Checklist