knurling-rs / probe-run

Run embedded programs just like native ones
Apache License 2.0
645 stars 75 forks source link

Allow single line log output #407

Closed ctron closed 11 months ago

ctron commented 1 year ago

Describe the bug

Allow the user to choose a single-line output format.

To Reproduce

Use probe-run in combination with defmt.

Expected and observed behavior

When I run something, I get the following output:

0.000274 INFO  Performing reset ...
└─ vat_card_reader::____embassy_main_task::{async_fn#0} @ src/main.rs:82
0.701110 INFO  Performing reset ... done!
└─ vat_card_reader::____embassy_main_task::{async_fn#0} @ src/main.rs:89
0.701477 INFO  Sending wakup
└─ vat_card_reader::____embassy_main_task::{async_fn#0} @ src/main.rs:93
1.702239 INFO  Run
└─ vat_card_reader::____embassy_main_task::{async_fn#0} @ src/main.rs:99
1.810455 INFO  Firmware: FirmwareVersion { ic: 50, version: 1, revision: 6, supports_iso18092: true, supports_iso14443_a: true, supports_iso14443_b: true }
└─ vat_card_reader::____embassy_main_task::{async_fn#0} @ src/main.rs:107

I find this hard to read and would prefer a format with a single line output, like this:

0.000274 INFO  Performing reset ...
0.701110 INFO  Performing reset ... done!
0.701477 INFO  Sending wakup
1.702239 INFO  Run
1.810455 INFO  Firmware: FirmwareVersion { ic: 50, version: 1, revision: 6, supports_iso18092: true, supports_iso14443_a: true, supports_iso14443_b: true }

Or, some mix:

0.000274 [vat_card_reader::____embassy_main_task::{async_fn#0} @ src/main.rs:82] INFO  Performing reset ...
0.701110 [vat_card_reader::____embassy_main_task::{async_fn#0} @ src/main.rs:89] INFO  Performing reset ... done!
0.701477 [vat_card_reader::____embassy_main_task::{async_fn#0} @ src/main.rs:93] INFO  Sending wakup
1.702239 [vat_card_reader::____embassy_main_task::{async_fn#0} @ src/main.rs:99] INFO  Run
1.810455 [vat_card_reader::____embassy_main_task::{async_fn#0} @ src/main.rs:107] INFO  Firmware: FirmwareVersion { ic: 50, version: 1, revision: 6, supports_iso18092: true, supports_iso14443_a: true, supports_iso14443_b: true }
Urhengulas commented 1 year ago

Hi @ctron, thank you for your request.

You can configure probe-run to output JSON and process that with any tool you like. Read more about it here: https://defmt.ferrous-systems.com/json-output.html.

In your case you could write a small script which outputs the logs in your preffered way.

Let me know what you think.

ctron commented 1 year ago

Yea, I got that running, somewhat like this:

env DEFMT_LOG=debug cargo run -- --json | jq -r '"\(.target_timestamp) \(.data)"'

however, it would be nice to just do:

cargo run -- --format single

And maybe even configure that in the .cargo/configtoml:

[target.'cfg(all(target_arch = "arm", target_os = "none"))']
runner = "probe-run --chip STM32L432KCUx --format single"

And have the colored output too.

Urhengulas commented 1 year ago

I get what you mean.

My only concern is that the number of output formats people would like to have is potentially unbound. Therefore it would be cool if anyone could just plug in their favorite format and they do not all have to live in our repo.

ctron commented 1 year ago

Indeed. On the other hand, supporting a "standard" log format like a wide use library (like env_logger) makes more sense IMHO than to force a (new) custom format on everyone. And ideally, it should be possible to stay with the tool/toolchain being used. Meaning: I can configure probe-run to be used by cargo run, but in this case I can't seem to use a different log format (using jq).

So I think there are two things which can be improved here:

  1. Add a --format single argument (using an existing "default" log output ,like env_logger)
  2. Allow a --format custom argument (with an additional --log-template argument), allowing the user to provide a log pattern

I guess that 1. is a low hanging fruit which would already cover most of the cases.

jonathanpallant commented 1 year ago

To me, probe-run is a highly opinionated tool that is aimed at people starting out in Embedded Rust and who want a batteries-included tool.

I think the flexibility we have is probably sufficient for that audience and I don't think I'm convinced of the need to maintain two different textual log output formats.

It is worth noting as well that you can create a shell-script wrapper (or another Rust program) which calls probe-run and parses the JSON output, then use that as your cargo run runner.

Urhengulas commented 1 year ago

I agree with @jonathanpallant.

Also note that there is the defmt-json-schema crate to make the processing of the JSON output easier: https://docs.rs/defmt-json-schema/0.1.0/defmt_json_schema/v1/struct.JsonFrame.html