Fix modifier-key and media-key macros in keymap parser#8
Open
dallanwagz wants to merge 2 commits into
Open
Conversation
The keymap parser stored modifier tokens as standalone bytes, misaligning
the 2-byte {modifier, keycode} pairs that send_keys() reads with a fixed
stride. Macros using bracketed modifier syntax produced wrong keystrokes
- [ctrl]p sent LEFT_CTRL + LEFT_SHIFT + RIGHT_CTRL instead of Ctrl+P,
and multi-modifier strings like [ctrl][shift]p silently dropped both
modifiers.
Fix introduces a pending_modifier accumulator in serialconsole.c that
collects modifier tokens (one or many) and applies them atomically to
the next keystroke.
Verified end-to-end on real DEF CON 29 badge hardware (SAMD21G16B) via
the serial console:
- Single-modifier macro [ctrl]p accepted and parsed
- Multi-modifier macros [ctrl][shift]p and [gui][shift]s accepted
- [gui][shift]s triggers Snipping Tool on the host (Win+Shift+S),
confirming both modifiers reach the OS atomically
Also rebuilds Firmware/Compiled/DC29.uf2 - the previously committed
binary was built with the wrong linker script (ORIGIN=0x0) and was
rejected by the badge's UF2 bootloader. See VALIDATION_NOTES.md for
the build configuration and a bootloader-mode gotcha worth knowing
before testing.
Two related parser bugs in serialconsole.c, same family as the issue #5 modifier-key fix in the previous commit: 1. [eject] was completely non-functional. The check `newKeystroke[x+5] != ']'` had the operator inverted — for any valid `[eject]` token, x+5 IS `]`, so the branch never fired and the token fell through to ascii parsing. 2. Eight media tokens ([play], [next], [back], [stop], [eject], [mute], [vol+], [vol-]) carried a stale `newKeystroke[x+N] != '['` lookahead that was the same broken-by-design pattern the issue #5 fix removed from the modifier branches. Effect: chaining adjacent media tokens in one macro (e.g. `[play][next]`) silently dropped the first token, because the char after its `]` is `[` (start of the next token), so the branch was skipped and gibberish ascii ended up in the keymap. Removing the lookahead makes media tokens behave like the modifier and F-key tokens already do: each token parses on its own, the loop advances past it, the next token parses on the next iteration. No new state, no new helpers — these are the minimal corrections that make the parser internally consistent.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes #5 plus two related parser bugs in the same family. Both commits
target the same root cause: stale
[x+N] != '['lookahead conditions inthe keymap parser that broke any token whose closing
]is followed byanother token's opening
[.Commits
3f9f88e) —[ctrl]p,[ctrl][shift]p,[gui][shift]setc. now produce the correct keystroke. Introduces apending_modifieraccumulator that collects modifier tokens and appliesthem atomically to the next keystroke.
[eject]typo and chained media-key macros (4abb960) —[eject]had its bracket check inverted (!= ']'instead of== ']')and was completely non-functional. Eight media tokens still carried the
stale lookahead the first commit removed from modifiers, so chains like
[play][next]lost the first token to ascii fall-through.Validated on hardware (SAMD21G16B)
All five scenarios verified end-to-end on a real DEF CON 29 badge:
[ctrl]p→ Ctrl+P (Print dialog)[ctrl][shift]p→ Ctrl+Shift+P (Command Palette)[gui][shift]s→ Snipping Tool overlay (multi-modifier compose)[eject]→ no literal text typed (pre-fix typed "eject")[mute][vol-]→ mute toggle + volume down (pre-fix typed garbage)Build: Microchip Studio Release config, ATSAMD21G16B, image fits within the
56 KB flash budget after the 8 KB UF2 bootloader (56536 bytes used, ~800 byte
headroom). See
VALIDATION_NOTES.mdfor build-config notes.