Defmt log / Unknown cmd

[alexkazik]

This moves the defmt logging in the lib to a new feature.
This allows to have defmt::Format for the structures and not have the crate-internal logging.
I can rewrite it to be the other way around (new feature for only derive, but I think this is what most people expect).

Also I wondered which unknown command my computer always sends to it, so I've added the command byte to the unknown enum field.
(In my case it was PREVENT ALLOW MEDIUM REMOVAL, which I think is ok to ignore for a "usb-stick".)
There is a doc to show that scsi commands are usually always shown in hex, but debug/defmt is not.

The last commit is a fixup to the previous, which uses a newtype to enforce hex display for that command.

Fell free to pick what looks good.

[apohrebniak]

Thanks for the PR! I'll take a look this week

[alexkazik]

I've found a few improvements after looking at it now, and pushed them.

[apohrebniak]

Pleasegit fixup and force-push you changes, so I can pull all your commits at once 😃

[apohrebniak]

The idea was to rely on defmt's filtering mechanism: https://defmt.ferrous-systems.com/filtering#disabling-logs
So one would enable a defmt feature on a crate and then compile a binary with DEFMT_LOG=trace,usbd-storage=off cargo run --bin app
Do you have a different use-case?

[alexkazik]

I'm very new to defmt, if that is the usual way, I'll use it no problem

[apohrebniak]

I like the idea of printing in hex, however I would expand it to all ScsiCommands and UfiCommands and therefore for u16, u64 and u32.
The newtype is an obvious choice but I wonder if we could rely on https://defmt.ferrous-systems.com/hints#propagation instead. If I understand it correctly, then by adding something like :#X to src/fmt.rs macros, we get Hex formatting everywhere for free?

I'm not sure that derive(defmt::Format) would add =u8-like typing info, so maybe it would require an explicit implementation of defmt::Format for ScsiCommand and UfiCommand? (may put them in fmt.rs)

As a last resort, I would still try to implement newtypes for other primitives and see how much of a code change it requires. (I don't mind changing a public API. that's what semver is for :) )

[alexkazik]

I don't think propagation does help, as entries with display hint (= show hex) can't be overwritten. Also only a struct (or so) can implement Format, but since the command is a u8 it can't be formatted without newtype.

Yes, I should add =u8, and probably use {=u8:#04x} instead of 0x{:02x}.

[apohrebniak]

I tried

#[derive(defmt::Format)]
enum FakeCommand {
    Unknown {
        u16: u16,
        u32: u32,
        u64: u64,
        u128: u128,
    }
}

#[cortex_m_rt::entry]
fn main() -> ! {
    let foo = FakeCommand::Unknown{
        u16: 0xCAFE,
        u32: 0xCAFE,
        u64: 0xCAFE,
        u128: 0xCAFE,
    };
    defmt::info!("{:#X}", foo);
}

and it printed
Unknown { u16: 0xCAFE, u32: 0xCAFE, u64: 0xCAFE, u128: 0xCAFE }.

So if :#X hints are added to fmt.rs macros, then crate-internal logging will output everything in hex.

Note, that this does not currently work with u64 and u128. Hopefully knurling-rs/defmt#869 will be merged

[apohrebniak]

Any updates on this PR?

[alexkazik]

Sorry, I kinda forgot about it.
But, to continue I'd delete the defmt and hex commits (because there are already established solutions about it, I didn't know) and then it's just reporting the unknown command and a typo - ok?