[spike] feat(plc4net): revive the .NET port — net8.0, SPI3 runtime, pure-.NET mspec generator - #2656
TomNewChao wants to merge 61 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Revives the plc4net (.NET) port by modernizing it to .NET 8, restoring test execution, aligning key API/SPI surfaces with PLC4X SPI3 concepts, and adding a foundational driver runtime plus a TCP transport implementation.
Changes:
- Retarget projects to
net8.0and centralize shared build/package properties viaDirectory.Build.props. - Fix the PLC value model’s virtual dispatch and add a bit-level codec (
BitReader/BitWriter) + repairedReadBuffer/WriteBuffer. - Add SPI3-aligned runtime building blocks (connection-string parsing, driver/connection bases, message codec) and a TCP transport with CI workflow coverage.
Reviewed changes
Copilot reviewed 49 out of 49 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| plc4net/transports/tcp/TcpTransportInstance.cs | Adds async TCP transport instance with background read loop + ring buffer. |
| plc4net/transports/tcp/TcpTransportConfiguration.cs | Defines TCP transport configuration defaults and options. |
| plc4net/transports/tcp/TcpTransport.cs | Implements TCP transport factory + address parsing and option parsing. |
| plc4net/transports/tcp/plc4net-transport-tcp.csproj | Introduces TCP transport project. |
| plc4net/spi/spi/transports/TransportException.cs | Adds transport-specific exception type. |
| plc4net/spi/spi/transports/RingBuffer.cs | Adds fixed-capacity ring buffer used by transports/codecs. |
| plc4net/spi/spi/transports/ITransportInstance.cs | Introduces transport instance contracts (sync + async listener variant). |
| plc4net/spi/spi/transports/ITransport.cs | Introduces transport factory + transport manager registry. |
| plc4net/spi/spi/transports/BaseTransportInstance.cs | Adds base transport instance with config + driver-config handling and Dispose. |
| plc4net/spi/spi/model/values/PlcWSTRING.cs | Fixes string value dispatch/exposure. |
| plc4net/spi/spi/model/values/PlcWORD.cs | Fixes overridden bit-accessors for WORD. |
| plc4net/spi/spi/model/values/PlcWCHAR.cs | Fixes string dispatch/exposure for WCHAR. |
| plc4net/spi/spi/model/values/PlcValueAdapter.cs | Makes IPlcValue API virtual to enable correct overriding/dispatch. |
| plc4net/spi/spi/model/values/PlcSTRING.cs | Fixes string dispatch/exposure. |
| plc4net/spi/spi/model/values/PlcSimpleValueAdapter.cs | Fixes overriding for “simple value” classification. |
| plc4net/spi/spi/model/values/PlcSimpleNumericValueAdapter.cs | Fixes numeric conversions/range checks and interface dispatch. |
| plc4net/spi/spi/model/values/PlcLWORD.cs | Fixes overridden bit-accessors for LWORD. |
| plc4net/spi/spi/model/values/PlcDWORD.cs | Fixes overridden bit-accessors for DWORD. |
| plc4net/spi/spi/model/values/PlcCHAR.cs | Fixes string dispatch/exposure for CHAR. |
| plc4net/spi/spi/model/values/PlcBYTE.cs | Fixes overridden bit-accessors for BYTE. |
| plc4net/spi/spi/model/values/PlcBOOL.cs | Fixes BOOL accessor dispatch and adds conversions. |
| plc4net/spi/spi/generation/WriteBuffer.cs | Reworks write buffer to use in-house bit writer + fixes float/string/array writing. |
| plc4net/spi/spi/generation/ReadBuffer.cs | Reworks read buffer to use in-house bit reader + fixes numeric/string/array reading. |
| plc4net/spi/spi/generation/ParseException.cs | Makes ParseException a real Exception type. |
| plc4net/spi/spi/generation/BitWriter.cs | Adds MSB-first bit writer. |
| plc4net/spi/spi/generation/BitReader.cs | Adds MSB-first bit reader. |
| plc4net/spi/spi/drivers/MessageCodecBase.cs | Adds SPI3-like message codec base and IMessage contract. |
| plc4net/spi/spi/drivers/DriverBase.cs | Adds SPI3-like driver base (transport resolution + connection creation). |
| plc4net/spi/spi/drivers/ConnectionBase.cs | Adds SPI3-like connection base wrapping a transport instance. |
| plc4net/spi/plc4net-spi.csproj | Removes net45-only dependency and aligns packaging with shared props. |
| plc4net/spi-test/test/transports/TcpTransportAddressTests.cs | Adds tests for TCP transport address parsing. |
| plc4net/spi-test/test/transports/RingBufferTests.cs | Adds ring buffer unit tests. |
| plc4net/spi-test/test/model/values/PlcValueTests.cs | Adds interface-dispatch-focused value model tests. |
| plc4net/spi-test/test/generation/BufferTests.cs | Adds codec round-trip tests for bit reader/writer and buffers. |
| plc4net/spi-test/test/drivers/DriverBaseTests.cs | Adds driver-base/transport-selection tests. |
| plc4net/spi-test/test/drivers/ConnectionStringTests.cs | Adds tests for SPI3-aligned connection string parsing + secret redaction. |
| plc4net/spi-test/plc4net-spi-test.csproj | Adds dedicated SPI test project with proper test SDK refs. |
| plc4net/plc4net.sln | Updates solution to include new test + transport projects and platforms. |
| plc4net/drivers/knxnetip/plc4net-driver-knxproj.csproj | Updates KNX driver project dependencies (e.g., NLog). |
| plc4net/drivers/knxnetip-test/test/knxnetip/readwrite/model/KnxDatapointTests.cs | Fixes KNX test vector and asserts float parsing. |
| plc4net/drivers/knxnetip-test/plc4net-driver-knxproj-test.csproj | Ensures test suite actually runs (adds Microsoft.NET.Test.Sdk, marks non-packable). |
| plc4net/Directory.Build.props | Centralizes target framework + shared packaging/build properties. |
| plc4net/api/PlcDriverManager.cs | Refactors driver manager to SPI3-style registry and sync connection creation. |
| plc4net/api/plc4net-api.csproj | Aligns API project with centralized build props. |
| plc4net/api/api/model/IPlcTag.cs | Renames Field→Tag concept for SPI3 alignment. |
| plc4net/api/api/IPlcDriver.cs | Updates driver contract to sync SPI3-like Connect() methods. |
| plc4net/api/api/IPlcConnection.cs | Updates connection contract to sync Close() + tag parsing. |
| plc4net/api/api/ConnectionString.cs | Adds SPI3-aligned connection-string parser + secret redaction. |
| .github/workflows/dotnet-platform.yml | Adds cross-platform CI job for building and running .NET tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Wow ... quite a bit happened here during my few days off :-) So one thing I wanted to point out: Even if in Java several external dependencies existed for various things (such as BitBuffers etc), I still rewrote everything from scratch in SPI3. One the one side, this way I could build a perfect fit for PLC4X, but also I thought this would be a perfect template for porting to other languages as there is no need to fins a "suitable but mostly not 100% replacement". Another thing I noticed: We currently rely on the plc4x build-tools maven plugin to generate code. In my commercial offering I decided to give something else a try: I built an mspec parser and code-generator that fits the target language (in that case Rust) ... possibly it might be worth investigating the options to use a pure dotnet code generation tool ... the antlr4 grammar for mspec should help a lot with that. In the end a pure dotnet toolchain would eliminate the dependency on Java for dotnet developers while still being able to call the build from the overall maven reactor. I think the API diverged quite a bit since it was created several years ago, but i think I saw you already picked up some of these changes. If you need any help ...don't hesitate pinging me. I can also invite you in the plc4x slack channel, if you want more instant feedback (during sensible EU times ;-) ) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 55 out of 55 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
plc4net/transports/tcp/TcpTransportInstance.cs:1
- On orderly remote shutdown (
bytesRead == 0), the socket is not disposed here. Because_openis set to 0 before any call toClose(), subsequentClose()calls can become no-ops (due to the CAS guard), leaving the socket/resources to finalization. Consider triggering the normal close/dispose path here (e.g., perform the same CAS-based shutdown/dispose sequence used byClose()), or refactorClose()to always dispose the socket even if_openis already 0.
plc4net/transports/tcp/TcpTransportInstance.cs:1 - Prefer
Array.Empty<byte>()overnew byte[0]to avoid an unnecessary allocation and follow common .NET conventions for empty arrays.
Thanks for the pointers — the code generation one lands on the open question in the description, and with an option I hadn't considered. The bit buffers were the same call in miniature: I dropped Ayx.BitIO and wrote the reader/writer rather than hunting for a closer package. That you rewrote that layer in SPI3 for the same reason is useful — I'll take SPI3 as the reference to follow rather than a source to copy line by line, and say so where .NET pushes a different shape. On a pure .NET toolchain: agreed, and for the reason you give. Removing the Java dependency for .NET developers is worth more than finishing the freemarker templates. I had a look at the antlr4 grammar and the parser side does look straightforward — the work sits above it, in the type model. Slack sounds like the right place for the rest — yes please, and thanks for the offer. I'm on UTC+8, so your working day runs from my afternoon into my evening; that lands inside sensible hours on both ends. |
|
As it's challenging to get github-user-to-email-addresses ... please send me the address I should send the invite to cdutz@apache.org |
b6bb5ba to
fa0c898
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 56 out of 56 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
plc4net/transports/tcp/TcpTransportInstance.cs:1
Socket.Send(...)can legally return 0 (e.g., when the connection has been closed), which would make this loop spin forever becauseoffsetnever increases. Consider capturing the return value, and if it is 0, treat it as a connection failure (throwTransportException/ close the connection) to avoid an infinite loop.
plc4net/transports/tcp/TcpTransportInstance.cs:1- When the ring buffer is full, the read loop polls with a 1ms delay. Under sustained backpressure this can cause unnecessary wakeups/CPU usage. Consider replacing this polling with a waitable signal (e.g., a
SemaphoreSlim/AsyncAutoResetEventthat the consumer signals after draining), or at least use a larger/exponential backoff delay to reduce churn.
e285d48 to
cf0302b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
.github/workflows/dotnet-platform.yml:28
- The path filter
plc4net**is overly broad (it also matches paths that merely start withplc4net, e.g.plc4netfoo/...). Usingplc4net/**is the typical and more precise way to scope to the directory tree.
paths:
- code-generation/**
- protocols/**
- plc4net**
pull_request:
.github/workflows/dotnet-platform.yml:65
actions/setup-javais configured withdistribution: 'adopt', but AdoptOpenJDK has been superseded by Eclipse Temurin and may stop being supported/updated. Switching totemurinkeeps the workflow on a maintained JDK distribution.
distribution: 'adopt'
plc4net/spi/spi/transports/RingBuffer.cs:140
- The comment says a single subtraction replaces a modulo, but the implementation uses a modulo. Either update the comment or change the implementation so the documentation matches the behavior.
plc4net/spi/spi/generation/ReadBuffer.cs:61 - HasMore currently returns true for negative bitLength values, which is nonsensical and can mask caller bugs. Consider rejecting negative sizes explicitly.
plc4net/transports/tcp/TcpTransport.cs:56 - receive-buffer-size can be set to 0 (or negative) via the connection string, which then crashes TcpTransportInstance when constructing the RingBuffer (capacity must be positive). Consider treating non-positive values as invalid and falling back to the default here.
|
@chrisdutz should that be part of 1.0.0? |
|
Well we shouldn't postpone the release too long. If something usable is done soon, sure. Otherwise the next release could be done any time. Is actually a quite streamlined process (as long as it's part of the monorepo. |
|
Also ... as this is considered a significant contribution .... before we can merge this you would need to file an ICLA with apache: https://www.apache.org/licenses/icla.pdf ... if you are doing this work as part of your day-job you should also consider your company signing a CCLA https://www.apache.org/licenses/cla-corporate.pdf |
|
Now that I had the time to catch up with everything else after my holiday yesterday, now I had the time to thorougly read your initial message. So the PLC4Net doesn't have to be identical to the Java version. It was allways our goal to make the libratries feel natural in their normal ecosystem. With Java we're using CompletableFutures, with Go we're using channels ... whatever is the best way to do things in the target language. What we do try to keep, is the general usage pattern: So if connections are synchronous in Java, it would sort of be bad if it wasn't in other languages. Also if the naming of things was kept ... like the usage of "Tag" and "Query" and that you can add a Tag or a TagString, .... Admittedly I don't quite understand what you mean with your first divergence: "ConnectionString is a public type in the API, not the SPI. It has no SPI dependency, and PlcDriverManager — which lives in the API — needs it to route the protocol code." Could you please explain that? I think when I did the first work on PLC4Net, I chose KNX as this used the most of our mspec-functionality ... I thought if I get this driver working, the rest will most probably also work. Usually when starting on a new language I started with Modbus as this is the simplest one of them all for which a test-bench is cheaply available. KNX also had this super odd encoding for 16 bit float which isn't really a full half-precision-iec float. In the new SPI3 java version, I think I used a dedicated float implementation: The implementation is here: However, I built things in SPI3 so theoretically a protocol module could bring along it's own Encoding implementation by putting the class in the driver module and registering it via a org.apache.plc4x.java.spi.buffers.bytebased.encoding.Encoding property file. As you could see I never really got very far with the .Net version as the person that claimed to want to help decided to go away. Usually my process was that I hand wrote some types from Modbus in the target language, then I copied the java templates and started adjusting them till the output matched the hand-written one. the data-io template you found was simply one copied but never touched by me as the work stopped before I had time to do that. As you saw on the discussion list, we're currently considering splitting out Go and also creating go-native code generators. Possibly this is something you would also like to do. Initially the idea was to have one system for generating code. One system anyone would understand and maintaining would therefore be easier. Also is it one thing to understand a programming language, but it's a totally different thing to understand the tooling available in a language, the best practices and how to nicely integrate this into a smoothly running build. For java and maven we knew how to do this, for the others not so much ;-) So the current template+maven approch was simply a quick win that we knew how to do. However, have I found out with my own closed-source work, that there are huge benefits for running this in a tool-native fashion ... then I guess only the resolution of the protocol modules would need to be implemented and the code-gen is built per language. The mspec format is documented in an Antlr4 grammar for which there should be tooling in most languages. The core mspec format is described here: I hope I manged to answer your questions ... if not ... don't hesitate to ask ... ideally here ... if it has to be quickly and you see I'm online in Slack, just ask there. |
Thanks — knowing the goal is to make the libraries feel natural in their normal On the ConnectionString divergence — it does not hold up The three states side by side: (*) = the full connection-string parser (1) and (2) are the same shape. I broke that in (3). What happened: I read "Uri cannot parse the whole s7:cotp://host form" — which 
Java has exactly the same limitation on the two-scheme form and lives with it, Counting the consumers settles which module the type belongs in: api/PlcDriverManager.cs:60 ConnectionString.Parse(..) 1 site <- the line I added Revert that one line and the type has no consumers left in the api module. So it On the general approach My intent throughout has been to follow how the Go and Java modules are used so On KNX I did not choose it, I inherited it — plc4net/drivers/knxnetip/src already On tool-native code generation I agree with building it per language. Each language has its own idioms, tooling Where I would like to go next
One request This PR is really me probing for direction rather than proposing something |
Christofer Dutz raised on PR apache#2656 that develop's prerequisite check now looks for the .NET 7 SDK and that the root README's .NET section is stale. The revived port targets net8.0 (LTS), so: - prerequisiteCheck.groovy — checkDotnet() requires 8.0.0 (was 7.0.0 on develop, 4.5.2 before that); the inline comment no longer describes the removed net452 / LangVersion 11 setup. - README.md — the language list drops "abandoned"; the PLC4Net build prerequisites drop the obsolete ".NET Framework 4.5.2 targeting pack / Mono" step and ask for the .NET 8 SDK. - THREAT-MODEL.md, website/.../users/pages/index.adoc — the four spots that quoted the old README wording are synced. The plc4net scope carve-out is unchanged: still out of the model, on the independent ground that it carries no "supported" mark in the protocols index. No source or generated-model change; all three .NET CI checks still pass locally (license headers, generated-code-is-current, 428 tests).
|
I wonder if this could utilize https://docs.github.com/en/pull-requests/how-tos/stacked-pull-requests to seperate the generated code from this PR. This way we had the slim base of just the changes and then a stacked pull request adding the generated code. ATM it is for example not possible to do CoPilot reviews as it just bails due to size |
tools/s7-verify last ran on 2026-09-03, before develop was merged, the prerequisite check moved to the .NET 8 SDK and AssemblyVersion was pinned. Re-ran it against the S7-1214C at 39e3792 to confirm those build and infra commits did not regress the S7 driver. - s7-hardware-report.md — new dated section: PASS 12/12 on two consecutive runs, negotiated PDU 240 bytes, DB100 layout unchanged from 2026-09-03; an added %I0.0 single-read probe (Ok) exercises %I-area addressing outside the data block. Docs only. No source or generated-model change; the S7 driver is untouched since the 2026-09-03 run.
Thanks — the size problem is real. Copilot has refused it since it crossed 20k lines, and 651 files is a lot to put in front of a human. What this PR is right now — an end-to-end revival spike, not a finished contribution. What I'm driving toward is S7 and Modbus working against real hardware: S7 passes on an S7-1214C now, Modbus is next, both within a week I'd expect. Until that path is proven I don't want to freeze the module boundaries, so decomposing now would be guesswork. Once it's proven — split into develop-targeted PRs, each Copilot-sized and reviewable on its own: the SPI3 runtime, then the pure-.NET code generator, then the transports, then one per driver. The generated model classes (424 of the 651 files) travel with their driver — regenerable from mspec, not something to read line by line. |
|
Couldn't you just delete the generated files ... they should be built as part of the built itself, right? And just be carefull to not re-commit them. ... Could add a temp gitIgnore rule. |
Not quite, actually — the build doesn't regenerate the models today. They're committed, and rebuilt only under -Pupdate-generated-code, same as plc4j and the other ports. So dotnet build just compiles what's checked in — there's no generator step to lean on yet. I'm already set on making this reviewable: the plan I gave sruehl splits it into per-driver PRs, and that's what gets each piece under the bot limits. Generating at build instead of committing is a bigger move than the gitignore rule suggests. It turns tools/code-gen into a hard build dependency of every driver — today a generator bug only trips the isolated drift check, not the whole build — and it's the generate-on-build model you moved plc4j away from for reproducible builds. It'd also make plc4net the only port not committing its generated code. There's a real case for it — the drift-check job and the noisy regenerated diffs on generator PRs only exist because the files are committed. I'll start the per-driver split regardless — I'd rather take up build-time generation separately once the split's landed, if it's still needed then. Given the scope and the reproducibility point, I'd like to hear more of your thinking here before deciding. |
Running tools/modbus-verify against a scripted Modbus/TCP slave (ahead of the real S7-1214C run) surfaced two bugs, neither hardware-specific: - ModbusConnection.Read (TCP) only handled Coil and HoldingRegister; DiscreteInput and InputRegister fell through to the default AccessDenied branch without ever reaching the wire. ModbusRtuConnection already handled all four tag types - the TCP connection was the incomplete one. Added the missing cases, mirroring ModbusRtuConnection's BuildReadPdu / ParseReadResponse. - modbus-verify's PrintValue probed IPlcValue.IsBool() first, which plc4net's value model answers true for every numeric adapter too (matching plc4j's PlcValue coercion semantics) - so a holding-register read that correctly produced PlcUINT(0x1000) printed as "True (BOOL)". The value was right, the report was wrong. Now renders by the Modbus tag type actually read, the way s7-verify already does, instead of probing the value. ModbusDriverTests gets two new TCP round-trip tests pinning the input-register and discrete-input wire format and decoding. 430/430 tests pass (was 428, +2). Re-verified end-to-end against a scripted Modbus/TCP slave: holding/input registers report their UINT16 value, coil/discrete their BOOL, an out-of-range read still maps to InvalidAddress.
|
are those then stacked PRs? |
Not the GitHub stacked-PR mechanism, no — each targets develop directly, opened one after the previous merges. They're dependency-ordered either way (each needs the last one's code to compile), so a formal stack wouldn't buy much here; it mainly helps when multiple people need to review different layers in parallel, which isn't the situation with one contributor. Happy to switch to literal stacking if you'd rather review them without waiting on each merge. |
|
Quick update: physical Modbus RTU verification is currently blocked by the RS-485 hardware setup. An independent raw-serial test sends a valid request, but receives no response either, so there is currently no evidence that this is a plc4net driver issue. I still need a known-good USB-to-RS485 adapter or an oscilloscope to isolate the converter from the CM1241. My proposal is to keep this limitation documented and proceed with the smaller, dependency-ordered PRs. Physical RTU verification can follow separately once the hardware link is confirmed. |
Signed-off-by: TomNewChao <chaotomzhu@gmail.com>
Signed-off-by: TomNewChao <chaotomzhu@gmail.com>
Signed-off-by: TomNewChao <chaotomzhu@gmail.com>
SerialTransportInstance's receive loop awaited _port.BaseStream.ReadAsync(buf, 0, toRead, ct), which does not observe the CancellationToken on a real SerialPort. Replaced it with a loop that checks _port.BytesToRead and does a synchronous _port.Read() when bytes are available, otherwise a short Task.Delay(1, ct). modbus-verify gained the diagnostics used to keep chasing the Modbus RTU hardware round-trip: --quantity/--count (1-125) to read multiple registers/coils in one raw exchange, a response-shape check against the requested unit id/function code/byte count with register-value decoding, a disconnect-listener hookup that logs when the receive loop stops, and a 100ms settle delay after opening the port before the first frame goes out. When --quantity is not 1 the driver-read step is skipped -- ModbusTag and ModbusRtuConnection's read path are still single-value. 449 tests green (404 spi-test + 45 knxnetip-test), 0 warnings.
…14C+CM1241 attempt
The Siemens S7-1214C + CM 1241 (RS422/485) Modbus RTU rig never completed a
round trip: request framing was byte-correct and the CM 1241 acknowledged
receiving it, but no response ever reached the master. The fault was
isolated to the RS-485 bench link (a CH340 adapter that repeatedly dropped
off USB, marginal idle bias with no termination fitted, and an edge-
triggered Modbus_Comm_Load whose instance DB carried state across RUN-mode
downloads) rather than the driver.
Switched the verification target to a Mitsubishi QJ71C24N running its
non-procedure ("no-protocol") communication mode as a fixed-response
Modbus RTU slave -- it has no native Modbus slave firmware, so its ladder
program answers every request with the same canned response regardless of
function code or address. It hit the same "request received, nothing sent
back" symptom at first; the cause was a ladder-side M-relay interlock
gating the response-send (G.OUTPUT) rung, not the link. With that cleared:
-> 01 03 00 00 00 01 84 0A (unit 1, read holding register 0)
<- 01 03 08 00 01 00 02 00 03 00 04 0D 14 (13 bytes)
CRC valid, register values 1/2/3/4 (D1000-D1003), and ModbusRtuConnection's
driver read returns 1 for holding:0 -- both the raw wire path and the
driver path verified, repeatably.
Updated modbus-hardware-verification.md (QJ71C24N/GX Works2 setup replaces
the CM 1241/TIA Portal steps, troubleshooting table, --baud 9600 examples),
modbus-hardware-report.md (new run log entry; the CM 1241 link post-mortem
stays as history), design.md (GAP-1 and GAP-2 marked done), and
testing.md's hardware-verification section.
…ale counts The S7 and Modbus procedures and run logs lived in four files that repeated each other's "run it" and packaging sections and had to be kept in sync by hand. They are now one `docs/hardware-verification.md`: a status table, the shared harness usage, then a section per protocol (setup, coverage, troubleshooting, dated run log, PLC-side evidence). Content is otherwise unchanged. Added PLC-side evidence for the Modbus RTU run: the QJ71C24N bench photo and the GX Works2 ladder capture showing the G.INPUT and G.OUTPUT rungs with the interlock that had been blocking the response, and D200 monitoring as 0x0301 - the first two bytes of the canned response. Stated the scope limits plainly, in the new document and in design.md's gap list: the QJ71C24N is a fixed-response stand-in that answers every request identically, so it proves the serial transport, RTU framing, CRC and the driver read path, but not slave-side address-range or exception-code handling. Modbus TCP was verified against a software slave, not hardware. KNXnet/IP has no hardware verification at all. Corrected counts and claims that had drifted: - testing.md summary 428 -> 449 total, spi-test 383 -> 404 - design.md project tree 344 -> 404 spi-test cases - design.md GAP-5 now records the expanded 50/50 run and the persistent matrix, not just the original 12/12 - design.md GAP-9 said the version stays 1.0.0-SNAPSHOT; it is 1.1.0-SNAPSHOT in Directory.Build.props. Also states plainly that publishing to nuget.org is a PMC decision via the ASF release process, not a contributor action. - design.md's in-progress table listed "PR apache#2656 title / draft removal" as the goal; that PR is now a draft umbrella and the work is decomposing it into reviewable slices against develop. Repointed the inbound references in s7-verify.csproj, modbus-verify.csproj and s7-verify's Program.cs at the merged document. 449 tests green, 0 warnings.
…on.g4 tools/code-gen/Expression.g4 is a verbatim copy of the shared grammar under code-generation/. Upstream carries the full Unlicense public-domain dedication in the file itself, crediting bkiers/tiny-language-antlr4, and the root LICENSE lists that path as Category A "UNLICENSE". The copy had that dedication replaced with an ASF Apache-2.0 header, leaving only a one-line "Based on ... (Unlicense)" comment, and the copy was never added to LICENSE. Stripping a third-party notice and stamping an ASF header on a public-domain file is not ours to do, and it would not survive a release audit. The file is now byte-identical to the upstream grammar again - the body never diverged, only the header did - and LICENSE names both paths. Since the file legitimately carries no ASF header, it is excluded from the header checks that would otherwise demand one: the git-ls-files check in the .NET workflow, and apache-rat via plc4net/pom.xml (which uses the correct <inputExclude> element; the root pom's **/Expression.g4 entry is spelled <excinputExcludelude> and so does not apply - that looks like an unrelated find/replace accident upstream, left alone here). Two build-honesty fixes alongside it: - The update-generated-code profile regenerated only modbus and s7, while the generated-code-is-current CI job regenerates knxnetip as well and then fails on drift. A contributor running the Maven profile locally could not reproduce a CI drift failure. Added the knxnetip execution and a note that the two must stay in step. The stale comment claiming knxnetip "is not regenerated yet" is gone; it has been generated and drift-checked since the generated KNX model replaced the Java-plugin output. - drivers/knxnetip declared a PackageReference on NLog that no source file in the tree uses. It shipped in the package and created a notice obligation for nothing. Removed. 449 tests green, 0 warnings. License-header check passes with the exclusion.
TomNewChao
changed the title
The five PLC-side captures were markdown links, so opening the hardware verification document showed a list of filenames and none of the evidence. They are embedded now, each with a caption that says what to look at: - the S7-1214C bench, and the QJ71C24N bench with the RS-485 wiring that presents COM3 - DB100 monitored in TIA Portal, so the non-optimized offsets the procedure builds can be checked against the addresses the driver uses - the I/Q/M watch table after the persistent write - the GX Works2 ladder, where the M2000 send-side interlock and the G.INPUT / G.OUTPUT rungs are visible, along with D100 and D200 both monitoring as 0x0301 and D205/D206 holding the response tail and CRC The S7 test-rig photo was 5.2 MB at 4032x3024, which was tolerable as a link and is not once it renders inline on every view. Resized to 1600x1200, 345 kB. The changelog now records that resizing is the only processing applied to any of these images.
The document opened with how to run the harnesses and buried the results in a reverse-chronological run log, so a reader had to reach the middle before learning whether anything passed. It also carried an eight-entry changelog, several of whose lines were timestamped to the minute - a working diary that duplicated both the run log and git history. Reordered to conclusion first, then evidence: a Verdict table with the scope limits stated immediately under it, then one section per protocol with the same four parts each - hardware under test, how to reproduce, results, coverage - followed by the PLC-side images and a troubleshooting table. - The run log is gone as a structure. The current numbers are the headline in Results; the earlier passing runs are three sentences; the one blocked run became a troubleshooting row, which is where its diagnostic value actually lives (a uniform AccessDenied across DB, %I, %Q and %M means access policy, not addressing). - The retired S7-1214C + CM 1241 Modbus rig moved to an appendix. It is kept because the failure analysis is the reason the target changed, but it is no longer interleaved with results that did pass. - The s7-verify flags are a table now rather than five paragraphs, with the physical-output warning attached to the flag that carries the risk. - The changelog is gone. The one load-bearing claim in it - that resizing is the only processing applied to any image - moved to the evidence section, where someone judging the evidence will actually see it. 620 lines to 438, with no verification content dropped.
The evidence section introduced three photographs with a sentence about confirmation coming "from somewhere other than the tool under test" and a declaration that no image had been "cropped, annotated or otherwise altered". That is chain-of-custody language for a bench photo and a pair of TIA Portal screenshots. It says nothing the reader cannot see, and protesting that hard about untouched evidence invites the suspicion it was meant to deflect. They are pictures of the rig the tests ran on, so the section says that now. The resize note went with it - the commit that resized the photo already records it, and git has the original. Also made the four coverage bullets parallel - full / wire and framing only / no hardware / none - so the verdict can be read at a glance instead of parsed out of prose.
…CP reproducible Two troubleshooting entries described ModbusRtuConnection behavior that 9767478 already fixed: - "reads whatever is available once >= 4 bytes arrive, no expected-length or t3.5 check" - SendAndReceiveLocked reads a 3-byte header, computes the exact frame length from the function code, and waits for that many bytes with a real timeout. This has been true since 2026-09-04. - "the driver does not strip [the half-duplex echo]" - it does, for reads, by comparing the received bytes against the sent request before parsing. The Appendix carried a third copy of the same claim, suggesting the length check as future work on the retired CM 1241 rig. That paragraph is dated 2026-09-06, two days after the fix landed - it was wrong the day it was written, not just stale by the time this document reached it. Removed. Made the Modbus TCP hardware-verification result reproducible. It was run against "a minimal raw-socket Modbus/TCP slave" per e86bdd0's own commit message, but that slave was never checked in, so PASS 5/5 was not something a reader could rerun - the only other Modbus/TCP fixture in the repo is plc4j's pymodbus-based test_server.py, built for a 32-type register map and an RTU/ASCII/TLS/UDP matrix that plc4net's single-table TCP check has no use for. Added tools/modbus-tcp-sim.py, a ~110-line stdlib-only script serving exactly the four values and the one out-of-range address the original run used, and reran modbus-verify against it: all five results match the document byte for byte. The reproduction commands are now in the doc. Also: gave the two Exception-code troubleshooting rows their actual scope (the QJ71C24N RTU rig can't produce either, since it answers every request identically regardless of address - the TCP fixture is what demonstrates 0x02, at holding:200) rather than the vaguer "only possible against a real slave," which excluded the fixture that proves it. Spelled out the S7 Verdict-table date range across its three verification dates instead of naming only the last one. Gave the Modbus RTU Verdict row an honest "1 raw exchange, 2 driver reads" rather than reaching for a pass/fail ratio modbus-verify has no counter for. 449 tests green, 0 warnings.
Close() cancelled the token, then closed and disposed the port without ever waiting for the background read loop to actually stop -- the Task.Run handle from the constructor was discarded. .NET's SerialPort has documented issues where Close()/Dispose() racing a concurrent Read() on another thread can hang rather than cleanly cancel (dotnet/runtime#20362, dotnet/corefx#36040). Close() now waits on the read-loop task with a bounded 1s timeout after tearing the port down. That wait would deadlock if a listener callback invoked synchronously from the read loop calls Close() itself -- the loop can't finish while its own callback is still on the stack. Guarded against it the same way TcpTransportInstance already does: track which thread is currently running a listener callback and skip the wait when Close() is called from that same thread. Also: - The idle poll backed off from a fixed 1ms Task.Delay whenever BytesToRead was 0, spinning at up to ~1000 calls/sec for as long as the line stayed idle -- realistic for hours on a production connection, not just this diagnostic tool's short runs. Backs off from 1ms to a 20ms cap while idle, resetting the moment bytes show up. - The generic catch-all invoked the disconnect listener unconditionally, even when the exception was a side effect of Close()'s own teardown. TcpTransportInstance's equivalent already guards this with its open flag; SerialTransportInstance now checks _closed the same way. - Restored a comment explaining why BytesToRead-polling replaces BaseStream.ReadAsync(ct) (the latter doesn't observe the token on real hardware) -- the original comment was deleted by 7ad2d45 with nothing taking its place, so the reason not to "simplify" this back was undocumented. - Documented why a zero-byte Read() retries instead of ending the loop: the guards above only ever request a positive count already confirmed available, and SerialPort.Read throws TimeoutException rather than returning 0 for a positive count, so this path is believed unreachable today. 449 tests green, 0 warnings.
… formula PrintRawValues re-checked the response shape via HasExpectedReadShape(frame, frame[0], tag, quantity) -- passing the frame's own address byte as the expected unit id instead of the real unit id already validated a few lines earlier. That made the check's frame[0] == unitId comparison true by construction, so a reply from the wrong slave address on a shared bus could still have its register values printed. AnalyzeRaw already reports an address mismatch separately, so this was a redundant check quietly doing nothing rather than a silent wrong answer, but it should do what it looks like it does. PrintRawValues now takes the already-computed shapeOk instead of recomputing a broken version of it. HasExpectedReadShape's frame.Length == dataBytes + 5 duplicated ModbusRtuConnection.SendAndReceiveLocked's 3 + head[2] + 2 -- the same arithmetic for the same four read function codes, maintained independently in two files. Extracted to ModbusFunctionCodes.ReadResponseRtuFrameLength, used by both, so a future change to one can't silently leave the other validating against a stale rule. Also: - Named the RTU response's byte offsets (address/function/byteCount/data) instead of hand-indexing them a third time in PrintRawValues on top of AnalyzeRaw and HasExpectedReadShape. - --quantity/--count validation ran before VerifyRtu's own try block, so a bad value crashed with a raw stack trace instead of this tool's usual Markdown "## Error" block that every other failure path produces. Wrapped it. - The disconnect-listener callback wrote directly to Console.Out from the background receive-loop thread, which could interleave with the main thread's own sequential report output. It now records the message and the main thread prints it once, in the finally block. - The quantity != 1 summary table had a "Raw response shape valid" row the quantity == 1 table didn't, despite both computing the same shared rawShapeOk, plus cosmetic drift (Mark(true) vs a hardcoded checkmark, a hyphen vs an em dash in an otherwise identical heading). Made the two tables consistent. Verified against the real QJ71C24N frame captured in hardware-verification.md (01 03 08 00 01 00 02 00 03 00 04 0D 14): still decodes to register values 1, 2, 3, 4, and now correctly fails the shape check when checked against the wrong unit id, which the old code could never do regardless of input. 449 tests green, 0 warnings.
serve() had no exception handling anywhere. A client disconnecting mid-response -- a reset socket, a killed modbus-verify run, a timeout -- raised BrokenPipeError/ConnectionResetError uncaught, past the per-connection `with conn:` block and the outer accept loop, killing the entire simulator process rather than just that connection. Every subsequent hardware- verification reproduction attempt then failed with connection refused until someone noticed and restarted it by hand. Each connection's handling is now wrapped, logging the drop and moving on to the next client. pdu = conn.recv(length - 1) assumed a single recv() call returns the whole PDU; socket.recv(n) is documented to return "up to" n bytes, not exactly n. Added a recv_exact helper, used for both the 7-byte MBAP header and the PDU, that loops until the requested size arrives or the peer closes early. Verified directly: the five rows documented in hardware-verification.md still pass, the simulator survives an abrupt reset mid-request and keeps serving new connections afterward, and a request sent one byte at a time still decodes correctly.
The root inputExclude for **/Expression.g4 was spelled <excinputExcludelude> -- a dead XML element apache-rat-plugin has never recognized. Present on plain upstream/develop too, so it predates the plc4net work; cf7a430 added a second Expression.g4 relying on this same non-functional line and explicitly deferred fixing the typo as unrelated. Fixing it now that it's been run down.
eb56666 merged four hardware documents into one and said content was otherwise unchanged, but three pieces of evidence didn't make it across: - The S7 persistent Q-write table's Before values, proving each write actually changed the output rather than the CPU already holding that value. Restored for the seven Output rows that genuinely have one (from the 2026-09-16 persistent run); Marker and DB rows are marked "not logged separately" rather than backfilled with an assumed value, since that run's own notes say marker writes weren't requested and DB100 is covered by the restore-tested suite instead. - The instruction to promote a working --device-group/--remote-tsap value into the driver's own documentation and defaults once found. - The specific reasoning ruling out the SerialTransportInstance receive-loop rewrite as the cause of the earlier Modbus RTU "zero bytes back" failures -- both the old and new loop hit the identical symptom, pointing at the link/ interlock rather than the loop. This only existed in docs/modbus-hardware-report.md, a file created and deleted within the same merge commit, invisible to a plain two-endpoint diff of the branch. Without it, the surviving "first hardware confirmation" sentence read as more strongly evidenced than what the document actually showed.
What this PR is
An end-to-end revival of
plc4netas a buildable, testable .NET 8 implementationof the PLC4X API/SPI. It replaces the abandoned
net452build, aligns the runtimewith SPI3, adds a pure-.NET
.mspec→ C# generator, and provides working Modbus,S7 and KNXnet/IP drivers with generated wire models.
It is deliberately a spike, not a merge candidate. Its job was to prove the
stack works against real hardware before the module boundaries were frozen —
decomposing earlier would have been guesswork. That job is now done, so this PR
stays a draft and serves as the reference, while the work lands as a series of
reviewable PRs against
develop.What I'd like from this thread: agreement on the decomposition list below.
Nothing else is blocking. If the order looks wrong to you, that is much cheaper
to say now than after the first slice is open.
Targets
develop/1.1.0-SNAPSHOT, relates to #2655, follows thedev@discussion from 2026-07-24. Currently 52 commits ahead of the merge base, 38
behind
develop.Proposed decomposition
Sizes for PR0 and PR1 are measured — I built that reduced tree and ran it. The
rest are estimates from the current file layout.
net452knxproj driver (drivers/knxnetip+ its test) and the build wiring that fed itpom.xmlbridge, LICENSE/NOTICE,api/,spi/, the loopback test transport, and the SPI test project — this is what replaces the remainingnet452api/andspi/.mspec→ C# generator — tool only, no generated modelsTwo notes on the shape of that list:
diff ~58k lines, which is worse than the status quo. Split out, the deletion is
reviewable at a glance —
--statplus the fact that nothing outside itreferences the removed tree.
this move, Modbus is the one holdout.
S7Connectioncalls the generatedS7Message.StaticParse/Serializedirectly, and so doesKnxNetIpMessageCodecforKnxNetIpMessage.ModbusConnection/ModbusRtuConnectionstill hand-roll PDU and framing bytes indrivers/modbus/messages/ModbusPDU.cs/ModbusCRC.cs, entirely separate fromthe generated
ModbusPDU/ModbusTcpADU/ModbusRtuADUtypes that onlyModbusGeneratedRoundTripTestsexercises today — so the shared-vector proofand the hardware evidence currently cover different code. PR4 does for Modbus
what already happened for the other two, rather than shipping the hand-written
codec now and deleting it later. This is a real migration, not omitted code —
a handful of
ModbusDriverTestsassert directly on the hand-written builders'byte output and will need rewriting or retiring, and some of that coverage is
already redundant with the generated round-trip tests. Since
ModbusPDUitselfdoesn't distinguish TCP from RTU (only
ModbusTcpADUvsModbusRtuADUdo),PR4 should build the PDU layer protocol-neutral so PR6 only swaps the framing
layer instead of repeating the PDU work.
Generated model trees travel with their driver's PR. They are regenerable from
the
.mspecand are not meant to be read line by line; each driver PR will sayso explicitly so reviewers can skip them.
Hardware verification
Full procedures, run logs, troubleshooting tables and PLC-side image evidence:
plc4net/docs/hardware-verification.md.tools/modbus-tcp-sim.py— no Modbus/TCP device on handBeing precise about what each of those is worth:
Communication (PDU 240), every scalar width from a DB, absolute
%I/%Q/%Mbit/byte/word/dword addressing, read-before → write → read-back for all seven
types across DB100, M100..M117 and Q0..Q17, then every target read again over
fresh connections. Plus an error path. The run also found and fixed a real
driver bug: a bare Ack (ROSCTR
0x02) carries the same 2-byte header error asan AckData and must be framed as 12 bytes, otherwise every following response
desyncs.
no native Modbus slave firmware, so its ladder answers every request with the
same canned frame regardless of function code, address or quantity. That proves
the serial transport, RTU framing, CRC and the driver read path against a real
byte-at-a-time UART. It does not exercise slave-side address-range handling or
exception codes. An earlier attempt against an S7-1214C + CM 1241 running
MB_SLAVEwas retired after its RS-485 bench link never completed a round trip— isolated to the link, not the driver.
(
tools/modbus-tcp-sim.py), not a physical device — reproducible by anyonereading this, but still not hardware.
gateway in the test suite, which I wrote from the same reading of the spec as
the driver — so it cannot catch a shared misunderstanding.
PLC-side evidence, in TIA Portal and GX Works2 rather than from my own tooling —
the S7 absolute addresses after the persistent write, read back independently:
Also checked in: DB100 online values,
the S7-1214C bench,
the QJ71C24N bench,
and the QJ71C24N ladder
showing the
G.INPUT/G.OUTPUTrungs behind the interlock that had beenswallowing the response.
Automated verification
dotnet build plc4net/plc4net.sln --no-restore --no-incremental: 0 warnings,0 errors.
dotnet test plc4net/plc4net.sln --no-build: 449/449 passed (404 SPI/driverParserSerializerTestsuite.xmlvectors — the same bytes plc4j and plc4govalidate — byte-identically, plus hand-built IEC-61131 vectors for the
DataItemdataIovalue codec. KNXnet/IP handshake, group read/write and busmonitoring run against a scripted UDP gateway.
.mspecandfails on any drift, by staging and then
git diff --cached --exit-code.dotnet pack→ local folder feed →PackageReferencefrom an independent consumer project.Caveat I should state rather than let you find: GitHub Actions has never
produced a check run on this PR — the workflow requires authorization for
non-committer PRs. So every number above is from my machine. Getting any one
slice merged clears that gate and makes the rest self-verifying, which is another
reason PR0 is worth doing first.
Known limitations
The ones I would raise if I were reviewing this:
ITransportInstanceexposesGetNumBytesAvailable()/Read(n), so framing consumers spin with shortsleeps, and connect paths block on async work. plc4j is netty-driven and plc4go
channel-driven; this is the piece I would most want your opinion on before it
is set, because it is the hardest thing to change afterwards.
IPlcConnectionexposessubscription builders and six
IPlcSubscription*interfaces with noimplementation; the KNX bus monitor uses a driver-specific callback instead.
Dead public API in a published package is a compatibility trap — I would rather
remove it before any release than keep it.
<Nullable>annotations</Nullable>, notenable, so null-flow warnings areoff and the nullable annotations are currently documentation rather than
enforcement.
fragmentation of an oversized payload is not implemented.
tools/code-genforks the shared.mspecANTLR grammar (one lexer predicateported from the Java runtime API to the C# one). Nothing currently detects
divergence if the shared grammar changes; a CI step diffing the two would fix
that and I am happy to add it.
Provenance and licensing
plc4net/tools/code-gen/Expression.g4is a verbatim copy of the shared grammar.It had the upstream Unlicense dedication replaced with an ASF header, which was
wrong of me — the file is public domain and credits
bkiers/tiny-language-antlr4. It is now byte-identical to the upstream copyagain, both paths are listed in the root
LICENSE, and it is excluded from theASF header checks that would otherwise demand a header it should not carry.
While fixing that I noticed the root
pom.xmlRAT exclusion for**/Expression.g4is spelled<excinputExcludelude>, so it never applied. Thatlooks like an unrelated find/replace accident and I have left it alone — flagging
it in case it matters for the next release audit.
Review guide, if you want to look at the spike as-is
655 files changed, of which 422 are generated model or parser code. The
hand-written surface is ~230 files. A focused pass:
.github/workflows/dotnet-platform.ymlandplc4net/Directory.Build.propsplc4net/api/and the hand-written runtime underplc4net/spi/plc4net/tools/code-gen/excludingsrc/generated/plc4net/drivers/{modbus,s7,knxnetip}/plc4net/test/,plc4net/docs/design.md,plc4net/docs/hardware-verification.md