decoder logging: add parameter to indicate defmt awareness
sometimes it's preferable to use an existing logger instead of a bespoke defmt-aware one. This PR makes it possible to preserve log levels in this case. It breaks a public API so requires a version bump.
Thank you for the PR. It does look good to me, but it is unclear when we want the next breaking change of defmt-decoder.
I am just wondering if it wouldn't be easier to just always set the log-level if it is Some? defmt doesn't need that but as far as I am aware it doesn't hurt either and then we wouldn't need to handle the two cases differently.
that could shave off a few lines - in that case I'd probably remove the level from the target payload.
in that case I'd probably remove the
levelfrom the target payload.
I am unsure if this is possible, since we use Option<Level> in the payload and None means println. But maybe it is, didn't try it out.
If we don't change the public api, then we could also merge this sooner, since it wouldn't be a breaking change anymore.
I am unsure if this is possible, since we use
Option<Level>in the payload andNonemeansprintln. But maybe it is, didn't try it out.
ok, probably not a good idea then. I didn't try it out either though 🙃
If we don't change the public api, then we could also merge this sooner, since it wouldn't be a breaking change anymore.
hm. any idea how to achieve that?
I guess I could introduce a new function for defmt-aware logging instead of adding a parameter to the existing one. New functions aren't actually breaking, even if they change an API surface, no?
I guess I could introduce a new function for defmt-aware logging instead of adding a parameter to the existing one. New functions aren't actually breaking, even if they change an API surface, no?
I am unsure. If someone did use defmt::* they would get an error if they happen to use the same function name.
But if we just always pass the log level I think we don't need to make any change to the API
@spookyvision There is another breaking release for defmt-decoder coming up (#762). I think we can just release your PR and that one together as soon as they are both done.
I think following code would allow to use both a defmt-aware logger and a normal one, without the need to add a new argument.
let mut builder = Record::builder();
// set Level, useful for non-defmt loggers
// defmt-aware loggers should use the level from the Payload
if let Some(level) = level {
builder.level(level);
}
let target = format!(
"{}{}",
DEFMT_TARGET_MARKER,
serde_json::to_value(Payload { timestamp, level }).unwrap()
);
log::logger().log(
&builder
.args(format_args!("{}", frame.display_message()))
.target(&target)
.module_path(module_path)
.file(file)
.line(line)
.build(),
);