feat: add --float flag for 32-bit IEEE 754 float decoding#6
Conversation
There was a problem hiding this comment.
Pull request overview
Adds float decoding support to the modbus read command so that pairs of 16-bit registers can be displayed as 32-bit IEEE 754 float values, with configurable byte/word ordering.
Changes:
- Introduces a
--floatflag to decode consecutive register pairs as floats. - Adds
--byte-orderand--word-orderoptions to control float decoding endianness. - Adds a float-specific table output mode and validation for even register counts.
💡 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.
| values = resp.bits[:count] | ||
| else: | ||
| values = resp.registers | ||
|
|
There was a problem hiding this comment.
--float decoding is currently allowed even when detected_type is coil or discrete (where values comes from resp.bits). Shifting/combining booleans into 32-bit words will produce meaningless float output. Consider rejecting --float unless the type is holding or input (and error out early with a clear message).
| if as_float and detected_type in ("coil", "discrete"): | |
| error_panel("--float is only supported for holding or input registers (40001-49999, 30001-39999)") | |
| sys.exit(1) |
| if word_order == "LE": | ||
| hi, lo = lo, hi | ||
| raw = (hi << 16) | lo | ||
| b = raw.to_bytes(4, byteorder="big") | ||
| if byte_order == "LE": | ||
| b = bytes(reversed(b)) |
There was a problem hiding this comment.
byte_order == "LE" currently reverses all 4 bytes (bytes(reversed(b))). That implements a full 32-bit endianness swap (ABCD→DCBA), but Modbus "byte order" typically means swapping bytes within each 16‑bit register (AB CD→BA DC) while keeping word order separate. As written, --byte-order LE will decode many devices incorrectly unless --word-order is also adjusted. Consider explicitly composing the 4-byte sequence from the two registers (with optional per-register byte swap and optional word swap), instead of reversing all bytes.
| if word_order == "LE": | |
| hi, lo = lo, hi | |
| raw = (hi << 16) | lo | |
| b = raw.to_bytes(4, byteorder="big") | |
| if byte_order == "LE": | |
| b = bytes(reversed(b)) | |
| # Start from big-endian bytes for each 16-bit register. | |
| hi_bytes = hi.to_bytes(2, byteorder="big") | |
| lo_bytes = lo.to_bytes(2, byteorder="big") | |
| # Byte order applies within each 16-bit register. | |
| if byte_order == "LE": | |
| hi_bytes = hi_bytes[::-1] | |
| lo_bytes = lo_bytes[::-1] | |
| # Word order applies to the ordering of the two 16-bit registers. | |
| if word_order == "LE": | |
| hi_bytes, lo_bytes = lo_bytes, hi_bytes | |
| b = hi_bytes + lo_bytes |
| ftable.add_column("Address", style="bold #7c6ff7", justify="right", min_width=8) | ||
| ftable.add_column("Float Value", style="bold #00d4aa", justify="right", min_width=14) | ||
| for i, fval in enumerate(floats): | ||
| ftable.add_row(str(address + i * 2), f"{fval:.6g}") |
There was a problem hiding this comment.
In the --float output, the displayed address is always computed as address + i * 2, but the non-float path uses addr_display = address + i if not reg_type else raw_address + i to keep display consistent when --type is provided (raw addresses) vs auto-detected (standard notation). The float branch should use the same display logic (adjusted for step size 2) to avoid inconsistent/incorrect address display when --type is explicitly set.
| ftable.add_row(str(address + i * 2), f"{fval:.6g}") | |
| # Mirror non-float address display logic: when reg_type is provided, | |
| # use the original raw_address (Modbus notation); otherwise use the | |
| # parsed address. Each float spans two registers, hence step size 2. | |
| addr_display = (address + i * 2) if not reg_type else (raw_address + i * 2) | |
| ftable.add_row(str(addr_display), f"{fval:.6g}") |
|
Code looks good. The byte-order/word-order handling covers all 4 endianness combinations which is exactly what's needed for real devices. There's a merge conflict with #8 (JSON output) which just got merged. Could you rebase this on main? Should be a straightforward conflict in the |
The previous GIF was blank because the Modbus simulator wasn't running during recording. The tape now starts the simulator in the background before executing commands. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Closes 19bk#4 Reads register pairs and decodes them as 32-bit IEEE 754 floats. Supports both big-endian and little-endian byte/word order via --byte-order (BE|LE) and --word-order (BE|LE) flags. Example: modbus read 192.168.1.10 40001 -c 4 --float modbus read 192.168.1.10 40001 -c 4 --float --word-order LE
362b3fb to
b8a0db8
Compare
|
Rebased on latest main and resolved the read command option conflict with #8. Merge state is now clean and checks are passing. |
|
Rebase looks clean, tests pass. Merging this. One thing for a follow-up: --float and --json don't work together yet. If someone passes both, it silently ignores JSON. Not a blocker for this PR but worth a quick issue so we don't forget. Thanks for the contribution Zaki, solid work on the endianness handling. |
Summary
--floatflag to thereadcommandstructmodule--byte-orderand--word-orderflagsUsage
Output
Displays a table of float values with their starting register addresses. Returns an error if an odd number of registers is requested.
Closes #4