Skip to content

Conversation

@bjoernQ
Copy link
Contributor

@bjoernQ bjoernQ commented Sep 28, 2023

Adds ETM functionality to SYSTIMER

@bjoernQ bjoernQ force-pushed the feature/systimer-etm branch from 040acd8 to fd46802 Compare September 28, 2023 08:44

#[entry]
fn main() -> ! {
esp_println::logger::init_logger_from_env();
Copy link
Contributor

Choose a reason for hiding this comment

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

Please put these behind a #[cfg(feature = "log")]. I just cleaned up like 26 of these in #810 and that isn't a fun activity

Copy link
Contributor

Choose a reason for hiding this comment

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

Also please add the examples to CI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah I keep forgetting about the log vs defmt thing 👍 most probably will just remove it

Copy link
Contributor

Choose a reason for hiding this comment

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

Normally this should fail in CI but these examples aren't checked (yet) ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we do cargo build --examples? there are no features needed so it should build it

Copy link
Contributor

Choose a reason for hiding this comment

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

True enough. esp-println's log is currently enabled by default right now which makes this pass...

Copy link
Contributor

@JurajSadel JurajSadel left a comment

Choose a reason for hiding this comment

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

LGTM!

@bjoernQ bjoernQ merged commit 547f6db into esp-rs:main Sep 28, 2023
@bjoernQ bjoernQ deleted the feature/systimer-etm branch September 28, 2023 09:59
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.

3 participants