LivingLikeKrillin opened a new pull request, #2651:
URL: https://github.com/apache/plc4x/pull/2651
### 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.mapWriteTag`** — `endCode 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.
### 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` (3 — OK
/ device-error →
`REMOTE_ERROR` / timeout → `REMOTE_ERROR`), `SlmpDriverTest` (1 — the
driver reports
`canRead`/`canWrite`), plus write cases in `SlmpErrorMappingTest` (2) and
`SlmpRequestBuildTest` (1).
- 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: 59 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
