-
-
Notifications
You must be signed in to change notification settings - Fork 75
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
defmt::println!() prints a <lvl> prefix #844
Comments
I think this is a probe-rs CLI tool bug. I see it too. |
Thanks! I've created probe-rs/probe-rs#2523 to track it there. Feel free to close this issue. |
Hi! I'm responding here because I don't consider this a probe-rs issue, though I can be convinced. probe-rs uses The change on probe-rs's part may be that we previously used custom format strings, and changed that to Let me know if there is a way in probe-rs to disable |
Thanks for the links! I've taken a look at probe-rs and defmt quickly and here is my understanding:
So it looks like my problem is indeed defmt-related. My feature request would be:
In my particular case, I would probably configure it like so:
If defmt decides to use this as its default (i.e. use only 2 constants without log level, ignoring location), then even better because I won't need to pass flags to probe-rs. Opinions? |
Ah ... ugh. I appreciate you both digging in to this - I'm travelling today and only have my phone. I'll make a note to look at this Monday. |
Have a safe travel! You can read the following on Monday. I just wanted to dump it now while it's fresh in my head. Here is a commit that uses 2 different formatter depending on whether there is a log-level. However, I think there might be a better solution: Add support for if-then-else in the formatter syntax. There's already a notion of nested log segments. We could use this to implement the same kind of if-then-else as in zsh prompts. For example, the default constants could change:
We essentially extend the grammar of log segments to something like this:
The segment The segment |
I think simply not printing If you allow configuring a different string for print and log, someone might expect per-loglevel configurability, too. Even two is stretching the limit in my opinion, I really wouldn't like to expose two CLI switches just for this in probe-rs(-tools). We have a number of places where we expect a single log string, along with timestamp and location enabling flags. Doubling those won't be ergonomic. |
I don't think so. Logging and printing are 2 different use-cases (although it's true that some use printing for debugging which is what logging is for). They happen to benefit from the same optimization that defmt is doing, so are provided by the same crate, but this doesn't mean they should behave the same. In particular, the To recap the alternatives:
|
Hi, as the author of the PRs that brought about these changes, I think this is just a bug that was introduced when I refactored the My understanding is that I had originally added the
and not like this
However it was decided that At this point I think I agree that |
Perfect! Thanks a lot for the response and the link. I indeed remembered that I had to add the timestamp myself, but couldn't find the reference. Good to know it's a defmt bug and the behavior will be for |
I'll submit a PR to fix this issue |
Reverts #502 The conclusion from knurling-rs/defmt#844 (comment) is that `println` should just print its input (no timestamp, no location).
I don't think this used to be the case. Is this a mis-configuration from my part?
I don't have a small repro, but a big repro would be:
which outputs:
We can see that
defmt::debug!()
printsDEBUG
whiledefmt::println!()
prints<lvl>
. In the past, it would look like this (no<lvl>
prefix, the line would start with the timestamp, at least as far as I remember):The text was updated successfully, but these errors were encountered: