Skip to content

feat: add --json flag to read, dump, and scan commands#5

Closed
zakiscoding wants to merge 1 commit into
19bk:mainfrom
zakiscoding:feat/json-output-flag
Closed

feat: add --json flag to read, dump, and scan commands#5
zakiscoding wants to merge 1 commit into
19bk:mainfrom
zakiscoding:feat/json-output-flag

Conversation

@zakiscoding
Copy link
Copy Markdown
Contributor

Summary

  • Adds a --json flag to the read, dump, and scan commands
  • When --json is passed, outputs structured JSON instead of Rich tables
  • Enables piping into jq or other tools

Usage

modbus read 192.168.1.10 40001 -c 5 --json | jq '.registers[].value'
modbus dump 192.168.1.10 40001 40100 --json > dump.json
modbus scan 192.168.1.10 --json | jq '.devices'

JSON Schema (read)

{
  "host": "192.168.1.10",
  "type": "holding",
  "slave": 1,
  "registers": [
    {"address": 40001, "raw": 237, "value": "237"}
  ]
}

Closes #1

Closes #1

Outputs results as JSON instead of Rich tables when --json is passed,
enabling piping into jq or other tools:
  modbus read 192.168.1.10 40001 -c 5 --json | jq '.registers[].value'
Copilot AI review requested due to automatic review settings March 12, 2026 16:39
Copy link
Copy Markdown

Copilot AI left a 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 adds a --json flag to the read, dump, and scan commands of modbus-cli, enabling structured JSON output for piping into tools like jq. It also adds --float, --byte-order, and --word-order options to the read command for decoding register pairs as 32-bit IEEE 754 floats (not mentioned in the PR description).

Changes:

  • Added --json flag to read, dump, and scan commands that outputs structured JSON instead of Rich tables
  • Added --float decoding support with --byte-order and --word-order options to the read command
  • Added _decode_floats helper function for IEEE 754 float decoding from register pairs

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread modbus_cli/cli.py
Comment on lines +394 to +396
if as_json:
print(__import__("json").dumps({"host": host, "range": scan_range, "register": register, "devices": [{"slave_id": sid, "register_value": val} for sid, val in found]}, indent=2))
return
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

When --json is used, Rich progress bars, spinner output, and "Found slave" messages are still printed to the console (lines 355-392) before the JSON is emitted. This pollutes stdout/stderr and makes the output unsuitable for piping into jq or other tools, which is the stated purpose of the --json flag. The JSON check should be moved earlier, or Rich output should be suppressed (e.g., by redirecting the console to stderr or skipping the progress display) when as_json is true.

Copilot uses AI. Check for mistakes.
Comment thread modbus_cli/cli.py
Comment on lines +569 to +572
if as_json:
registers = [{"address": start_address + i, "raw": val, "value": _format_value(val, fmt)} for i, val in enumerate(all_values)]
print(__import__("json").dumps({"host": host, "type": detected_start, "slave": slave, "registers": registers}, indent=2))
return
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

Similar to the scan command issue: when --json is used, Rich output (connection header at line 539, progress bars at lines 544-557) is still printed to the console before the JSON is emitted. This contaminates the JSON output and breaks piping. The Rich output should be suppressed or redirected to stderr when as_json is true.

Copilot uses AI. Check for mistakes.
Comment thread modbus_cli/cli.py
Comment on lines +199 to +200
from rich.table import Table
from rich.panel import Panel
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

Table and Panel are already imported at the top of the file (lines 9-10). These local re-imports are unnecessary.

Suggested change
from rich.table import Table
from rich.panel import Panel

Copilot uses AI. Check for mistakes.
Comment thread modbus_cli/cli.py
Comment on lines +196 to +197
pairs = [{"address": address + i * 2, "value": floats[i]} for i in range(len(floats))]
print(__import__("json").dumps({"host": host, "type": detected_type, "slave": slave, "float_values": pairs}, indent=2))
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

IEEE 754 special values (NaN, Infinity) can appear in Modbus register data. Python's json.dumps will serialize these as bare NaN/Infinity tokens, which are not valid JSON and will cause downstream JSON parsers (like jq) to fail. Consider using json.dumps(..., allow_nan=False) with error handling, or converting these values to null or string representations before serialization.

Copilot uses AI. Check for mistakes.
Comment thread modbus_cli/cli.py
floats = _decode_floats(list(values), byte_order=byte_order, word_order=word_order)
if as_json:
pairs = [{"address": address + i * 2, "value": floats[i]} for i in range(len(floats))]
print(__import__("json").dumps({"host": host, "type": detected_type, "slave": slave, "float_values": pairs}, indent=2))
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

json is already imported at the top of the file (line 3). Using __import__("json") here is unnecessary and inconsistent. Just use json.dumps(...) directly.

Copilot uses AI. Check for mistakes.
Comment thread modbus_cli/cli.py
addr_display = address + i if not reg_type else raw_address + i
int_val = int(val)
registers.append({"address": addr_display, "raw": int_val, "value": _format_value(int_val, fmt) if not isinstance(val, bool) else str(int_val)})
print(__import__("json").dumps({"host": host, "type": detected_type, "slave": slave, "registers": registers}, indent=2))
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

Same issue: json is already imported at the top of the file. Use json.dumps(...) instead of __import__("json").dumps(...). This pattern is repeated in all three commands (read, scan, dump).

Copilot uses AI. Check for mistakes.
Comment thread modbus_cli/cli.py

if as_float:
if len(values) % 2 != 0:
from .theme import error_panel
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

error_panel is already imported at the top of the file (line 14). This local re-import is unnecessary.

Suggested change
from .theme import error_panel

Copilot uses AI. Check for mistakes.
Comment thread modbus_cli/cli.py
Comment on lines +152 to +154
@click.option("--float", "as_float", is_flag=True, default=False, help="Decode register pairs as 32-bit IEEE 754 floats.")
@click.option("--byte-order", default="BE", type=click.Choice(["BE", "LE"]), help="Byte order for float decoding (default: BE).")
@click.option("--word-order", default="BE", type=click.Choice(["BE", "LE"]), help="Word order for float decoding (default: BE).")
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

The --float, --byte-order, and --word-order options are not mentioned in the PR description or issue #1. These are unrelated features bundled into a PR that's supposed to only add --json support. Consider splitting these into a separate PR for cleaner review and history.

Copilot uses AI. Check for mistakes.
Comment thread modbus_cli/cli.py
Comment on lines +210 to +217
if as_json:
registers = []
for i, val in enumerate(values):
addr_display = address + i if not reg_type else raw_address + i
int_val = int(val)
registers.append({"address": addr_display, "raw": int_val, "value": _format_value(int_val, fmt) if not isinstance(val, bool) else str(int_val)})
print(__import__("json").dumps({"host": host, "type": detected_type, "slave": slave, "registers": registers}, indent=2))
return
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

When --json is used with the read command, Rich output is still written to stdout before the JSON (e.g., console.print() at line 171, connection_header() at line 176, console.status() at lines 173 and 178). This contaminates the JSON output when piping. Consider suppressing Rich output or redirecting it to stderr when as_json is true.

Copilot uses AI. Check for mistakes.
@19bk
Copy link
Copy Markdown
Owner

19bk commented Mar 12, 2026

Closing in favor of the focused PRs: #8 (JSON, merged) and #6 (float, pending rebase). Splitting features into separate PRs makes review easier. Appreciate the work!

@19bk 19bk closed this Mar 12, 2026
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.

Add --json output format

3 participants