Skip to content

--json docs + Readme #669

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

Merged
merged 4 commits into from
Mar 30, 2022
Merged

--json docs + Readme #669

merged 4 commits into from
Mar 30, 2022

Conversation

Dajamante
Copy link
Contributor

  • completed documentation (v0.3.3+ is necessary to use the --json flag)
  • example of json output with formatter
  • small link broken in the readme

Copy link
Member

@japaric japaric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks. I left some suggestions.

@@ -5,10 +5,13 @@ The following printers are currently available:

- [`probe-run`], parses data sent over RTT (ARM Cortex-M only).
> 💡 If you are using the git version of defmt, make sure you also install the tool from git and not crates.io.

Since v0.3.3, `probe-run` has now a [`--json`] flag to format the output.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should say that the main goal of --json is to produce machine readable output. one of the use cases for that is changing the human-readable format

@@ -72,6 +75,47 @@ Afterwards `levels.json` looks like this:

It indicates the version of the json format you are using. `probe-run` will always output it as a header at the beginning of each stream of logs. We anticipate that the format will slightly change while `probe-run` and `defmt` evolve. Using this version you always know which revision is in use and can act upon that.

> 🤔: What if I want formatted output?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this questions overlaps with the one that says "what can I do with the JSON output?". I would suggest simply mentioning that one use case is changing the human readable output and the piped command. jq without any flags is not a good example of human readable output, maybe change it to only show the data field?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can also remove the whole reference to jq, as it is not relevant information here.

Copy link
Member

@japaric japaric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not part of this PR but could you fix this line; it has a missing link:

To do that, we supply a few things in [defmt_json_schema]:

@@ -68,6 +71,19 @@ Afterwards `levels.json` looks like this:
{"data":"println","host_timestamp":1643113389707313290,"level":null,"location":{"file":"src/bin/levels.rs","line":15,"module_path":{"crate_name":"levels","modules":[],"function":"__cortex_m_rt_main"}},"target_timestamp":"4"}
```

One use case is changing the human readable output with a piped command.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest moving this addition to the very end and mention that jq is an existing tool that can be used to change the output format. But again, the example output should be be JSON but something more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean what is above, or what is under?

{
  "data": "I am a PRINTLN-statement",
  "host_timestamp": 1647942992494142200,
  "level": null,
  "location": {
    //...
    }
  },
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But again, the example output should be be JSON but something more readable.

should not be JSON.

by "example output" I meant the code block. Ideally, it should be something like

$ cargo r --example foo | jq "something here"
I am a PRINTLN-statement
the next log statement

Copy link
Contributor Author

@Dajamante Dajamante Mar 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest moving this addition to the very end and mention that jq is an existing tool that can be used to change the output format. But again, the example output should be be JSON but something more readable.

Dumb question, do you mean at the very end of the page or the very end of the paragraph? I fear it will be a bit out of context at the very end of the page.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant at the end of the page. After re-reading the page I think a good place may be after the first paragraph of the "Data transfer objects"

So, what can I do with the JSON output?
There really are no boundaries. (..)
(NEW) One option is to use a program like jq to extract the parts of interest. blah blah show console example
If you wish to deserialize (..)

Copy link
Contributor Author

@Dajamante Dajamante Mar 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe that sentence is enough ⬆️ . Everybody already saw a json output. The console example does not bring anything.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, the sentence can be enough but I guess it should say "pipe to a program like jq" instead of "use"

@Dajamante
Copy link
Contributor Author

To do that, we supply a few things in [defmt_json_schema]:

I don't think the link is very informative...
https://crates.io/crates/defmt-json-schema

I will remove the linking instead, since Johann wrote extensive information.

@japaric
Copy link
Member

japaric commented Mar 30, 2022

I don't think the link is very informative...

I think the link still helps because you can tell it's a crate and what version it's available.

@Dajamante Dajamante closed this Mar 30, 2022
@Dajamante Dajamante reopened this Mar 30, 2022
@japaric
Copy link
Member

japaric commented Mar 30, 2022

thanks!
bors r+

@bors
Copy link
Contributor

bors bot commented Mar 30, 2022

Build succeeded:

@bors bors bot merged commit d3300cf into main Mar 30, 2022
@bors bors bot deleted the readme branch March 30, 2022 14:18
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.

None yet

2 participants