esp-hal icon indicating copy to clipboard operation
esp-hal copied to clipboard

esp-println: compile time module level log filtering

Open rhomber opened this issue 1 year ago • 5 comments

Why not just model it from env_logger and send the output to the UART? The filtering that is provided by the standard log crates is really good. I'm just not sure why there's custom handling here for all the unrelated stuff vs. just working on channeling the output of how logging normally works? I'd just honor the regular env variable names too and set any internal ones you need based on them. Consistency would be awesome. Just a thought.

rhomber avatar Apr 16 '24 07:04 rhomber

Not really sure I get what you are proposing. Mind expanding on that?

Adopting the naming and format of the env-variable from env-logger and re-implementing it?

bjoernQ avatar Apr 16 '24 07:04 bjoernQ

Hey, I mean that RUST_LOG is the standard we use in rust. See: https://rust-lang-nursery.github.io/rust-cookbook/development_tools/debugging/config_log.html

Within that single variable you can define a wealth of filtering, it's very useful. I'm not entirely sure if the code that handles it is in env-logger or somewhere in the log crate. However what I am suggesting is you leverage the stuff that does all of that and simply setup a hook to channel the log output to the UART (which you can reference within env-logger as well). I did this for another project I worked on, and it turned out really well.

But my contention is that I don't want to learn how to use the logging here, I already know how to use RUST_LOG. I am not sure if your crate can filter the same way I am use to, but also don't have time to figure it out right now. I'm sure others may experience the same frustrations. I like standards, they avoid this drama.

Thanks for your cool project though :)

rhomber avatar Apr 16 '24 14:04 rhomber

These are some good points, thanks for bringing it to our attention @rhomber. I believe the original reasoning behind a different variable is because of confusion between host tooling and the target. For instance, specifying RUST_LOG=info and then using some host tooling to flash the chip would result in both the tooling and the embedded application printing info logs. I don't know if this is a big deal, or if it can be worked around, but I believe that's why we did it original. The defmt project also has its own variable for this reason.

However what I am suggesting is you leverage the stuff that does all of that and simply setup a hook to channel the log output to the UART (which you can reference within env-logger as well). I did this for another project I worked on, and it turned out really well.

Sounds interesting, would you be able to share the project, or share a minimal demonstration of what you mean?

MabezDev avatar Apr 18 '24 10:04 MabezDev

I mean, the variable isn't really my issue. It's more the contents of it. if it worked like RUST_LOG it would be very powerful :)

The code I wrote was for a company I no longer work for sadly. But if you check out the code here: https://github.com/rust-cli/env_logger/blob/main/src/logger.rs#L623

I think I may have made a custom Formatter? Either that or a custom Logger like they have. It was something like that anyway. Later I needed more complexity for like structured logging, but to get the stuff you need working it would have been fine. I'd look at custom Formatter and see if that's enough, then bundle it up and include env-logger (I think it's no-std?).

rhomber avatar Apr 18 '24 12:04 rhomber

@rhomber Would you be interested in collab on the esp-println design?

I had a look at the code, due to a separate issue, and think basing it more on what logger provides would be worth a go!

Note: I am still new to Rust on embedded, and this would make a good exercise on something real.

lure23 avatar Jun 15 '24 13:06 lure23