esp-rs / espflash

Serial flasher utility for Espressif SoCs and modules based on esptool.py
Apache License 2.0
457 stars 111 forks source link

Add log-format argument #493

Closed SergioGasquez closed 8 months ago

SergioGasquez commented 9 months ago

~Add -d/--defmt flag for monitor and flash subcommands.~ Add -f/--log-format argument for monitor and flash subcommands.

cc @bugadani since you added this feature

~Draft until properly tested.~ Did some testing and seems to be working fine!

We still need to figure out if we want opt-in or opt-out of defmt

bugadani commented 9 months ago

Since the framing should mostly be compatible with the classic string streams, I'd probably suggest this to be opt out instead of opt in. It's probably much more annoying to enable defmt constantly for those who want it, than to disable for the few who have some custom binary in their logs. Transmission errors might shade this a bit for those who use noisy UART still, so I'm not sure.

If you end up going down the opt-in road, maybe it wouldn't be the worst idea to future proof it a bit with --log-format=defmt, so that if new formats (or new defmt versions!) appear, this doesn't have to be broken.

SergioGasquez commented 9 months ago

To be honest, I don't have a preference if it should be opt-in or opt-out, any suggestion @esp-rs/espressif?

bugadani commented 9 months ago

As an idea, maybe we could enable printing location info with --log-format=defmt-verbose or similar option, so I think I shall update my opinion and vote for that format :)