-
Notifications
You must be signed in to change notification settings - Fork 229
--json Outputs JSON objects for --nodes, --request-telemetry/position #577
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
base: master
Are you sure you want to change the base?
Conversation
Am I alone in thinking that any output produced in the presence of a |
It appears that these formats are, in fact, JSON (everything is produced via However, they do look fairly ad-hoc and I think we need a consistent approach, as we're discussing over in #569 -- the discussed plan there so far has largely been to do as little modification of protobufs as we can other than turning them to JSON, and to identify things by the (lowercased) name of the protobuf. This will probably require a bit of internal refactoring for some things (e.g. I don't believe right now we're holding onto a copy of the actual |
I totally agree.. it would be preferred to eliminate conversions back and forth into JSON like this. Refactoring the whole codebase to facilitate a clear separation of concern / interface was not my intention. I guess It's best to close this PR and keep the effort to myself while a proper solution is drafted in Discussion: machine-readable output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances meshtastic's command line output by adding a --json flag to produce machine-parsable JSON output for nodes, telemetry, and position requests.
- Updated test cases and mocks to verify the JSON output behavior for node listing.
- Modified methods in mesh_interface.py to accept a jsonResponse flag and print JSON output instead of formatted tables or plain text.
- Enhanced main.py to integrate the --json command line option and adjust its execution flow accordingly.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
meshtastic/tests/test_main.py | Added and updated test cases to verify JSON output for the --nodes command flag. |
meshtastic/mesh_interface.py | Modified showNodes, sendPosition, onResponsePosition, sendTelemetry, and onResponseTelemetry for JSON output; added printJsonPosition and printJsonTelemetry. |
meshtastic/main.py | Integrated the --json flag with conditional output behavior for various commands. |
Comments suppressed due to low confidence (1)
meshtastic/mesh_interface.py:457
- [nitpick] The JSON output formatting in printJsonPosition and printJsonTelemetry is very similar. Consider creating a shared helper function for JSON formatting to reduce duplication and improve maintainability.
def printJsonPosition(position: mesh_pb2.Position, destinationId: Union[int, str] = BROADCAST_ADDR):
if jsonResponse: | ||
print(json.dumps(rows, indent=2)) | ||
else: | ||
table = tabulate(rows, headers="keys", missingval="N/A", tablefmt="fancy_grid") | ||
print(table) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The showNodes function prints either a table or JSON but always returns the rows list. Consider separating the logic for generating node data from the presentation logic to maintain consistency and better testability.
Copilot uses AI. Check for mistakes.
In a attempt to make the output of meshtastic a bit more machine parsing friendly (eg by nushell) I propose the addition of '--json' to switch the output to something easily parsable when piped on the commandline.