Skip to content

feat(plc4j/slmp): add Batch Write (0x1401) support - #2651

Merged
sruehl merged 15 commits into
apache:developfrom
LivingLikeKrillin:feature/slmp-batch-write
Jul 21, 2026
Merged

feat(plc4j/slmp): add Batch Write (0x1401) support#2651
sruehl merged 15 commits into
apache:developfrom
LivingLikeKrillin:feature/slmp-batch-write

Conversation

@LivingLikeKrillin

@LivingLikeKrillin LivingLikeKrillin commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

What & why

The SLMP (MELSEC Communication Protocol) driver merged in #2640 is read-only. This adds write
support via the 3E-frame Batch Write command 0x1401 in word units (subcommand 0x0000),
mirroring the existing Batch Read (0x0401) path: same 3E framing, same word-device addressing
(D / W / R), the same typed-value layer, the same single-frame constraints, and the same per-tag
error isolation. The only genuinely new pieces are a request-data type that carries the payload words
and a value encode that is the mirror of the existing decode.

Changes

  • mspec (protocols/slmp/slmp.mspec) — add a SlmpWriteRequest case (0x1401) to the
    SlmpRequestData typeSwitch: the same device addressing as SlmpReadRequest
    (uint 24 headDeviceNumber + SlmpDeviceCode + uint 16 numberOfPoints) followed by
    byte[] writeData count 'numberOfPoints * 2'. The payload is carried as raw bytes; typed encoding
    stays in the driver layer, symmetric with how the read response leaves responseData raw. The
    generated readwrite model (SlmpWriteRequest, updated SlmpRequestData) is regenerated and
    committed. No response-type change is needed — Batch Write success is endCode 0x0000 with an
    empty payload, which the existing SlmpResponseFrame3E already handles.
  • SlmpDataType.encode(PlcValue, quantity) — the mirror of decode: little-endian, same buffer
    options, per-type WORD/UINT → writeUnsignedInt(16), INT → writeSignedShort(16),
    DINT → writeSignedInt(32), UDINT → writeUnsignedLong(32), REAL → writeFloat(32). Returns
    null on a type/arity mismatch (the caller maps that to INVALID_DATA), symmetric with decode
    returning null on a short buffer.
  • SlmpResponseMapper.mapWriteTagendCode 0x0000 → OK, otherwise log + REMOTE_ERROR.
  • SlmpConnection.onWrite / writeSingleTag — structurally mirror onRead / readSingleTag:
    per-tag sequential dispatch, executeThrottled + the single-slot request/response correlation,
    allOf(...).handle(...) partial-failure isolation (timeout → REMOTE_ERROR, other → INTERNAL_ERROR),
    building a 0x1401 frame. An un-encodable value short-circuits to INVALID_DATA without sending a frame.
  • SlmpDriver.canWrite() → true — declare the capability alongside the existing canRead().
  • Housekeeping (no behavior change): the shared SlmpTag single-frame ceiling is unchanged at 960
    words, but its javadoc and the PlcInvalidTagException message it throws are generalized from
    "Batch Read" to "Batch Read/Write" (the ceiling now guards both paths); the driver module's POM
    <description> and the mspec header comments drop the now-inaccurate "read-only" wording; and the
    SH-080008 section references are corrected against the manual (device access is chapter 8; §8.1 is
    the device data spec, §8.2 is "Batch Read and Write" — the comments previously mislabelled
    batch read as §8.1).

Data types & devices

Word devices D (decimal), W (hex), R (decimal) and data types WORD, INT, UINT, DINT,
UDINT, REAL
— the same set the reader supports. Multi-element writes (quantity ≥ 1) are bounded by
the existing 960-word single-frame ceiling, enforced in the shared SlmpTag validation for both read
and write (the numeric limit is unchanged).

Out of scope

Random Write (0x1402), bit-unit writes (subcommand 0x0001), bit devices (M/X/Y/B), and multi-frame
splitting — consistent with the read driver's current scope.

Review follow-ups (commits after the initial review)

  • Echo builder error items — a tag the request builder had already rejected (bad address →
    INVALID_ADDRESS, un-coercible value → INVALID_DATA) reached onWrite as a null tag/value and
    NPE'd into a misleading INTERNAL_ERROR. Both onWrite and onRead now check
    getTagResponseCode(...) first and echo the builder's code (the pattern SimulatedConnection
    uses); nothing is sent on the wire for rejected tags. onRead had the same pre-existing flaw and
    is fixed symmetrically.
  • Unexpected-payload tripwire — per SH-080008 a Batch Write success carries no response data, so
    endCode 0x0000 with a non-empty payload (e.g. a late, mis-attributed read response under the
    documented no-correlation-id caveat) now maps to REMOTE_ERROR instead of a silent OK.
  • Timed-out-write semantics documented — a timed-out write may nevertheless have been applied by
    the device (3E carries no correlation id, so the driver cannot tell); noted on sendRequest/onWrite
    so callers don't blindly retry side effects.
  • Quieter logging for un-coercible values (expected caller error → DEBUG, no stack trace), a
    reflection-free capability test, and message-casing/naming cleanups from the first review round.

Testing (hardware-free, mirrors the reader)

  • New unit tests: SlmpDataTypeEncodeTest (11 — per-type little-endian encoding, high-bit unsigned,
    two's-complement, DINT low-word-first, IEEE754, list encoding, the three null-return guard paths,
    and encode → decode round-trips), SlmpConnectionWritePathTest (4 — OK / device-error →
    REMOTE_ERROR / timeout → REMOTE_ERROR / builder-error echo with per-tag isolation),
    SlmpDriverTest (1 — the driver reports canRead/canWrite), plus write cases in
    SlmpErrorMappingTest (3) and SlmpRequestBuildTest (1), and a read-side builder-error echo test.
  • The connection-level encode()==null → INVALID_DATA short-circuit is a two-line branch over the
    same null the encode-level guard tests pin directly; the builder rejects un-coercible values
    before the driver sees them (now covered by the builder-error echo tests).
  • New end-to-end driver-testsuite case: a PlcWriteRequest for D350 = 0x1234 is driven through
    the real driver, the exact 0x1401 frame bytes are checked, an empty-payload success response is fed
    back, and the api response code is asserted OK.
  • Full plc4j-driver-slmp module suite green: 62 unit tests + the 2 driver-testsuite IT cases (read + write).

Notes for reviewers

  • Wire field layouts and the 0x1401 command come from the public Mitsubishi manual SH(NA)-080008
    (§8.2, "Batch Read and Write") — the same source and citation discipline as the merged reader; a
    fact/spec, not copyrightable expression. encode / onWrite / mapWriteTag are mechanical mirrors
    of the existing decode/read path. The request layout ([head device 3B][device code 1B][points 2B]
    [write data]) and "there is no response data for this command" were checked against §8.2 (p.92).
  • Beyond the committed tests, I validated the full write→read round-trip end-to-end over TCP against an
    independent SLMP 3E server built from the manual: writing WORD / INT / DINT / REAL / WORD[3] and
    reading them back all matched, with the expected little-endian, low-word-first multi-word ordering.
  • This touches only the SLMP driver + its protocol mspec — no change to AbstractBuffer or the shared
    buffers.

Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
… docblock for Batch Write

Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
…ch Read/Write

Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
SlmpDriver overrode canRead() but not canWrite(), so it inherited
DriverBase.canWrite() -> false, unlike every other writeable driver
(S7, Modbus, ADS, EIP). Now that Batch Write (command 0x1401) is
supported, override canWrite() to return true. Adds SlmpDriverTest
asserting the driver reports both canRead()==true and canWrite()==true,
mirroring the reflection idiom in S7DriverTest/AdsPlcDriverTest.

Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
…s supported

The mspec 'Scope of this initial version' comment still described the
protocol as read-only even though it lists Batch Write (command 0x1401).
Remove the contradictory 'read-only' from the scope sentence and soften
the 'read-only road-map' phrase in the device-code comment to 'initial
road-map'. Comment-only edits; generated sources are unchanged.

Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
…scription

Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
…nual

Verified the citations against SH(NA)-080008 (ed. AB): device access is
chapter 8, where section 8.1 is "Data to be Specified" (devices, device code
list, points), section 8.2 is "Batch Read and Write" (batch read 0x0401 AND
batch write 0x1401), 8.3 Random, 8.4 Multiple Blocks. The header's "Batch Read
data layout ... section 8.1" was inaccurate (batch read/write is 8.2, not 8.1);
merge it with Batch Write under 8.2 and point the command list at chapter 7.

Also fix the same 8.1 mislabel on the batch-read worked example in
ParserSerializerTestsuite.xml and SlmpRequestBuildTest, and quote the full
8.2 section title ("Batch Read and Write") in the SlmpWriteRequest comment.

Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 SLMP 3E-frame Batch Write (0x1401) support (word units, subcommand 0x0000) to the PLC4J SLMP driver, mirroring the existing Batch Read path while keeping the protocol layer payload as raw bytes and doing typed value encoding/decoding in the driver.

Changes:

  • Extend the SLMP protocol mspec + generated model to support SlmpWriteRequest (0x1401) with [headDevice][deviceCode][points][writeData].
  • Implement driver-side typed encode (SlmpDataType.encode) and the write execution path (SlmpConnection.onWrite / writeSingleTag) including per-tag error isolation and endCode mapping.
  • Add unit + integration tests (including driver-testsuite XML) to validate request bytes and response handling; update docs/comments/POM wording to remove “read-only”.

Reviewed changes

Copilot reviewed 14 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
protocols/slmp/src/test/resources/protocols/slmp/ParserSerializerTestsuite.xml Updates manual section reference in protocol testsuite comments.
protocols/slmp/src/main/resources/protocols/slmp/slmp.mspec Adds Batch Write request layout to the protocol model and updates header comments/scope.
plc4j/drivers/slmp/src/test/resources/slmp/slmp-driver-testsuite.xml Adds an end-to-end driver-testsuite testcase covering Batch Write bytes + OK mapping.
plc4j/drivers/slmp/src/test/java/org/apache/plc4x/java/slmp/SlmpRequestBuildTest.java Adds a unit test asserting the exact serialized 0x1401 request frame bytes.
plc4j/drivers/slmp/src/test/java/org/apache/plc4x/java/slmp/SlmpErrorMappingTest.java Adds unit tests for write endCode → response code mapping.
plc4j/drivers/slmp/src/test/java/org/apache/plc4x/java/slmp/SlmpDriverTest.java Adds a driver capability test for canRead/canWrite.
plc4j/drivers/slmp/src/test/java/org/apache/plc4x/java/slmp/SlmpDataTypeEncodeTest.java Adds unit tests for typed value encoding and encode↔decode round-trips.
plc4j/drivers/slmp/src/test/java/org/apache/plc4x/java/slmp/SlmpConnectionWritePathTest.java Adds write-path tests for OK/device-error/timeout mappings via scripted transport.
plc4j/drivers/slmp/src/main/java/org/apache/plc4x/java/slmp/tag/SlmpTag.java Generalizes single-frame word ceiling messaging/docs from read-only to read/write.
plc4j/drivers/slmp/src/main/java/org/apache/plc4x/java/slmp/SlmpResponseMapper.java Adds mapWriteTag for endCode-only Batch Write responses.
plc4j/drivers/slmp/src/main/java/org/apache/plc4x/java/slmp/SlmpDriver.java Declares canWrite() -> true.
plc4j/drivers/slmp/src/main/java/org/apache/plc4x/java/slmp/SlmpDataType.java Adds encode(...) to mirror existing typed decode logic.
plc4j/drivers/slmp/src/main/java/org/apache/plc4x/java/slmp/SlmpConnection.java Implements onWrite and request building/sending for 0x1401.
plc4j/drivers/slmp/src/main/generated/org/apache/plc4x/java/slmp/readwrite/SlmpWriteRequest.java Generated request type for Batch Write payload.
plc4j/drivers/slmp/src/main/generated/org/apache/plc4x/java/slmp/readwrite/SlmpRequestData.java Updates discriminated parsing to include the new 0x1401 case.
plc4j/drivers/slmp/pom.xml Updates module description to no longer claim “read-only”.

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

- encode(): an un-coercible PlcValue (PlcIncompatibleDatatypeException) is an
  expected caller error that maps to INVALID_DATA -- log it at DEBUG without a
  stack trace instead of WARN; genuine buffer/encoding failures keep the WARN.
- SlmpDriverTest: drop the reflection -- the test shares the driver's package,
  so the protected canRead()/canWrite() are directly accessible.
- SlmpTag: normalize the ceiling wording to "Batch Read/Write".

Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
… as INTERNAL_ERROR

The request builders store a DefaultPlcTagErrorItem (INVALID_ADDRESS for an
unparseable address, INVALID_DATA for an un-coercible value) instead of
throwing; for such a tag getTag()/getPlcValue() return null. onWrite cast the
null and NPEd in writeSingleTag, so the per-tag aggregation reported
INTERNAL_ERROR and masked the correct code. onRead had the same pre-existing
flaw for INVALID_ADDRESS and is fixed symmetrically.

Both loops now check request.getTagResponseCode(tagName) first (the pattern
SimulatedConnection.onWrite established) and echo a non-OK code straight into
the response without sending anything for that tag.

Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
… timed-out-write semantics

Per SH-080008 a Batch Write success response carries no data, so a zero
endCode with a non-empty payload is the signature of a mis-attributed
response (e.g. a late read answer arriving after its own request timed out;
3E has no correlation id). mapWriteTag now takes the response payload, warns
and maps that combination to REMOTE_ERROR instead of a false OK.

Also generalizes the sendRequest javadoc caveat from reads to all requests
and spells out that a timed-out write may still have been applied by the
device (so retrying on REMOTE_ERROR may double-apply), renames the driver
testsuite to 'SLMP 3E Batch Read/Write' now that it covers writes, and
updates the stale mspec scope comment: the typed value codec and tag layer
are owned by the driver layer by design, not 'not built yet' (comment-only,
no generated-source changes).

Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated no new comments.

@sruehl
sruehl merged commit 9f298a1 into apache:develop Jul 21, 2026
21 of 22 checks passed
@LivingLikeKrillin
LivingLikeKrillin deleted the feature/slmp-batch-write branch July 21, 2026 12:05
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.

3 participants