feat(esp-println): Add timestamp to logging #3194
Conversation
|
I'm not against pulling in esp-hal (I am considering requiring an initializer that takes ownership of the peripheral used for logging), but we can do better I think by requiring an |
I'd much prefer an |
| @@ -17,7 +17,7 @@ use esp_hal::timer::timg::TimerGroup; | |||
| #[embassy_executor::task] | |||
| async fn run() { | |||
| loop { | |||
| esp_println::println!("Hello world from embassy using esp-hal-async!"); | |||
| log::info!("Hello world from embassy using esp-hal-async!"); | |||
There was a problem hiding this comment.
Or... well, maybe? I would probably expect println! to print the timestamp, but also it can be equally unexpected sometimes...
There was a problem hiding this comment.
I've wanted to change these to demo the feature, but I can revert them back
|
I've refactored this PR to only provide a hook gated by the |
|
Could this be done easily outside esp-println? It's just a matter of writing a custom logger I'm thinking of #1843 which was closed because esp-println was meant to stay simple. |
- Introduce new `timestamp` feature to enable logging with a timestamp in millis, in the same format as the bootloader.
Generally yes - it should stay simple - but this seems to be simple enough |
Thank you for your contribution!
We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:
Submission Checklist 📝
cargo xtask fmt-packagescommand to ensure that all changed code is formatted correctly.CHANGELOG.mdin the proper section.Extra:
Pull Request Details 📖
Description
timestamp(non-default) feature to enable logging with a timestamp in millis, in the same format as the bootloader.Considering Rust offers no standard way to fetch time through APIs or traits, this new feature requires pullingesp-halas a dependency.Testing
I've tested the changes in a local project, and it both build with and without the feature enabled.