Add ':cbor' hint for CBOR Diagnostic Notation (EDN)

[chrysn]

I often find myself sending CBOR (a format for standardized data interchange with embedded devices; binary, using a superset of JSON's data model) through defmt. CBOR comes with a human-readable format (that is a superset of JSON's serialization), which is designed with human usability and not embedded devices in mind. The diagnostic notation is exactly capable of round-tripping binary data to a human readable form and back to binary.

Currently, the best way to send CBOR over defmt is in {=[u8]:02x}, so that output such as a1, 04, 41, 2b can be copy-pasted into the right side of https://cbor.me/, so that after pressing the right-to-left green button, it shows what that means: {4: h'2B'}. Deriving Format on parsed items is an alternative, but it has severe downsides of decreasing severity:

1. it does not work on parsing failures (eg. when there are mandatory-to-understand fields in the data item that the parsing struct does not have),

more reasons

2. it discards data that the parsing struct ignores (eg. because it is optional-to-understand fields in the data),
3. it is verbose on the wire (what is a single compact 4-byte string in the example becomes several short string references interspersed with chunks of data from the compact full CBOR item)
4. deriving Format shows the structure of the parsed item, and manually implementing it to look like CBOR EDN again is manual work (and moreover, there may be situations in which one would want to see the struct's view rather than the original data view)

This PR proposes a new hint, :cbor, to defmt. If the same data [a1, 04, 41, 2b] is passed into {=[u8]:cbor}, the output is then {4: h'2B'} on display right away, which a developer can make better use of.

Note that this is not trying to hard-code an exception to the design choice that we don't take types that then need host knowledge of the type: This is not a concrete type, but a format of data serialization. It can be used with serialized data from a large set of CBOR serializing crates, and a large set of protocols and their implementations in types, using a single mechanism.

Example code

trace!("Evaluating peer's credenital {=[u8]:cbor}", id_cred_x.as_full_value());

Output:

TRACE Evaluating peer's credenital {4: h'2b'}
[...]
TRACE Evaluating peer's credenital {14: {2: "me", 8: {1: {1: 2, 2: h'2b', -1: 1, -2: h'c0112830c95d49eaa15138c1160d29f45eb2bebeb0f544c326b05cd58e43fc82', -3: h'30'}}}}

PR status

This is a working draft, but a bit minimal in that

• the error handling could be prettier and better thought-through
• I haven't even checked yet whether there are tests for all this or how they should be run
• the handling of empty CBOR sequences could be done better than just showing nothing
• so far this only works with {=[u8]:cbor} and not with {:cbor} -- maybe that's OK, maybe I'd just need to find the right spot in the code to put the code in (which will then need refactoring into a dedicated function)

I still ask for review now already because while I've seen some slight positive reaction on the Rust Embedded channel, I don't want to sink too much time in here if the project does not want to go this way at all.

[chrysn]

Seeing CI items fail one by one:

• Changelog entry is something I'll obviously just fix.
• Should DisplayHint be #[non_exhaustive]? Having that closed seems to me like it would severely impede defmt's ability to evolve compatibly.
• The cbor-edn crate has a relatively new MSRV (just b/c the crate is not old). There is an older crate available for the same purpose (cbor-diag) but it is a bit verbose in places where it would not need to be. As the author of cbor-edn, if it's just the MSRV that's causing trouble, I'm sure I can arrange things on the cbor-edn side.

[jonathanpallant]

Apologies for the delay, we were out for Christmas vacation.

The MSRV is set to support Ferrocene and isn't something we can change on this crate at this time. If you can adjust the dependencies to work with what we have, that would be great.

Adding a new variant in DisplayHint is a breaking change to anyone exhaustively matching on it. That enum should probably be non_exhaustive (which is itself a breaking change, but just the one). I'm relatively relaxed about breaking changes in demft-parser, as long we send PRs to probe-rs to pick up the new version.

[chrysn]

Thanks for your feedback!

There's now a commit in that does mark DisplayHint as non_exhaustive; when it's time to squash (can do any time, but some reviewers prefer PRs to be fast-forward), I'll re-arrange that to be the first commit. What's the convention in this crate on dealing with breaking change and semver-checks – given this is the first commit in main to change defmt-parser-next, should this also be the PR where defmt-parser's (and thus defmt's) version is bumped, or is that left for release time? (Or, if the latter, is it OK for PRs that introduce breaking changes to have that CI step fail?)

cbor-edn is now updated and tested to match defmt's MSRV. I expect a less experimental version of that crate soon (we're having a discussion on the last working group changes of the output format today; not that any of that would affect the API used here).

I'm already testing this with probe-rs-tools, which needs nothing but an update to defmt-decoder once that is released with the change.

[jonathanpallant]

We do the version bump in a release PR, and we mark breaking changes with a breaking-change tag, which stops the semver-check CI job from complaining.

I'm not against changing that policy though. Maybe having main carry the version numbers for -next would make it happier. Just leave the CHANGELOG wrangling for the release PR.

[chrysn]

I have applied some small fixes, enhancing documentation and bringing cbor-edn to its latest version. It's still an early version as the WG specifying the diagnostic format is not done with its document, but they are converging far enough that I'm confident progressing. (Note that that crate is only responsible for producing a human readable version of CBOR, and not affecting the format, so even if the WG did a 180 on everything in there, what is produced is still a human readable version of the binary data, and will stay a human readable version when updated to an RFC of EDN where it looks different).

Just leave the CHANGELOG wrangling for the release PR.

So, can I rebase this onto latest main, and remove the commt that does

--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -544,2 +544,3 @@ Initial release
 * [#902] Minor change to `impl StreamDecoder` for `Raw` and `Rzcobs`, eliding a lifetime specifier to satisfy Clippy 1.83. No observable change.
+* [#916] Support the ":cbor" display hint, adding new dependency `cbor-edn`.
 
@@ -609,2 +610,5 @@ Initial release
 
+* [#916] Mark `DisplayHint` as `non_exhaustive`. This is a breaking change.
+* [#916] Add display hint ":cbor", indicating RFC8949 encoded data to be displayed in diagnostic notation.
+
 ### [defmt-parser-v0.4.1] (2024-11-27)

from the PR?

(I think rebasing will help with some of the failing CI steps).

[jonathanpallant]

To clarify - I think adding notable changes to the CHANGELOG is good, but we want to hold off on version number bumps in these feature PRs.

[chrysn]

Thanks for the clarification. I've rebased things to sort out the CI issues, but no success ("E: Package 'qemu' has no installation candidate").

On the open issues I had originally:

• Documentation is now added to the book.
• tests: I looked around for where to place tests, but found no place that goes from calling a defmt macro to a formatted string (the comments in the book are probably not tests); where would this best be added?
• Handling of empty byte sequences is probably OK, as the documentation has been updated, and an empty sequence is conventionally rendered empty.
• I don't know defmt well enough yet to understand whether {:cbor} would even be a valid formatter for what I've used so far as {=[u8]:cbor} – should it be? (I'm happy to add it, but that's easier once I know how to run easy tests.)

[jonathanpallant]

I think ubuntu-latest moved to ubuntu-24.04 and packages have changed. I'll fix it.

[chrysn]

This is currently labelled as "waits on author"; I think the more accurate description is that this is blocked by #922; I think it is done from my side.

[edit: unblocked now, awaiting CI verdict…]

[jonathanpallant]

Fantastic. Now I just need to decide whether adding :cbor requires a Defmt Version dump (to Version 5, in this case). I don't think so, but let me check what an older decoder does with it.

[jonathanpallant]

OK, it seems fine.

With probe-rs 0.25:

INFO  Starting up...
└─ bsp_demo::__cortex_m_rt_main @ src/main.rs:17
INFO  cbor: [130, 1, 130, 2, 3]
└─ bsp_demo::__cortex_m_rt_main @ src/main.rs:22

With probe-rs 0.26 patched to use this PR's version of defmt-decoder:

INFO  Starting up...
└─ bsp_demo::__cortex_m_rt_main @ src/main.rs:17
INFO  cbor: [1, [2, 3]]
└─ bsp_demo::__cortex_m_rt_main @ src/main.rs:22

This is because defmt-decoder uses ParserMode::ForwardsCompatible.

So no Defmt Version bump required.

[chrysn]

At least for CBOR, the rendering a la [0, 255, 1, 2, 3] that I think is the fallback on DisplayHint is a sensible rendition (it could be misinterpreted only in rare cases).

[jonathanpallant]

I don't know defmt well enough yet to understand whether {:cbor} would even be a valid formatter for what I've used so far as {=[u8]:cbor} – should it be? (I'm happy to add it, but that's easier once I know how to run easy tests.)

So as far as I know, if you say {:cbor}, and pass a byte slice for that argument position, then what gets emitted is:

• {:cbor}
• {=[u8]}
• [a, b, c, d]

i.e. any {} gets the actual format determined at compile time and emitted as an extra string.

If you do {:ts} and pass a byte slice, it prints in decimal and the :ts timestamp formatting is ignored. If you do {:x} and pass a byte slice, the byte slice is hex formatted - effectively the outer display hint applies to all the inner values.

So, {:cbor} should decode CBOR when given a byte slice argument, but do nothing when it encounters any other kind of argument. This will also allow:

#[derive(defmt::Format)]
struct Config<'a> {
   name: &'a str,
   payload: &'a [u8],
}

let c = Config { name: "test", payload: &[130, 1, 130, 2, 3] };
info!("Config is {:cbor}", c);

In this PR it currently prints:

INFO  Config is Config { name: "test", payload: [130, 1, 130, 2, 3] }
└─ bsp_demo::__cortex_m_rt_main @ src/main.rs:31

but probably should print:

INFO  Config is Config { name: "test", payload: [1, [2, 3]] }
└─ bsp_demo::__cortex_m_rt_main @ src/main.rs:31

[jonathanpallant]

Although #925 might be a problem.

[chrysn]

How will this interact here? In the pattern of core::fmt, AIU the :x top-level is stored in the formatter. That is probably unavailable in the formatter (it is information that the embedded system should not need to care about), but then the host side formatter would store the information, and apply it where something "looks for it" (as number formatting would do).

For :cbor, there's not much to store, because it only acts on arrays, so there are no nested formattings. At most, something could do

let item: Item = todo();
info!("My item is {:x}", item);

impl defmt::Format for Item {
    fn … {
        write!("I am an item with scalar {} and details {=[u8]:cbor}", self.scalar(), self.details());
    }
}

Without doing anything, the fix for #925 would affect the scalar {} part. If I want to be fancy, I could make the :cbor formatter look into that context and go with the hex versions that CBOR EDN has (but if not, it's not wrong EDN either, just ignore a hint that would also be ignored if there were any other explicit display hint in there).

[jonathanpallant]

Just to clarify, defmt::Format impls only concern themselves with writing (interned) format strings and byte-encoded scalar values to the output stream - nothing else. The Display Hints are only considered by the defmt-parser and defmt-decoder.

In your example, I expect :x to apply to self.scalar() and I expect :cbor to take precedence over :x on self.details() because :cbor has a tighter binding.

But I still think it's a bug that "{:cbor}", self.details() doesn't work and only "{=[u8]:cbor}", self.details() works.