-
Notifications
You must be signed in to change notification settings - Fork 19
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 optional automatic defmt implementation #42
Conversation
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.
Your changes are quite good. Still, I would change a few parts to make it easier to add new formatting/logging crates in the future, if the need arises.
I'll start on these changes, I just have a few clarifying questions first... |
Ok, I removed the extra If you're keeping the separate boolean flags, some other projects use e.g. |
Ok, thanks for the changes. I merge this. |
Let's do this in another PR, as accessing features inside proc macros is a bit tedious. |
Thanks!
Oh? It's more complicated than adding |
Ive extended this to support arbritrary |
This PR adds a
defmt
option to thebitfield
macro, which adds code to implementdefmt::Format
. It also adds info to the README and a few appropriate tests.Some implementation notes you might want to know:
defmt::Format
implementations on any custom types. I feel like this better matches the defaultfmt::Debug
implementation. I considered doing both, and making it configurable, but that seemed like too big a change.This macro assumes any type named
u8
is the primitiveu8
, etc. This is also what the official defmt derive macro does, to more efficiently encode these values. It looks like thebitfield
macro already makes this assumption when figuring out default bit widths, so probably not a big deal.The generated code uses
defmt
to refer to defmt, and not::defmt
. This is what the defmt crate itself does in its own macros, for some pretty subtle reasons. I've added a big comment in the code about this, to hopefully prevent somebody from 'fixing' it.Please let me know if you would like any changes, or have any questions!