Decode breaks when not calling write!

[Dirbaio]

    struct Foo;

    impl defmt::Format for Foo {
        fn format(&self, fmt: Formatter) {}
    }

    defmt::info!("boom: {=?}", Foo);

produces

(HOST) ERROR failed to decode defmt data: [9, 0, 0, 0, 0, 0, 0, 0, 0, 8, 1, 0, 0, 0, 0, 0, 0, 0, 0]

I guess this is somewhat related to #455

[jonas-schievink]

This should not be too hard to fix, by remembering whether a Formatter was written to or not, and panicking if a custom Format impl didn't do that

[Dirbaio]

This would increase code size somewhat. Also, making Formatter zero-sized leads to big code size wins (#456), and this would prevent that.

Assuming we can break wire format, I think this can be fixed with a new trait without breaking API (also fixing #455 #458).

• New trait RawFormat, allows writing the tag and data separately. Impls must guarantee the tag is a constant for a given type.
• Primitives from the defmt crate impl RawFormat.
• derive(Format) impls RawFormat since the generated code guarantees the tag is constant.
• Formats wire format changes to be tag+data, tag+data, tag+data, .., terminator. This would allow Formats to do multiple write!() calls too!
• [RawFormat]s wire format changes to tag+len+data+data+data... This can't break since RawFormat guarantees the tag is constant.
• [RawFormat;N]s wire format changes to tag+data+data+data.., similarly.
• Remove the needs_tag optimization

// This trait would be "public but not really", in defmt::export.
// Not intended to be impl'd by the end user, only by code in `defmt`, and the macros.
// This allows us to change it in the future again if needed.
trait RawFormat {
    fn format_tag() -> u16,
    fn format_data(&self);
}

// The current Format trait stays unchanged
trait Format {
    fn format(&self, fmt: Formatter<'_>);
}

impl<T: Format + ?Sized> RawFormat for T {
    fn format_tag() -> u16 {
        internp!("{=_format}") // some new special tag
    }
    fn format_data(&self) {
       self.format(Formatter::new());
       defmt::export::write_u8(0x00); // some terminator byte
    }
}


impl<T: RawFormat> RawFormat for [T] {
    fn format_tag() -> u16 {
        internp!("{=_slice}") // some new special tag
    }
    fn format_data(&self) {
       defmt::export::write_tag(T::format_tag());
       defmt::export::write_usize(self.len());
       for x in self {
           x.format_data();
       }
    }
}

impl<T: RawFormat, const N: usize> RawFormat for [T;N] { .. etc } 

[jonas-schievink]

That sounds good, I guess we should really focus on allowing breaking changes to the wire format then

[Dirbaio]

Actually that is technically breaking: primitives like u8 will no longer implement Format, only RawFormat. It could break user code that has where T: defmt::Format bounds, but I'm not sure how common that is.

[Dirbaio]

Maybe RawFormat's existence should be part of the public API, but the method contents shouldn't be? Not sure if there's any precedent for that...

User code might want to do where T: defmt::RawFormat if they want to be generic over "anything printable".

[japaric]

this appears to already have been fixed in the main branch (possibly by #508)