Fix FC17 response decode: identifier included the run indicator byte

[abdulsamie10]

ReportDeviceIdResponse.encode() and .decode() disagree about what the FC17
byte-count field covers. encode() counts identifier + run indicator:

length = len(self.identifier) + 1

decode() slices byte-count bytes as the identifier alone:

self.identifier = data[1 : self.byte_count + 1]

So decoding the frame the library itself encodes for identifier=b"Pymodbus",
status ON (\x09Pymodbus\xff) yields identifier == b"Pymodbus\xff" — the
run-indicator byte lands in the identifier. Every
client.report_device_id() against a pymodbus server returns a polluted
identifier, reproducible on dev with a stock ModbusTcpServer +
AsyncModbusTcpClient on loopback:

	identifier returned
dev	b'Pymodbus\xff'
this PR	b'Pymodbus'

The encode convention is the one the rest of the library depends on:
rtu_byte_count_pos = 2 means calculateRtuFrameSize computes
byte_count + 5, which only frames FC17 RTU responses correctly if the count
covers the run indicator. So the fix is decode-side only:

• identifier = data[1 : byte_count] (counted bytes minus the run indicator);
• status is read at data[byte_count] — the last counted byte — instead of
  data[-1], so a byte a device appends past the count is no longer taken for
  the run indicator;
• a byte-count guard raising ModbusIOException, the same shape as the FC03
  response guard ReadHoldingRegistersResponse.decode has carried since Refactor PDU, add strong typing to base classes. #2438
  (with the function_code= propagation from Propagate known request identity on ModbusIOException #2993), because the new
  data[byte_count] access must not turn a lying byte count into a bare
  IndexError. This is a response decode (client side; the FC17 request
  carries no data), so the don't-raise-in-server-request-decode concern from
  Validate FC23 write count against MAX_WRITE_COUNT, not MAX_READ_COUNT #3016 does not apply.

Provenance: 4de9e3c7 (2011-01-31) shipped a self-consistent pair —
encode() counted len(identifier) + 2, decode() sliced
data[1:length - 1]. b442af51 (2011-09-06, "Finishing the remaining modbus
protocol") changed the slice to data[1:length + 1] and left encode() alone;
the decoded identifier has been one byte too long ever since — through
0a77f6c1 (2017), which moved encode() to today's len(identifier) + 1, and
through the PDU refactor (#2160).

Existing coverage could not catch it: test_pdu_decode decodes without
asserting field values, and test_report_device_id fed decode a malformed
frame (b"\x03\x12\x00" — byte count 3, two bytes of payload) and then
asserted that re-encoding produces a different, 4-byte frame — the test was
pinning the asymmetry. That vector is now the well-formed
b"\x03\x12\x00\x00" and the re-encode assertion demonstrates a true
round-trip.

Two behavior changes to weigh before merging:

• User code that worked around this with identifier[:-1] gets the corrected,
  shorter identifier.
• A device whose byte count covers the identifier alone loses its last
  counted byte, which is now read as the run indicator. 0x11 report_slave_id got False and lost some bytes #1600's capture is
  exactly such a device — payload 07 01 02 42 00 11 01 03 23, count 7 with
  eight bytes after it. On dev it decodes
  identifier=b'\x01\x02B\x00\x11\x01\x03', status from the uncounted trailing
  0x23; with this PR it decodes identifier=b'\x01\x02B\x00\x11\x01', status
  from 0x03. Status is False either way, so for that device this change is
  a byte lost from the identifier, not a repair.

Spec §6.17 calls the FC17 payload "device specific" and settles neither
reading, so no decode is right for every vendor. What this PR buys is the one
invariant the library can own: pymodbus decodes its own framing convention —
the same one its RTU size math already assumes. The identifier stays a raw
value for the user to interpret, as the docstring says, and frames with
trailing bytes beyond the count still decode. If you would rather keep exotic
layouts byte-for-byte, keeping the raw payload on the PDU as an escape hatch
is an additive follow-up I am happy to write.

New tests:

• decode assertions on the exact frame the test already encodes
  (b"\tPymodbus\xff" → b"Pymodbus", status ON);
• decode(encode()) round-trip preserves identifier and status for both
  status values (fails on dev: identifier grows a byte per pass);
• byte count 0 and byte count past the end of the frame raise
  ModbusIOException (fails on dev: silently decodes garbage).

Validation, macOS / Python 3.13:

pytest test/pdu/test_other_messages.py   # 9 passed (3 failed before the fix)
pytest test                              # 1972 passed, 5 skipped
ruff format --check . / ruff check .     # clean
pylint --recursive=y examples pymodbus test  # 10.00/10
codespell / zuban check                  # clean

[Copilot]

🟢 Approval recommended

The decode-side fix is consistent with the existing encode/framing assumptions, includes appropriate error handling, and is backed by targeted regression and round-trip tests.

Pull request overview

This PR fixes an FC17 (ReportDeviceIdResponse) decoding off-by-one bug where the run-indicator byte was being included in the returned identifier, making encode()/decode() asymmetric and causing client-side decoded identifiers to be polluted.

Changes:

• Align ReportDeviceIdResponse.decode() with the existing encode() convention by slicing identifier to exclude the run-indicator byte and reading status from the last counted byte.
• Add a byte-count bounds guard in ReportDeviceIdResponse.decode() that raises ModbusIOException (with function_code attached) instead of risking an IndexError / silent garbage decode.
• Update and extend tests to assert correct field values, verify decode(encode()) round-trips, and cover invalid byte-count cases.

File summaries

File	Description
pymodbus/pdu/other_message.py	Fix FC17 response decode slicing/indexing and add byte-count validation raising ModbusIOException.
test/pdu/test_other_messages.py	Strengthen FC17 tests with field assertions, round-trip coverage, and invalid byte-count exception checks.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[janiversen]

LGTM thanks.