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.

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

Add basic support for writing keymaps in Rust #7475

Closed
wants to merge 2 commits into from

Conversation

daniel5151
Copy link
Contributor

@daniel5151 daniel5151 commented Nov 25, 2019

QMK is great.
Rust is also great.
Wouldn't it be cool to use Rust with QMK?

This PR adds Rust support to QMK in a totally optional and non-intrusive manner. This PR does not provide a complete easy-to-use user facing experience to writing keymaps, and only seeks to lay the foundations of using Rust with QMK,

In other words, this PR is all about integrating Rust with QMK's build system, and setting up some infrastructure to ergonomically use QMK features from Rust (i.e: autogenerated FFI bindings, with corresponding high-level, "idiomatic Rust" wrappers). Future PRs can improve the ergonomics of using Rust with QMK by introducing additional abstractions / macros / wrappers around QMK's FFI.

Description

Disclaimer: I'm definitely not an expert at make, and QMK's Makefiles are kind of insane, so if there's a cleaner way to do some of the things I'm doing, please let me know! Feedback / contributions are more than welcome!

With that out of the way, here is a brief overview of what this PR adds to QMK:

lib/rust/qmk-sys and lib/rust/qmk

To make working with QMK from Rust as painless as possible, I've written two Rust crates that act as the "glue" between Rust and QMK.

  • qmk-sys uses bindgen to generate FFI bindings to QMK's C/C++ headers. e.g: void send_string(const char* str); gets translated into extern "C" { pub fn send_string(str: *const ctypes::c_char); }
  • qmk builds on-top of qmk-sys, wrapping the unsafe auto-generated FFI calls in higher-level, safe to use Rust constructs.
    • This is also where certain "language level" features are defined, such as the panic-handler, and (optionally) the Global Allocator

Makefile additions (in tmk_core/rules.mk)

Rust support is totally opt-in, enables through the use of several new Makefile options:

  • Required
    • RUST_CRATE: the name of the rust crate with the user's keymap
    • RUST_TARGET: the platform to generate code for (e.g: thumbv7em-none-eabi)
  • Optional
    • RUST_TOOLCHAIN: which Rust toolchain to use (e.g: beta, nightly). Defaults to stable.
    • RUST_QMK_FEATURES: Optional features to enable in the qmk crate.
      • At the moment, the only optional feature is malloc, which enables the malloc/free based global allocator
    • RUST_QMK_HEADERS: Additional headers to generate bindings for (in qmk-sys). e.g: setting RUST_QMK_HEADERS = rgb_matrix.h will create bindings to the RGB Matrix header file.

User keymaps in Rust

While it's definitely not as "seamless" an experience to write keymaps in Rust, all-in-all, it's not too bad (especially considering this is only a first-pass)

  • run cargo new <somename> --lib in their keymap directory
  • change the the Cargo.toml file to include [lib] crate-type = ["staticlib"]
  • specify RUST_CRATE=<somename> and RUST_TARGET in rules.mk

At that point, all gloves are off, and its up to the user to implement whatever QMK methods/structures they so desire. This currently requires writing at least some extern "C" functions (to hook into QMK), but it shouldn't be too difficult to abstract these behind a nicer, more idiomatic Rust interface via a future PR to the qmk crate.

Future Work

In the context of this PR:

  • Settle on a "proper" way to integrate with QMK's Makefile
  • Try to solve the RUST_TARGET issue?
  • Document all the things!
    • We might want to remove the example keymap I currently have as part of this PR and add a simplified version of it as part of the documentation.

As mentioned earlier, the goal of this PR is to lay the foundations of using Rust with QMK, not to provide a complete easy-to-use user experience of writing keymaps in Rust.

If this PR gets merged, subsequent PRs can improve the qmk and qmk-sys crates with additional abstractions/safe wrappers over QMK's functionality. For example:

  • @houqp recently published an awesome blog post on how he wrote his keymap in Rust with QMK, and while his build-system integration isn't very "upstreamable," some of the macros and support-code that he's written would make for a wonderful addition to the qmk crate. Integrating those macros (or something similar to them) into the qmk crate would enable any user to define their layout in Rust in a cool and elegant way!
  • There are lots on "low hanging fruit" of QMK methods that could have a safe Rust wrapper. e.g: something as simple as send_string requires unsafe code to call. All a safe wrapper has to do is add a null terminator to the provided string, and call the underlying method
    • alternatively, it should be possible to rewrite certain methods in Rust entirely. e.g: instead of wrapping send_string, rewrite send_string in pure Rust, calling send_char via the FFI under-the-hood.

Oh, and if you want to see an example of something totally awesome that Rust support in QMK might enable, check out this branch of my QMK repo 😄

Known Issues

  • Rust doesn't support AVR targets. There's an unofficial fork of Rust which does support them, but mainline does not.
  • Code-bloat can be an issue. I haven't done too much research into where the bloat is coming from, but it should be possible to cut down on a lot of it.
    • On the bright side, since AVR isn't supported, we only have to worry about ARM targets, which tend to have a bit more leeway with code-size.
  • Users have to specify a RUST_TARGET themselves, which is not ideal. This sort of nitty-gritty detail should be left to the build system / keyboard-level configuration. I'm not sure if there's a clean solution here (aside from gradually specifying Rust targets on a board-by-board basis).
  • There seems to be some weird behavior with weak-linking and name-mangling going on between gcc and rustc, which prevents overriding weakly linked functions / symbols from Rust directly (e.g: process_record_user).
    • This can be worked-around somewhat for functions by using a "trampoline" method in keymap.c (e.g: define a strongly-linked version of process_record_user which calls process_record_user_rs, a method that's declared but not defined in the keymap.c file). See the included rust_example keymap for an example.

If someone with a bit more experience wants to help out and look into some of these issues, that'd be super helpful!

... Thanks for reading this (kinda massive) PR. It's a pretty big change, and one that I'd love to see merged [in some form or another] 😄

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@elfmimi
Copy link
Contributor

elfmimi commented Nov 25, 2019

Hi! I've tried your fork in OCT (daniel5151@f8e68ae) and it worked. Which is great!

I want to let you know that there it another attempt by Houqp-san. @houqp
https://about.houqp.me/posts/rusty-c/
https://github.com/houqp/qmk_firmware/tree/massdrop_houqp_rust/rust
Sorry, I read through your post after posting this comment.

I think it's good if you could also define keymaps,
const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] for long,
in keymap.rs along with process_record_user() .

@elfmimi
Copy link
Contributor

elfmimi commented Nov 25, 2019

There seems to be some weird behavior with weak-linking

It seems linker do not pick symbols from libraries over weak symbols in object files from the first place.
In Houqp's example, he is extracting a single object file from rust's build directory.
But you have LINK_TIME_OPTIMIZATION_ENABLE enabled and unfortunately this prevents Houqp's method.

What else can we do?
This is kind of a hackery but
OBJ := -Wl,-uprocess_record_user $(RUST_CRATE_RELEASE_DIR)/lib$(RUST_CRATE).a $(OBJ)
instead of
OBJ += $(RUST_CRATE_RELEASE_DIR)/lib$(RUST_CRATE).a
should solve your problem.
Of course you need to add no_mangle to fn process_record_user in keymap.rs .

@daniel5151
Copy link
Contributor Author

OBJ := -Wl,-uprocess_record_user $(RUST_CRATE_RELEASE_DIR)/lib$(RUST_CRATE).a $(OBJ)

Why not just EXTRALDFLAGS += -uprocess_record_user?

Regardless, I'm not sure how well this approach scales, since a quick grep through QMK lists ~290 instances of __attribute__((weak)) 😬

That said, having users manually write #[no_mangle] methods to hook into QMK isn't great, and should be avoided in the long term. There aught to be a set of abstractions (either macros or methods) defined in the qmk crate that take care of the nitty-gritty details of exposing these QMK lifecycle methods to Rust.

Theoretically, it could be possible to set up some codegen using the Makefile + qmk/build.rs that enumerates all the weak symbols in QMK and outputs Rust wrappers for them (using your linker flags approach, or something like my trampoline approach), but that might be finicky. A more robust, albeit less comprehensive approach would be implementing these abstractions on a method-by-method basis.

@drashna drashna requested a review from a team November 26, 2019 07:42
@drashna drashna added the core label Nov 26, 2019
@elfmimi
Copy link
Contributor

elfmimi commented Nov 26, 2019

Why not just EXTRALDFLAGS += -uprocess_record_user?

You are right. I thought the order matters but I was wrong. Yet, the order of object files and libraries do matter.

I've done further experiment. I removed keymaps from keymap.c and added it to keymap.rs .
What do you think will happen with this setup? keymaps is not a weak symbol so the linker has no choice but to pull it in. and when the linker pulls in keymaps it pulls in whole crate from librust.a and then process_record_user in keymap.rs supersede the weak one. you don't need -ukeymaps nor -uprocess_record_user.

You just need either one of
OBJ += $(RUST_CRATE_RELEASE_DIR)/lib$(RUST_CRATE).a
or
EXTRALDFLAGS += -L$(RUST_CRATE_RELEASE_DIR)/ -l$(RUST_CRATE)
Do not need both. but having both does no harm.

@stale
Copy link

stale bot commented Jan 10, 2020

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@daniel5151
Copy link
Contributor Author

daniel5151 commented Jan 12, 2020

Well, I see no one has gotten a chance to look at this PR / comment on it.

Personally, I think it would be awesome to have Rust support in QMK, but if this isn't something people are interested in, then there's no reason to keep this PR open.

@mwilliammyers
Copy link

Just adding my vote of interest!

Also, AVR targets were recently merged into the nightly compiler. rust-lang/rust#44052 (comment)

https://www.avr-rust.com

@jakeisnt
Copy link

jakeisnt commented Apr 3, 2021

Would love to see this merged! Much better experience than C, IMO.

@Geobert
Copy link

Geobert commented Jun 11, 2021

I'm all for it! I have a question thouhg: any impact on the hex size? From what I read from the linked blog post, it shouldn't, but I'm not an expert at this low level of the code :)

@tzarc tzarc changed the base branch from master to develop October 30, 2021 23:35
@tzarc
Copy link
Member

tzarc commented Nov 1, 2021

Unfortunately we're going to have to skip this at this stage.

To put it bluntly, the number of QMK collaborators with the time and appropriate knowledge of Rust for maintaining this (and explaining the implementation to others) doesn't inspire confidence that this will be workable in the long term.

Apologies for dragging this on, and whilst we appreciate the amount of work that went into this, it's just not feasible to pull it in at this stage. Sorry!

@tzarc tzarc closed this Nov 1, 2021
@daniel5151
Copy link
Contributor Author

All good, and totally understandable!

As cool as it would be to see this sort of integration, if the community doesn't have enough people enthusiastic about Rust in QMK (along with having the technical chops willing to support an implementation), then so be it.

For anyone stumbling across this PR in the future: please feel free to re-use any code / ideas from this PR, and let me know if you end up making anything particularly interesting with your Rust <-> QMK fusion!


As a taste of what could've been: I used this Rust integration to port Zork to my Planck EZ by leveraging an existing Rust Z-Machine emulator (with some light tweaks):

That's the thing about Rust - aside from being great at a language-level, using Rust gives you access to a vast number of libraries from crates.io, making it possible to rapidly implement complex functionality (as opposed to wrestling with Makefiles and how best to integrate a particular C library).

@tzarc tzarc mentioned this pull request Jan 11, 2022
4 tasks
@waynevanson
Copy link

I won't say RIP because we can dream.

To put it bluntly, the number of QMK collaborators with the time and appropriate knowledge of Rust for maintaining this (and explaining the implementation to others) doesn't inspire confidence that this will be workable in the long term.

Thank you for proper clarification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants