Skip to content
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

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chrysn
Copy link

@chrysn chrysn commented Dec 21, 2024

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
  1. it discards data that the parsing struct ignores (eg. because it is optional-to-understand fields in the data),
  2. 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)
  3. 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
Copy link
Author

chrysn commented Dec 21, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant