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

Use defmt-rtt and panic-probe for evk boards #141

Merged
merged 4 commits into from
Oct 28, 2023

Conversation

SpinFast
Copy link
Contributor

The evk boards have a built in debug probe. When running firmware with probe-rs defmt-rtt provides a very fast channel of low cost debug println's that everyone finds really helpful.

Using panic-probe with defmt-rtt has a second benefit in allowing for programs to inform probe-rs the firmware has exited cleanly. This can be used for writing samples and tests that exit.

This also makes the imxrt-log defmt feature optional to the boards otherwise the two implementations of the defmt logger cause duplicate symbols to show up.

@mciantyre
Copy link
Member

Happy to merge this or something similar. Ping me if integrating becomes troublesome, and I can help out.

This also makes the imxrt-log defmt feature optional to the boards otherwise the two implementations of the defmt logger cause duplicate symbols to show up.

I'm OK with this, but this breaking change might require extra work to integrate. Another approach is to disable all default imxrt-log features in board/Cargo.toml, then selectively enable log and defmt depending on the board.

I'm also OK with chucking imxrt-log in favor of defmt-rtt throughout. Teensy 4 users would need to fend for themselves.

@SpinFast SpinFast force-pushed the imxrt-defmt-rtt branch 2 times, most recently from c70f5db to b5f4a3b Compare October 23, 2023 21:11
@SpinFast SpinFast marked this pull request as ready for review October 23, 2023 22:11
@SpinFast
Copy link
Contributor Author

Believe I've fixed the remaining CI issues. I don't have an 1170evk to test on. I tried a few variations on not removing defmt as a default feature for imxrt-log but ran into strange issues with cargo workspaces and features.

@SpinFast SpinFast force-pushed the imxrt-defmt-rtt branch 2 times, most recently from 9d0cbaf to 14b9d21 Compare October 26, 2023 02:56
The evk boards have a built in debug probe. When running firmware with probe-rs
defmt-rtt provides a very fast channel of low cost debug println's that everyone
finds really helpful.

Using panic-probe with defmt-rtt has a second benefit in allowing for programs to
inform probe-rs the firmware has exited cleanly. This can be used for writing samples
and tests that exit.

This also makes the imxrt-log defmt feature optional to the boards otherwise
the two implementations of the defmt logger cause duplicate symbols to show up.

Tested on an 1010evk and 1060evk.
Cargo doc errored noting that the explicit target is redundent, so
remove it to fix the error.
@SpinFast
Copy link
Contributor Author

This passes CI now and doesn't remove the defmt from the default features of imxrt-log

Following the precedent of other EVKs. Tested on hardware.
We can't detect a dependency's features in our build. We can show this
by modifying the rtic_logging example to use the Frontend::Defmt
frontend, then building the example for a Teensy 4. The example fails to
build.

We can only detect our own features in our build. Add a board feature
that we can enable for a board, and rewrite the conditional includes to
use the feature.
Copy link
Member

@mciantyre mciantyre left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for working through the integration. I tested on a 1170EVK and Teensy 4. There's a few extra commits to use defmt-rtt on the 1170EVK, and one more to ensure that we can still use the defmt logging backend for the Teensy 4.

@mciantyre mciantyre merged commit e443ce9 into imxrt-rs:main Oct 28, 2023
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants