-
Notifications
You must be signed in to change notification settings - Fork 95
feat: Add RMK keymap parser and update related functionality #187
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
resolves #179 |
|
I haven't tested this in any comprehensive way, only with my primary keymap. I'll drop a link into the rmk issues and maybe others can test there |
caksoylar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution! I am very unfamiliar with RMK so apologies if I am asking things that I can learn by looking them up. Overall the code looks simple enough, but please see review comments.
If you don't get around to them I will eventually take a look myself but given the amount of asks there were for RMK so far, it might be a while.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for parsing RMK (Rust-based keyboard firmware) TOML keymap files. RMK uses a TOML-based configuration format, which differs from the devicetree format used by ZMK and JSON format used by QMK.
Key changes:
- New RmkKeymapParser class that handles TOML parsing, layer extraction, combo parsing, and physical layout detection
- Configuration option
rmk_combosfor customizing combo visualization - CLI integration with
-r/--rmk-keymapflag
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| keymap_drawer/parse/rmk.py | New parser implementation for RMK TOML keymaps with support for layers, combos, hold-taps, and one-shot modifiers |
| keymap_drawer/parse/init.py | Exports the new RmkKeymapParser class |
| keymap_drawer/main.py | Adds CLI argument and parsing logic for RMK keymaps |
| keymap_drawer/config.py | Adds rmk_combos configuration field for customizing parsed RMK combos |
| README.md | Documents RMK parsing usage and adds community example |
| CONFIGURATION.md | Documents the rmk_combos configuration option with examples |
| .gitignore | Removes the entire .gitignore file (previously contained only __pycache__) |
Comments suppressed due to low confidence (1)
keymap_drawer/parse/rmk.py:12
- Import of 'Path' is not used.
from pathlib import Path
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Cem Aksoylar <[email protected]>
willpuckett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you don't get around to them I will eventually take a look myself but given the amount of asks there were for RMK so far, it might be a while.
Thanks for taking time for the detailed review. I will work through the remaining suggestions tomorrow afternoon when I can test as I go through them...
…apping logic in rmk.py
|
I think all the review comments have been addressed now. I have tested it on a couple of different keymaps and it seems to be running well. |
Removed key position mapping for 'OSL(3)' in configuration.
No description provided.