Skip to content

Add SLMP (MELSEC Communication 3E) protocol module (read-only wire layer)#2597

Open
LivingLikeKrillin wants to merge 2 commits into
apache:developfrom
LivingLikeKrillin:feature/slmp-protocol
Open

Add SLMP (MELSEC Communication 3E) protocol module (read-only wire layer)#2597
LivingLikeKrillin wants to merge 2 commits into
apache:developfrom
LivingLikeKrillin:feature/slmp-protocol

Conversation

@LivingLikeKrillin

Copy link
Copy Markdown
Contributor

First increment of the SLMP driver proposed in #2585 — the codegen-layer protocol module only (protocols/slmp), opened as a draft per the Slack suggestion so the work is visible early.

What''s in it

  • 3E binary frame (request/response discriminated on the subheader byte), little-endian, read-only
  • Commands: Batch Read (0x0401), Random Read (0x0403), Batch Read Multiple Blocks (0x0406), word units
  • Root type declares the new-SPI encoding defaults
  • ParserSerializer vectors derived from the worked examples in the public Mitsubishi reference manual SH-080008 (sections 8.1 / 8.3 / 8.4)

Validation so far

  • mvn -pl protocols/slmp test green on current develop (post-SPI3 merge): the mspec parses and all type references resolve
  • 0x0401 / 0x0403 cross-checked against pymcprotocol (independent MIT-licensed client): its request bytes are byte-identical to the vectors except the monitoring-timer field, and it decodes the responses to the manual''s exact values
  • 0x0406: pymcprotocol has no multi-block API, so the request vector rests on the manual''s worked example plus an independent re-encode check — flagging this honestly
  • The ParserSerializer suite is not yet executed by any build module — like the other protocols, it runs from the (upcoming) driver module; until then the in-module test validates that the mspec parses and all type references resolve
  • Byte round-trip through generated code will come with the driver module

Next (not in this PR)


Developed with AI assistance; every wire-layer byte was verified against SH-080008, with the batch and random reads additionally cross-checked against pymcprotocol as noted above.

…wire layer

Adds the codegen-layer protocol module for SLMP / MELSEC Communication
protocol (Mitsubishi PLCs), as proposed in the SLMP driver proposal issue:

- slmp.mspec modelling the 3E binary frame (request/response), with
  Batch Read (0x0401), Random Read (0x0403) and Batch Read Multiple
  Blocks (0x0406) in word units, read-only
- ParserSerializer test suite with vectors derived from the worked
  examples in the public Mitsubishi reference manual SH-080008
  (sections 8.1/8.3/8.4)
- Root type declares the new-SPI encoding defaults (little-endian)

The driver module (plc4j/drivers/slmp) will follow on top of the new
SPI as a separate step.

Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
@LivingLikeKrillin LivingLikeKrillin marked this pull request as ready for review June 18, 2026 14:21
@ottlukas ottlukas added the awaiting-feedback This label is applied when an issue has been opened and we need more information from the issuer. label Jun 22, 2026
@ottlukas ottlukas requested a review from chrisdutz June 22, 2026 14:22
@chrisdutz

Copy link
Copy Markdown
Contributor

I've checked out your PR and created a dummy slmp driver module where the code is generated from your mspec and added a ParserSerializerTest to use the generated code with your xml ... I had to set it to fix one general issue: in your initial XML you had "deviceCode" modelled as:

<deviceCode>D</deviceCode>
However it's an enum with a numerical value, so correct would be:

                <deviceCode>
                  <SlmpDeviceCode dataType="uint" bitLength="8" stringRepresentation="D">168</SlmpDeviceCode>
                </deviceCode>

This was the only issue I found in getting the driver's types, parsers and serializers generated and the test-suite running.

You want me to send you my changes or do you want to learn it yourself?

deviceCode is a numeric enum (SlmpDeviceCode), but the ParserSerializer
test vectors used the shorthand <deviceCode>D</deviceCode> form. That
does not round-trip through the generated parser/serializer, which
expects the full enum element with its numeric value:

    <deviceCode>
      <SlmpDeviceCode dataType="uint" bitLength="8" stringRepresentation="D">168</SlmpDeviceCode>
    </deviceCode>

Convert all deviceCode occurrences in ParserSerializerTestsuite.xml to
the canonical form so the vectors round-trip once the generated driver
types exercise them.
@LivingLikeKrillin

Copy link
Copy Markdown
Contributor Author

Thanks for taking the time to check it out and for wiring up the generated-driver test run — much appreciated.

Good catch on the deviceCode representation. I actually ran into the exact same thing while building the driver on my (separate, stacked) branch, and had already converted the test vectors to the canonical enum form (<SlmpDeviceCode … stringRepresentation="D">168</SlmpDeviceCode>). I've just pushed that fix to this PR, so no need to send yours — but thank you for the offer.

On the bigger picture: this PR is intentionally the protocol-only module (mspec + generated wire layer), which is why the test suite isn't exercised here yet. The Java driver that actually runs these vectors is the next, separate PR, stacked on top of this one — I'll open it once this merges so its diff stays clean. Happy to adjust if you'd prefer different sequencing.

@hutcheb hutcheb 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.

Looks good.

@chrisdutz

Copy link
Copy Markdown
Contributor

Ok ... well ... as the PR says: Protocol Module I think it's ok to merge ... could you however do me a favor and sign an ICLA with Apache? This is something all contributors with more than minor contributions have to sign ... especially if you want to become a long time contributor or even committer here.

https://www.apache.org/licenses/icla.pdf

Then we've got everything in the right form and I'm happy to merge this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-feedback This label is applied when an issue has been opened and we need more information from the issuer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants