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.

Sign up for GitHub

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

Jump to bottom

Defmt support for enums and structs #76

Merged
merged 1 commit into from
May 23, 2022
Merged

Defmt support for enums and structs #76

merged 1 commit into from
May 23, 2022

Conversation

haata
Copy link
Contributor

@haata haata commented Aug 23, 2021

  • Hidden behind defmt-impl feature
    (similar to how heapless handles defmt)

@stappersg
Copy link
Contributor

But why?

Longer version:
The pull request has been seen, not yet seen by me is what it improves. Consider that it is me who isn't seen the larger picture. Please elaborate defmt and tell what it makes better.

@haata
Copy link
Contributor Author

haata commented Aug 24, 2021

USB tends to be timing sensitive, so in order to add logging inside a USB hal crate (what I'm currently working on https://github.com/atsam-rs/atsam4-hal/pull/49/files) defmt is necessary.

My current development flow is to use probe-run + defmt for logging and bracktrace support.
defmt (https://ferrous-systems.com/blog/defmt/ is a good read, but I'll give the tl;dr here) strips out debug messages from the firmware, places it on the host side, then uses a protobuf-like mechanism to pass any necessary arguments back to the host to reassemble the print message. The benefit, significantly smaller firmware images (if you have lots of messages) as well as faster logging as fewer bytes need to be sent over the debug link (e.g. RTT).

If these derive macros aren't added, I would need to maintain either a fork or wrap each enum and struct with another struct in order to handle log message formatting.

Does that make sense?

@stappersg
Copy link
Contributor

stappersg commented Aug 24, 2021 via email

@haata
Copy link
Contributor Author

haata commented Aug 29, 2021

Sorry for the delay, I've been under the weather.

ryan-summers
ryan-summers requested changes May 22, 2022
View reviewed changes
Copy link
Member

@ryan-summers ryan-summers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please also add a CHANGELOG entry for adding defmt implementations for various structures?

Cargo.toml Outdated Show resolved Hide resolved
@haata
Copy link
Contributor Author

haata commented May 22, 2022

Done

ryan-summers
ryan-summers requested changes May 23, 2022
View reviewed changes
src/bus.rs Outdated Show resolved Hide resolved
@haata
Defmt support for enums and structs
2bdc364 
USB tends to be very timing sensitive, using defmt makes it possible to
use debug prints with USB (will negatively affect timings if you use
traditional debug prints and effect the results).

- Adds CI check for the defmt feature
@ryan-summers ryan-summers merged commit 202e5ee into rust-embedded-community:master May 23, 2022
@haata
Copy link
Contributor Author

haata commented May 23, 2022

Thanks!

@ryan-summers
Copy link
Member

No problem - thanks for the help! :)

@mrdxxm mrdxxm mentioned this pull request Oct 14, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryan-summers ryan-summers ryan-summers requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@haata @stappersg @ryan-summers