From 1f983d610f75f91254c5599d74041caa11ab99ba Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 9 Oct 2026 06:16:16 -0400 Subject: [PATCH 1/3] =?UTF-8?q?test(abcip):=20hw5=20hardware=20gate=20?= =?UTF-8?q?=E2=80=94=20#566=20idle=20close=20drops=20TCP;=20native=20STRIN?= =?UTF-8?q?G=20write=20on=20Z-35162=20(#592)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A. #566 on a real CompactLogix (Z-35162, read-only): a class-3 connection idled 60 s past its 16 s timeout is closed at the TCP level, not with CIP 0x0107/0x0203; the next request fails on its write (Broken pipe -> BadConnection) in 9 ms and the one after reopens. The transport already handles this, so no production behaviour changed; the shape is pinned offline by CipConnectionTests.A_target_that_resets_an_idle_session_... (FakeEipServer.ResetAllConnections), proven red with a reconnect-skipping mutation. The diagnostics-only exchange tap now also sees Forward Open replies (granted RPIs). B. Native STRING write through the driver on Program:ES03_TemperatureControl._StrTmp: write, read back, restored byte-exact (user-approved). Array write NOT run: the census's unreferenced DINT[32] _SEQ_EP arrays are aliased sequencer step registers (raw L5X grep), so none qualifies. #567 still impossible (a 128 B array / 88 B STRING never fragments). New env-gated harness: AbCip.IntegrationTests/HardwareGate (IdleConnectionEvidenceTests, StringWriteGateTests). Docs: gate results (hw5 section + raw output), AbCip.md, CLAUDE.md. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- CLAUDE.md | 2 +- docs/drivers/AbCip.md | 32 ++-- ...-10-07-native-ab-transport-gate-results.md | 82 +++++++++ .../hw5-idle566.txt | 16 ++ .../hw5-string-write.txt | 15 ++ .../Cip/CipConnection.cs | 4 +- .../Cip/ConnectionDiagnostics.cs | 4 +- .../Cip/CipConnectionTests.cs | 30 ++++ .../Fakes/FakeEipServer.cs | 18 ++ .../IdleConnectionEvidenceTests.cs | 136 +++++++++++++++ .../HardwareGate/StringWriteGateTests.cs | 164 ++++++++++++++++++ 11 files changed, 488 insertions(+), 15 deletions(-) create mode 100644 docs/plans/2026-10-07-native-ab-transport-t21/hw5-idle566.txt create mode 100644 docs/plans/2026-10-07-native-ab-transport-t21/hw5-string-write.txt create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/IdleConnectionEvidenceTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/StringWriteGateTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 0d1e228f..90c8c729 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1087,7 +1087,7 @@ re-emit a concrete default. The defaults differ per driver (final review, 2026-1 because the driver factory AND the admin-node Test Connect probe both read it. ⚠️ So **the upgrade restart alone moves no-key AbCip drivers to Native**; to stay on libplctag, set the node key — NOT a per-driver key through the old AdminUI, whose typed form drops unknown keys (#576). Evidence: AbCip reads proven on a real - CompactLogix (Z-35162); scalar native writes (DINT/BOOL/REAL) proven on it once (2026-10-09, user-approved one-off, restored) — and native UDT member writes (DINT/BIT/REAL, #611 gate, same day); array/STRING, libplctag member and fragmented writes are `ab_server`-only. + CompactLogix (Z-35162); scalar native writes (DINT/BOOL/REAL) proven on it once (2026-10-09, user-approved one-off, restored) — and native UDT member writes (DINT/BIT/REAL, #611 gate, same day) and a native STRING write (hw5, same day, restored byte-exact); array (element), libplctag member and fragmented writes are `ab_server`-only (no logic-unreferenced array exists on Z-35162 — its `_SEQ_EP` arrays are aliased sequencer step registers). #566: a real Logix drops TCP on a connection idled past its timeout (no 0x0107/0x0203); the faulted session reopens on the next request. - **AbLegacy: an absent key resolves to `Libplctag` for every family; `Native` is opt-in** (no node key). On a real MicroLogix 1400 (Z-33323, idle, 2026-10-09) native read S/N/F/B/T/C/R/ST/O/I incl. `S:0`, T/C/R sub-elements and status bits, and ST byte order, all correct, and wrote N/F8/B-bit (restored). libplctag (the default) diff --git a/docs/drivers/AbCip.md b/docs/drivers/AbCip.md index 214790cb..5c7b8576 100644 --- a/docs/drivers/AbCip.md +++ b/docs/drivers/AbCip.md @@ -165,8 +165,8 @@ The driver has two interchangeable wire transports, selected by one `DriverConfi the connection size. A read whose size cannot be bounded (structure / unknown authored type) is sent alone; writes and status-only replies count their 4-byte header. Pinned by `RequestSchedulerTests.Mixed_scalar_string_and_array_replies_are_packed_within_the_reply_budget`. - ⚠️ RESIDUAL: mixed STRING/array packing has not been exercised against a real controller (the real-press - gate predates the budget); the budget is unit-tested against the scheduler and fake channel only. + Exercised on a real CompactLogix (Z-35162, hw4, 2026-10-09): a STRING, an array element, a DINT[32] and two + scalars rode one Multiple Service Packet in 5/5 attempts, values equal to single reads. - **Fragmented-read integrity (#567):** `Read Tag Fragmented` replies are compared against the first reply; a changed type code *or* structure handle mid-read fails the read with `BadReply` rather than stitching two types into one value. @@ -179,9 +179,14 @@ The driver has two interchangeable wire transports, selected by one `DriverConfi and the next request reopens through the throttle. The request that got the reply fails `BadCommunicationError` (transport `BadConnection`); a keep-alive that gets it recycles the connection before any request fails. Every other status — tag-level errors, other `0x01` extended codes — is left to - the request. ⚠️ RESIDUAL: which of the two codes a real Logix controller returns for a forgotten - connection (and whether it answers at all rather than dropping TCP) is unobserved; the check is correct - whichever it does — a silent target still faults the session through the exchange timeout. + the request. **Observed on a real CompactLogix (Z-35162, hw5, 2026-10-09): neither code.** A connection left + idle past its 16 s timeout (keep-alive disabled for the test) was closed at the TCP level; the next request + failed on its write in 9 ms (`Broken pipe` → `BadConnection` → `BadCommunicationError`, no CIP or + encapsulation status) and the request after it reopened with a fresh Forward Open. That path needs no CIP + check — a faulted session always reopens on the next send — and is pinned offline by + `CipConnectionTests.A_target_that_resets_an_idle_session_fails_the_next_request_fast_and_the_one_after_reopens`. + The 0x0107/0x0203 recycle stays for targets that keep TCP up. In production the keep-alive (every 8 s) stops + a held connection idling out, so this shape follows a controller restart / re-download or a network gap. - `ConnectionSize` / `Timeout` / `AllowPacking` are honoured per pooled connection, and the **first** driver/device to open a given device key wins — including a Test Connect probe on a fused `admin,driver` node (its connection carries the probe timeout). A later opener with different options is ignored and @@ -241,16 +246,21 @@ The driver has two interchangeable wire transports, selected by one `DriverConfi and templates on Z-35162; a user-approved one-off on 2026-10-09 added **scalar native writes on a real CompactLogix** (DINT `test2`, BOOL `Program:UN01_Press.xxtest`, REAL `Program:ES03_EM03_Zone3._ff_test`: write, read back, restore, all Good), and the #611 gate added **native UDT member writes** (DINT, BIT and REAL members of `_EmptyTemperatureSegment`). - Still ab_server-only: array and STRING writes, libplctag member writes, and fragmented writes. Mixed STRING/array/scalar + hw5 (2026-10-09) added a **native STRING write** (`Program:ES03_TemperatureControl._StrTmp`, through the driver: + write, read back, restored byte-exact). Still ab_server-only: array (element) writes — no logic-unreferenced array + exists on Z-35162 (`_SEQ_EP` is aliased into live sequencer step registers) — libplctag member writes, and fragmented + writes. #566 is answered (the controller drops TCP; see "A target-closed CIP connection"). Mixed STRING/array/scalar Multiple Service Packet reads (#582) are proven on the press (one 5-read MSP, values equal to single reads). Results: [native-ab-transport-gate-results](../plans/2026-10-07-native-ab-transport-gate-results.md). -- **libplctag cannot be deleted yet.** Scalar native writes are proven on a CompactLogix (2026-10-09); what remains is #567 - (fragmented wide-struct writes), array/STRING writes on hardware, and AbLegacy native (simulator-only, no PCCC hardware). +- **libplctag cannot be deleted yet.** Scalar, UDT-member and STRING native writes are proven on a CompactLogix + (2026-10-09); what remains is #567 (fragmented wide-struct writes), array element writes on hardware, and AbLegacy + native beyond the MicroLogix 1400 evidence. - UDT **member writes** (#611) are proven on a real CompactLogix on the **native** transport only (2026-10-09); the libplctag member-write path is `ab_server`-only. See [Member writes](#member-writes). -- ⚠️ RESIDUAL (#567): wide-struct **fragmented writes** (8-byte alignment) are unit-tested only. Production - presses are read-only and there is no bench controller, so this cannot be proven; it stays open until a - bench controller exists. +- ⚠️ RESIDUAL (#567): wide-struct **fragmented writes** (8-byte alignment) are unit-tested only. A write fragments only + above `MaxRequestBytes` (4000 B on a Large Forward Open), so it needs a ~4 KB value; Z-35162 has no safe one (a DINT[32] + or a STRING is a single Write Tag, and AbCip has no whole-array write anyway). It stays open until a bench controller + or an approved large scratch tag exists. - Native browse (#572, resolved in code): listing paging no longer assumes ascending instance order, pages are paced, and a browse's template reads share one lease. Observed on Z-35162 (2026-10-09, 41 pages / 3162 tags): every page strictly ascending, so out-of-order listing has never been seen on a real controller — the diff --git a/docs/plans/2026-10-07-native-ab-transport-gate-results.md b/docs/plans/2026-10-07-native-ab-transport-gate-results.md index 9ff57893..faf825ab 100644 --- a/docs/plans/2026-10-07-native-ab-transport-gate-results.md +++ b/docs/plans/2026-10-07-native-ab-transport-gate-results.md @@ -702,3 +702,85 @@ ST14:10 String Native value="Z-33323" status=0x00000000 ( Every address reads identically on both transports. `S:4` is the free-running clock, so its value differs read to read on each transport (both Good). `I:0` = -29376 = 0x8D40, and bits `/6` and `/15` read true on both while `/0`, `/1`, `/7` read false — the bit is extracted from the word libplctag read, not defaulted. ab_server has no S/O/I files, so this real-controller read is the only wire evidence; the translation is unit-pinned against a model of the `pccc.c` parser (`LibplctagLegacyNameTests`). Caveats: `R6:0.DN` and `R6:0.EN` read `false` on both transports, so this read does not discriminate the control-word bit number (a wrong bit would very likely also read false) — the R bit mapping rests on the native table and the `pccc.c` parser model, not on this wire evidence. Writes: on libplctag every address the translation changes (`S:`/`O:`/`I:`, R/UA status bits) is refused `BadNotSupported` (#659 review), so this fix adds no libplctag write path. + +## hw5 — #566 idle-connection close and a native STRING write on Z-35162 — 2026-10-09 (user-approved) + +Press: Z-35162, CompactLogix 5069-L320ERS2 rev 34.11, `ab://10.209.70.160/1,0`, family CompactLogix, Mac VPN. The user +approved exactly two things: (A) one read-only idle-connection session for #566 and (B) native array and STRING writes on +logic-unreferenced tags. Harness: `AbCip.IntegrationTests/HardwareGate/` (its own namespace, outside the read-only +`Differential` guard), both tests env-gated. No packet capture (`tcpdump` needs sudo); the diagnostics tap +(`ConnectionDiagnostics.ExchangeObserver`, now also fed the Forward Open reply so the granted RPIs are visible) and the +transport's exception text are the evidence. + +### A. #566 — what a real Logix does with a class-3 connection idle past its timeout: it drops TCP, no CIP status + +`AB_DIFF_ENDPOINT=10.209.70.160:44818 AB_EVID_IDLE=1 AB_EVID_IDLE_SECONDS=60` → +`IdleConnectionEvidenceTests` (raw: [hw5-idle566.txt](2026-10-07-native-ab-transport-t21/hw5-idle566.txt)). One private +pool, keep-alive pushed out to 1 h so nothing touched the connection, open-attempt budget 2, three Read Tag requests of +`test2` (DINT) and nothing else. 10:04:49Z–10:05:54Z UTC. + +| t (s) | Event | +|---|---| +| 0.26 | Large Forward Open (0x5B) granted: O→T API = T→O API = 2 000 000 µs (the requested RPI, unchanged). Multiplier 1 (×8) ⇒ connection timeout **16 s** | +| 0.32 | read 1: general status 0x00 | +| 0.32 – 60.33 | idle, no traffic (3.75 × the timeout) | +| 60.33 | read 2: **failed in 9 ms on the write**: `IOException: Broken pipe` ← `SocketException(Shutdown)` → `AbTransportException(BadConnection)` → OPC UA `BadCommunicationError` (0x80050000). No CIP reply, no encapsulation status, `ConnectionsClosedByTarget` 0 | +| 62.47 | read 3: fresh Large Forward Open granted (new O→T id), read general 0x00 — **recovered on the next request**, open attempts 2/2 | +| 62.56 | lease released: Forward Close sent/acked 1/1 | + +- **Answer to #566: neither 0x0107 nor 0x0203.** The controller closed the TCP session; the next request's write hit a dead + socket. EPIPE on the *first* write after the idle (not a successful write followed by EOF) means the close was already + complete on the Mac's side, which is consistent with an RST (or FIN + RST) from the controller; without a capture the + segment type and the exact moment within the 60 s window are not observed — only that it happened after the 16 s + timeout and before 60 s. +- **The transport handles it correctly as it stands, so no production code changed.** The failed write faults the session + (`EncapsulationClient.Fault`, `IsConnected` false), the failing request alone fails `BadConnection` (fast, not an + exchange timeout), the driver maps that to `BadCommunicationError` and evicts the runtime / reports `Degraded` for that + poll, and the next send reopens through the Forward Open throttle. The 0x0107/0x0203 recycle (#566) stays: it is + correct for a target that keeps TCP up, and harmless here (not exercised). +- **In production this path needs more than idleness.** The keep-alive fires at half the expiry (8 s), so a connection the + driver holds never idles 16 s; the shape above arises after a controller restart / re-download or a network gap. +- **Pinned offline:** `CipConnectionTests.A_target_that_resets_an_idle_session_fails_the_next_request_fast_and_the_one_after_reopens` + replays the shape (a `FakeEipServer.ResetAllConnections` RST with no request in flight): `BadConnection` in < 4 s, + `ConnectionsClosedByTarget` 0, one Forward Open before / two after. Proven able to fail: with `EnsureOpenAsync` made to + reuse a non-connected session it goes red (the reopen never happens). + +### B. Native writes: STRING PASS; array NOT RUN (no tag qualifies) + +**STRING — `Program:ES03_TemperatureControl._StrTmp` (PASS, restored byte-exact).** Census: refs 0, extrefs 0, not in the +Galaxy map, program not HMI/Wonderware; raw grep of `SN3244.L5X` finds the name only at its declaration (line 69426). +`AB_HW_WRITE_GATEWAY=ab://10.209.70.160/1,0 AB_HW_WRITE_APPROVED=1 AB_HW_WRITE_STRING_TAG=Program:ES03_TemperatureControl._StrTmp` +→ `StringWriteGateTests`, through the production `AbCipDriver` on the native transport, 10:10:54Z–10:11:29Z UTC (raw: +[hw5-string-write.txt](2026-10-07-native-ab-transport-t21/hw5-string-write.txt)). A read-only dry run (`AB_HW_WRITE_DRYRUN=1`) +first confirmed the preflight. The raw bytes come from the tap on each Read Tag Fragmented reply. + +| Step | Status | Value | Raw (type, handle, LEN, DATA…) | +|---|---|---|---| +| snapshot 1 | Good | `ES03_TemperatureControl:Fault` | `A002 CE0F 1D000000 4553…6C74 00…` (92 B) | +| snapshot 2 (+30 s) | Good | same | identical to snapshot 1 | +| preflight | — | — | built-in STRING (0x02A0 / handle 0x0FCE / 88 B), LEN 29, every byte past LEN zero | +| write `OTOPCUA-TEST` | Good (0x00000000) | — | — | +| read back | Good | `OTOPCUA-TEST` | `A002 CE0F 0C000000 4F544F504355412D54455354 00…` | +| restore original | Good | — | — | +| read back | Good | `ES03_TemperatureControl:Fault` | **byte-equal to snapshot 1** | +| final snapshot (+2 s) | Good | same | byte-equal to snapshot 1 | + +The original matches the L5X export's value. The write is one unfragmented Write Tag (88 B of data plus the path is far +below the 4000-byte request limit of the Large Forward Open). + +**Array — not run; no candidate passes the brief's own rule.** The only logic-unreferenced, Read/Write atomic arrays in the +census are `Program:EM01_PowerUnit._SEQ_EP` and `Program:EM02_Clamp._SEQ_EP` (DINT[32]), and the raw grep shows they are +**not** unreferenced: aliases `_SEQ_EP01_AutoCycle → _SEQ_EP[1]` (EM01) and `_SEQ_EP01_Jog / _SEQ_EP02_Auto / +_SEQ_EP03_Stopping → _SEQ_EP[1..3]` (EM02) are the step registers of the power-unit auto cycle and the clamp jog / auto / +stopping sequencers, used by dozens of rungs (`MOV(…,_SEQ_EP02_Auto)`, `EQU(_SEQ_EP01_AutoCycle,300)`…). The census counts +references by tag name, so references through an alias were invisible to it. A whole-array write would rewrite live +sequencer state, and even a single unaliased element fails the "name appears only at its declaration" check. The other +arrays are referenced (`_DisableStates`, `_ModeTransitions`, `_ModeNames`, `_VacuumCalibration_*`, `Stored`, +`_monitorID`), `ExternalAccess="Read Only"` (`_SEQ_UP`, `Inp_CycleTime`) or framework lookup tables (`Inf_LookupFwkB`). No +array write was made. An element write needs a tag the user picks (or an unaliased element they approve explicitly). + +**#567 (fragmented wide-struct write) — still cannot be done here, and a DINT[32] would not have done it.** AbCip has no +whole-array write at the driver layer (array tags are read-only, `DriverArrayWriteSupport`); and at the transport layer a +128-byte array (or this 88-byte STRING) is a single Write Tag: `LogixTagClient.WriteAsync` fragments only when the request +exceeds `MaxRequestBytes` (4000 B on the Large Forward Open this controller grants). A fragmented write needs a value of +roughly 4 KB. diff --git a/docs/plans/2026-10-07-native-ab-transport-t21/hw5-idle566.txt b/docs/plans/2026-10-07-native-ab-transport-t21/hw5-idle566.txt new file mode 100644 index 00000000..333395cb --- /dev/null +++ b/docs/plans/2026-10-07-native-ab-transport-t21/hw5-idle566.txt @@ -0,0 +1,16 @@ +# hw5 #566 idle-connection gate, Z-35162, 2026-10-09 10:04:49Z-10:05:54Z UTC +# AB_DIFF_ENDPOINT=10.209.70.160:44818 AB_EVID_IDLE=1 AB_EVID_IDLE_SECONDS=60 -> HardwareGate/IdleConnectionEvidenceTests + Passed ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.HardwareGate.IdleConnectionEvidenceTests.An_idle_connection_past_its_timeout_reports_what_the_controller_does_and_recovers [1 m 2 s] + [ 0.00 s] Target 10.209.70.160:44818 path '1,0' family CompactLogix (compactlogix, connection size 4002), max tags 200, max templates 50, pace 20 ms; tag 'test2'; requested RPI 2000000 us both ways, timeout multiplier 1 (x8) => expiry 16.0 s; keep-alive pushed to 01:00:00; idle 60 s + [ 0.26 s] Forward Open 0x5B granted: O->T id 0xFFFE58C1, O->T API 2000000 us, T->O API 2000000 us; raw DB000000C158FEFF067DB1E37B4F3713143E526D80841E0080841E000000 + [ 0.32 s] exchange: request service 0x4C (12 B) -> reply 10 B: CC000000C40000000000 + [ 0.32 s] read 1 (opens): reply in 314 ms, general 0x00 + [ 0.32 s] Forward Opens 1; now idle for 60 s with no traffic + [ 60.33 s] read 2 (after idle): THREW after 9 ms: AbTransportException status=BadConnection (OPC UA 0x80050000) message='Connection lost: Unable to write data to the transport connection: Broken pipe.' inner=IOException('Unable to write data to the transport connection: Broken pipe.') <- SocketException(Shutdown) + [ 60.33 s] After read 2: Forward Opens 1 (delta 0), closed-by-target 0, success=False + [ 62.47 s] Forward Open 0x5B granted: O->T id 0xFFFE58C2, O->T API 2000000 us, T->O API 2000000 us; raw DB000000C258FEFF489E51887C4F3713143E526D80841E0080841E000000 + [ 62.51 s] exchange: request service 0x4C (12 B) -> reply 10 B: CC000000C40000000000 + [ 62.51 s] read 3 (recovery): reply in 178 ms, general 0x00 + [ 62.51 s] After read 3: Forward Opens 2, closed-by-target 0, open attempts used 2/2, success=True + [ 62.56 s] Lease released: Forward Closes sent/acked 1/1 + Total time: 1.0757 Minutes diff --git a/docs/plans/2026-10-07-native-ab-transport-t21/hw5-string-write.txt b/docs/plans/2026-10-07-native-ab-transport-t21/hw5-string-write.txt new file mode 100644 index 00000000..f5b68cad --- /dev/null +++ b/docs/plans/2026-10-07-native-ab-transport-t21/hw5-string-write.txt @@ -0,0 +1,15 @@ +# hw5 native STRING write gate, Z-35162, 2026-10-09 10:10:54Z-10:11:29Z UTC (user-approved; restored) +# AB_HW_WRITE_GATEWAY=ab://10.209.70.160/1,0 AB_HW_WRITE_APPROVED=1 AB_HW_WRITE_STRING_TAG=Program:ES03_TemperatureControl._StrTmp -> HardwareGate/StringWriteGateTests + Passed ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.HardwareGate.StringWriteGateTests.A_native_STRING_write_reads_back_and_restores_the_exact_original [32 s] + Gateway ab://10.209.70.160/1,0; tag 'Program:ES03_TemperatureControl._StrTmp'; transport Native; snapshot gap 30 s + snapshot 1: status 0x00000000, value 'ES03_TemperatureControl:Fault', raw (92 B) A002CE0F1D000000455330335F54656D7065726174757265436F6E74726F6C3A4661756C7400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + snapshot 2: status 0x00000000, value 'ES03_TemperatureControl:Fault', raw (92 B) A002CE0F1D000000455330335F54656D7065726174757265436F6E74726F6C3A4661756C7400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + Preflight OK: type/handle A002CE0F, LEN 29, tail zero; original 'ES03_TemperatureControl:Fault' + write test value: 'OTOPCUA-TEST' (ASCII 4F544F504355412D54455354) -> status 0x00000000 + read back test value: status 0x00000000, value 'OTOPCUA-TEST', raw (92 B) A002CE0F0C0000004F544F504355412D54455354000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + restore original: 'ES03_TemperatureControl:Fault' (ASCII 455330335F54656D7065726174757265436F6E74726F6C3A4661756C74) -> status 0x00000000 + read back restored: status 0x00000000, value 'ES03_TemperatureControl:Fault', raw (92 B) A002CE0F1D000000455330335F54656D7065726174757265436F6E74726F6C3A4661756C7400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + Test-value raw LEN 12; restored raw equals original: True + final snapshot: status 0x00000000, value 'ES03_TemperatureControl:Fault', raw (92 B) A002CE0F1D000000455330335F54656D7065726174757265436F6E74726F6C3A4661756C7400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + RESULT: write, read-back, exact restore — PASS + Total time: 34.8142 Seconds diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/CipConnection.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/CipConnection.cs index 52861634..221daf17 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/CipConnection.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/CipConnection.cs @@ -221,7 +221,9 @@ public sealed class CipConnection : ICipChannel, IAsyncDisposable _o.RpiMicros, size, _route); var frame = ConnectionManagerServices.ForwardOpen(p); _diagnostics.CountForwardOpen(); - return CipReply.Parse(await client.SendRRDataAsync(frame, _o.Timeout, ct).ConfigureAwait(false)); + var raw = await client.SendRRDataAsync(frame, _o.Timeout, ct).ConfigureAwait(false); + _diagnostics.NotifyExchange(frame, raw); // evidence tap: the granted RPIs / connection ids (#566 gate) + return CipReply.Parse(raw); } private ushort NextConnectionSerial() diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/ConnectionDiagnostics.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/ConnectionDiagnostics.cs index 2bc12f38..a474ab49 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/ConnectionDiagnostics.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/ConnectionDiagnostics.cs @@ -92,8 +92,8 @@ internal sealed class ConnectionDiagnostics } /// - /// Evidence tap for the gate harness (#582 / #572): sees the CIP request and the raw CIP reply of every - /// completed exchange. Null in production; set only by the read-only differential harness. A throwing + /// Evidence tap for the gate harness (#582 / #572 / #566): sees the CIP request and the raw CIP reply of every + /// completed exchange, Forward Opens included (their replies carry the granted RPIs). Null in production; set only by the read-only differential harness. A throwing /// observer is swallowed so it can never affect a request. /// internal Action, ReadOnlyMemory>? ExchangeObserver { get; set; } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/CipConnectionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/CipConnectionTests.cs index bffdec3f..28ea632c 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/CipConnectionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/CipConnectionTests.cs @@ -565,6 +565,36 @@ public sealed class CipConnectionTests target.ForwardOpens.ShouldBe(2); } + [Fact] + public async Task A_target_that_resets_an_idle_session_fails_the_next_request_fast_and_the_one_after_reopens() + { + // The shape a real CompactLogix produced (Z-35162, hw5 #566 gate): a session idle past its connection + // timeout was reset with NO CIP status, so the next write hit a dead socket ("Broken pipe"). It must surface + // at once as BadConnection (not wait out the exchange timeout), must not count as a CIP-level close, and the + // request after it must reopen with a fresh Forward Open. + await using var target = new FakeCipTarget(); + target.ConnectedHandler = cip => Task.FromResult(Ok(cip)); + var time = new ManualTimeProvider(); + await using var conn = Create(target, Options(timeoutSeconds: 5), time); + await conn.SendAsync(ReadTag, TestCt); + + target.Server.ResetAllConnections(); + await FakeCipTarget.Until(() => target.Server.OpenConnections == 0, Guard, "the reset to land"); + var started = System.Diagnostics.Stopwatch.StartNew(); + var ex = await Should.ThrowAsync(() => conn.SendAsync(ReadTag, TestCt)); + ex.Status.ShouldBe(AbStatus.BadConnection); + started.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(4), "a dead socket is not a timeout"); + conn.Diagnostics.ConnectionsClosedByTarget.ShouldBe(0); // no CIP status was involved + target.ForwardOpens.ShouldBe(1); + + var reopen = conn.SendAsync(ReadTag, TestCt); + await time.WaitForTimerDueAtAsync(time.Start + TimeSpan.FromSeconds(1), Guard); // through the throttle + time.Advance(TimeSpan.FromSeconds(1)); + CipReply.Parse(await reopen.WaitAsync(Guard, TestCt)).GeneralStatus.ShouldBe((byte)0); + target.ForwardOpens.ShouldBe(2); + target.Server.Connections.ShouldBe(2); + } + [Fact] public async Task Other_error_statuses_describe_the_request_and_leave_the_connection_alone() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Fakes/FakeEipServer.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Fakes/FakeEipServer.cs index b8ff62cb..d401868e 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Fakes/FakeEipServer.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Fakes/FakeEipServer.cs @@ -73,6 +73,24 @@ internal sealed class FakeEipServer : IAsyncDisposable /// Close the connection instead of answering the next non-session request. public bool DropOnNextRequest { get; set; } + /// + /// Reset every accepted connection (SO_LINGER 0 ⇒ RST) without any request in flight — what a real Logix + /// (Z-35162, Gitea #566 hw5 gate) did to a class-3 session left idle past its connection timeout: no CIP + /// status, the next request's write just fails on a dead socket. + /// + public void ResetAllConnections() + { + foreach (var c in _clients) + { + try + { + c.Client.LingerState = new LingerOption(true, 0); + c.Client.Close(); + } + catch (ObjectDisposedException) { /* already gone */ } + } + } + /// Queue one-shot responders, consumed one per non-session request. public void Script(params Func[] steps) { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/IdleConnectionEvidenceTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/IdleConnectionEvidenceTests.cs new file mode 100644 index 00000000..0ef43227 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/IdleConnectionEvidenceTests.cs @@ -0,0 +1,136 @@ +using System.Buffers.Binary; +using System.Diagnostics; +using System.Globalization; +using System.Net.Sockets; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport; +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip; +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Logix; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.Differential; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.HardwareGate; + +/// +/// Gitea #566 hardware evidence (hw5): what a real Logix does with a class-3 connection that has been left idle +/// past its connection timeout. READ-ONLY: three Read Tag requests of one tag on one private pool, nothing else. +/// +/// Opt-in: AB_DIFF_ENDPOINT (the target, see ) AND +/// AB_EVID_IDLE=1. +/// AB_EVID_IDLE_TAG — the tag read, default test2 (a DINT). +/// AB_EVID_IDLE_SECONDS — idle time, default 60, in [1, 300]. It must exceed the expiry the +/// controller grants (logged from the Forward Open reply), or the read just succeeds. +/// +/// The keep-alive is pushed out to one hour on this private pool so nothing touches the connection while it is +/// idle (in production it fires every half expiry, which is why this case only arises after a controller +/// restart / re-download or a network gap). The open-attempt budget is 2: the first open and one reconnect. +/// +[Trait("Category", "LiveIntegration")] +public sealed class IdleConnectionEvidenceTests +{ + private const string DefaultTag = "test2"; + + [Fact] + public async Task An_idle_connection_past_its_timeout_reports_what_the_controller_does_and_recovers() + { + var target = DifferentialTarget.FromEnvironment(); + if (target is null) Assert.Skip(DifferentialTarget.SkipReason); + if (Environment.GetEnvironmentVariable("AB_EVID_IDLE") != "1") Assert.Skip("AB_EVID_IDLE=1 not set."); + var tag = Environment.GetEnvironmentVariable("AB_EVID_IDLE_TAG") is { Length: > 0 } t ? t.Trim() : DefaultTag; + var idleSeconds = 60; + if (Environment.GetEnvironmentVariable("AB_EVID_IDLE_SECONDS") is { Length: > 0 } rawIdle + && (!int.TryParse(rawIdle, NumberStyles.None, CultureInfo.InvariantCulture, out idleSeconds) || idleSeconds is < 1 or > 300)) + throw new InvalidOperationException($"AB_EVID_IDLE_SECONDS='{rawIdle}' must be an integer in [1, 300]."); + + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(idleSeconds + 120)); + var ct = cts.Token; + var pool = new DeviceSessionPool(); + var diagnostics = pool.Diagnostics; + var p = target.Params(tag); + var key = NativeAbCipDevice.Key(p); + var options = NativeAbCipDevice.Options(p) with { KeepAliveInterval = TimeSpan.FromHours(1) }; + var clock = Stopwatch.StartNew(); + var log = new List(); + void Note(string line) + { + var stamped = $"[{clock.Elapsed.TotalSeconds,7:F2} s] {line}"; + log.Add(stamped); + TestContext.Current.TestOutputHelper?.WriteLine(stamped); + } + + diagnostics.ExchangeObserver = (req, rep) => Note(DescribeExchange(req.Span, rep.Span)); + using var budget = diagnostics.LimitOpenAttempts(key, 2); + var request = LogixServices.ReadTag(CipPath.EncodeSymbolic(tag), 1); + Note($"Target {target}; tag '{tag}'; requested RPI {options.RpiMicros} us both ways, timeout multiplier " + + $"{options.TimeoutMultiplier} (x{4 << options.TimeoutMultiplier}) => expiry {options.ConnectionExpiry.TotalSeconds:F1} s; " + + $"keep-alive pushed to {options.KeepAliveInterval}; idle {idleSeconds} s"); + + var lease = pool.Acquire(key, options); + try + { + var first = await ReadAsync("read 1 (opens)", lease.Channel, request, Note, ct); + first.ShouldBeTrue("the first read must succeed, or there is no connection to idle"); + Note($"Forward Opens {diagnostics.ForwardOpens}; now idle for {idleSeconds} s with no traffic"); + await Task.Delay(TimeSpan.FromSeconds(idleSeconds), ct); + + var opensBefore = diagnostics.ForwardOpens; + var second = await ReadAsync("read 2 (after idle)", lease.Channel, request, Note, ct); + Note($"After read 2: Forward Opens {diagnostics.ForwardOpens} (delta {diagnostics.ForwardOpens - opensBefore}), " + + $"closed-by-target {diagnostics.ConnectionsClosedByTarget}, success={second}"); + + await Task.Delay(TimeSpan.FromSeconds(2), ct); // past the 1 s Forward Open interval + var third = await ReadAsync("read 3 (recovery)", lease.Channel, request, Note, ct); + Note($"After read 3: Forward Opens {diagnostics.ForwardOpens}, closed-by-target " + + $"{diagnostics.ConnectionsClosedByTarget}, open attempts used {budget.Attempts}/{budget.Budget}, success={third}"); + third.ShouldBeTrue("the transport must recover on the request after the failed one"); + } + finally + { + await lease.DisposeAsync(); + diagnostics.ExchangeObserver = null; + Note($"Lease released: Forward Closes sent/acked {diagnostics.ForwardClosesSent}/{diagnostics.ForwardClosesAcked}"); + } + } + + private static async Task ReadAsync(string label, ICipChannel channel, byte[] request, Action note, + CancellationToken ct) + { + var sw = Stopwatch.StartNew(); + try + { + var reply = CipReply.Parse(await channel.SendAsync(request, ct)); + note($"{label}: reply in {sw.ElapsedMilliseconds} ms, general 0x{reply.GeneralStatus:X2}" + + $"{(reply.ExtendedStatus.Length > 0 ? $" extended {string.Join(",", reply.ExtendedStatus.Select(e => $"0x{e:X4}"))}" : "")}"); + return reply.GeneralStatus == 0; + } + catch (AbTransportException ex) + { + note($"{label}: THREW after {sw.ElapsedMilliseconds} ms: AbTransportException status={ex.Status} " + + $"(OPC UA 0x{AbOpcUaStatusMap.MapStatus(ex.Status):X8}) message='{ex.Message}' inner={Inner(ex)}"); + return false; + } + } + + private static string Inner(Exception ex) + { + var parts = new List(); + for (var e = ex.InnerException; e is not null; e = e.InnerException) + parts.Add(e is SocketException se ? $"SocketException({se.SocketErrorCode})" : $"{e.GetType().Name}('{e.Message}')"); + return parts.Count == 0 ? "none" : string.Join(" <- ", parts); + } + + private static string DescribeExchange(ReadOnlySpan request, ReadOnlySpan reply) + { + var service = request.Length > 0 ? request[0] : (byte)0; + var hex = Convert.ToHexString(reply[..Math.Min(reply.Length, 48)]); + if (service is 0x54 or 0x5B && reply.Length >= 4 && reply[2] == 0) + { + // Forward Open reply data: O->T id, T->O id, serial, vendor, originator serial, O->T API, T->O API. + var d = reply[(4 + 2 * reply[3])..]; + if (d.Length >= 24) + return $"Forward Open 0x{service:X2} granted: O->T id 0x{BinaryPrimitives.ReadUInt32LittleEndian(d):X8}, " + + $"O->T API {BinaryPrimitives.ReadUInt32LittleEndian(d[16..])} us, T->O API {BinaryPrimitives.ReadUInt32LittleEndian(d[20..])} us; raw {hex}"; + } + return $"exchange: request service 0x{service:X2} ({request.Length} B) -> reply {reply.Length} B: {hex}"; + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/StringWriteGateTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/StringWriteGateTests.cs new file mode 100644 index 00000000..e6eaa4c7 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/StringWriteGateTests.cs @@ -0,0 +1,164 @@ +using System.Text; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.HardwareGate; + +/// +/// Gitea #592 (hw5) — ONE user-approved native STRING write on a real controller, through the production driver +/// (, native transport): snapshot twice ≥ 30 s apart (raw bytes, from the exchange tap), +/// write a distinctive value, read it back, restore the exact original, read back, final raw snapshot equals the +/// original. Fails closed before ANY write when the tag is not a built-in 88-byte STRING, moved between the two +/// snapshots, or holds non-zero bytes past its LEN (the driver's encoder zero-fills, so such a value could not be +/// restored byte-exact). +/// +/// AB_HW_WRITE_GATEWAY — ab://host/1,0; absent ⇒ skip. +/// AB_HW_WRITE_STRING_TAG — must be one of (the brief's allow-list). +/// AB_HW_WRITE_APPROVED=1 — the explicit opt-in. +/// AB_HW_WRITE_GAP_SECONDS — snapshot gap, default 30, at least 30. +/// AB_HW_WRITE_DRYRUN=1 — stop after the read-only preflight (no write). +/// +/// Never set by any other run; (the simulator opt-in) is deliberately not +/// what enables this. +/// +[Trait("Category", "LiveIntegration")] +public sealed class StringWriteGateTests +{ + /// The only tags this gate may write: logic-, alias-, trend- and Galaxy-unreferenced (L5X census + raw grep). + private static readonly string[] Approved = + [ + "Program:ES03_TemperatureControl._StrTmp", + "Program:ES03_TemperatureControl._Prefix", + ]; + + private const string TestValue = "OTOPCUA-TEST"; + private const int BuiltInStringBytes = 88; + private const int StringCapacity = 82; + + [Fact] + public async Task A_native_STRING_write_reads_back_and_restores_the_exact_original() + { + var gateway = Environment.GetEnvironmentVariable("AB_HW_WRITE_GATEWAY")?.Trim(); + if (string.IsNullOrEmpty(gateway)) Assert.Skip("AB_HW_WRITE_GATEWAY not set."); + if (Environment.GetEnvironmentVariable("AB_HW_WRITE_APPROVED") != "1") Assert.Skip("AB_HW_WRITE_APPROVED=1 not set."); + var tagPath = Environment.GetEnvironmentVariable("AB_HW_WRITE_STRING_TAG")?.Trim() ?? ""; + if (!Approved.Contains(tagPath, StringComparer.Ordinal)) + throw new InvalidOperationException($"AB_HW_WRITE_STRING_TAG='{tagPath}' is not on the approved list."); + var gap = 30; + if (Environment.GetEnvironmentVariable("AB_HW_WRITE_GAP_SECONDS") is { Length: > 0 } rawGap + && (!int.TryParse(rawGap, out gap) || gap < 30 || gap > 300)) + throw new InvalidOperationException($"AB_HW_WRITE_GAP_SECONDS='{rawGap}' must be in [30, 300]."); + + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(gap + 120)); + var ct = cts.Token; + const string Ref = "gate-string"; + var diagnostics = DeviceSessionPool.Shared.Diagnostics; + var replies = new List(); + diagnostics.ExchangeObserver = (req, rep) => + { + // Read Tag Fragmented (0x52) replies only: service 0xD2, status 0, then type 0x02A0 + handle + data. + if (req.Length > 0 && req.Span[0] == 0x52) lock (replies) replies.Add(rep.ToArray()); + }; + + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(gateway, AbCipPlcFamily.CompactLogix, DeviceName: "hw5-gate")], + RawTags = + [ + new RawTagEntry(Ref, $$"""{"tagPath":"{{tagPath}}","dataType":"String","writable":true}""", + WriteIdempotent: false, DeviceName: gateway), + ], + Timeout = TimeSpan.FromSeconds(5), + Transport = AbCipTransport.Native, + Probe = new AbCipProbeOptions { Enabled = false }, + EnableControllerBrowse = false, + EnableAlarmProjection = false, + }, "hw5-string-gate"); + try + { + await drv.InitializeAsync("{}", ct); + Log($"Gateway {gateway}; tag '{tagPath}'; transport Native; snapshot gap {gap} s"); + + var (v1, raw1) = await SnapshotAsync(drv, Ref, replies, "snapshot 1", ct); + await Task.Delay(TimeSpan.FromSeconds(gap), ct); + var (v2, raw2) = await SnapshotAsync(drv, Ref, replies, "snapshot 2", ct); + + // Preflight — fail closed before any write. + raw1.ShouldBe(raw2, "the tag changed between the two snapshots: something else writes it; NOT writing"); + raw1.Length.ShouldBe(4 + BuiltInStringBytes, "expected a built-in STRING (type 0x02A0 + handle + 88 bytes)"); + var data = raw1.AsSpan(4); + var len = BitConverter.ToInt32(data[..4]); + len.ShouldBeInRange(0, StringCapacity); + data[(4 + len)..].ToArray().ShouldAllBe(b => b == 0, + "bytes past LEN are not zero, so the driver's zero-filling encoder could not restore them exactly; NOT writing"); + Log($"Preflight OK: type/handle {Convert.ToHexString(raw1.AsSpan(0, 4))}, LEN {len}, tail zero; original '{v1}'"); + if (Environment.GetEnvironmentVariable("AB_HW_WRITE_DRYRUN") == "1") + { + Log("AB_HW_WRITE_DRYRUN=1: stopping after the read-only preflight; nothing written."); + return; + } + + var restored = false; + byte[] rawTest; + try + { + await WriteAsync(drv, Ref, TestValue, "write test value", ct); + (var vTest, rawTest) = await SnapshotAsync(drv, Ref, replies, "read back test value", ct); + vTest.ShouldBe(TestValue); + await WriteAsync(drv, Ref, v1, "restore original", ct); + restored = true; + } + finally + { + // Whatever failed after the first write, the original goes back (own token: never skip the restore). + if (!restored) await WriteAsync(drv, Ref, v1, "restore original (after a failure)", CancellationToken.None); + } + var (vRestored, rawRestored) = await SnapshotAsync(drv, Ref, replies, "read back restored", ct); + vRestored.ShouldBe(v1); + rawRestored.ShouldBe(raw1, "restored bytes must equal the original byte for byte"); + Log($"Test-value raw LEN {BitConverter.ToInt32(rawTest.AsSpan(4, 4))}; restored raw equals original: {rawRestored.SequenceEqual(raw1)}"); + + await Task.Delay(TimeSpan.FromSeconds(2), ct); + var (vFinal, rawFinal) = await SnapshotAsync(drv, Ref, replies, "final snapshot", ct); + rawFinal.ShouldBe(raw1); + vFinal.ShouldBe(v1); + Log("RESULT: write, read-back, exact restore — PASS"); + } + finally + { + diagnostics.ExchangeObserver = null; + await drv.ShutdownAsync(CancellationToken.None); + } + } + + private static async Task<(string Value, byte[] Raw)> SnapshotAsync( + AbCipDriver drv, string reference, List replies, string label, CancellationToken ct) + { + lock (replies) replies.Clear(); + var snap = (await drv.ReadAsync([reference], ct)).Single(); + byte[] reply; + lock (replies) + { + // The first read of a fresh runtime is two wire reads (the create/existence read, then the read itself). + replies.Count.ShouldBeInRange(1, 2, $"{label}: Read Tag Fragmented replies on the wire"); + replies.ShouldAllBe(r => r.SequenceEqual(replies[0]), $"{label}: the wire reads of one call disagree"); + reply = replies[^1]; + } + reply[2].ShouldBe((byte)0, $"{label}: CIP general status"); + var raw = reply.AsSpan(4).ToArray(); // type (2) + handle (2) + data + Log($"{label}: status 0x{snap.StatusCode:X8}, value '{snap.Value}', raw ({raw.Length} B) {Convert.ToHexString(raw)}"); + snap.StatusCode.ShouldBe(0u, label); + return ((string)snap.Value!, raw); + } + + private static async Task WriteAsync(AbCipDriver drv, string reference, string value, string label, CancellationToken ct) + { + var result = (await drv.WriteAsync([new WriteRequest(reference, value)], ct)).Single(); + Log($"{label}: '{value}' (ASCII {Convert.ToHexString(Encoding.ASCII.GetBytes(value))}) -> status 0x{result.StatusCode:X8}"); + result.StatusCode.ShouldBe(0u, label); + } + + private static void Log(string line) => TestContext.Current.TestOutputHelper?.WriteLine(line); +} From c36224d95653201f69c368eda5d2a4c15e5ca05c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 9 Oct 2026 06:21:55 -0400 Subject: [PATCH 2/3] =?UTF-8?q?test(abcip):=20hw5=20review=20fixes=20?= =?UTF-8?q?=E2=80=94=20HardwareGate=20guard,=20(host,=20tag)=20allow-list,?= =?UTF-8?q?=20differential-run=20refusal=20(#592)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - HardwareGateGuardTests: only StringWriteGateTests may name a write path in HardwareGate/; the allow-list is exactly the two Z-35162 (10.209.70.160) STRING tags; the opt-in, the (host, tag) check and the AB_DIFF_ENDPOINT refusal are present. Proven red by a WriteAsync mutation. - StringWriteGateTests: allow-list entries are (host, tag); any other endpoint is refused; skips while AB_DIFF_ENDPOINT is set, like IntegrationWriteGate. - AbCip.md #566: the close happened somewhere in the 16-60 s window; RST vs FIN not captured. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- docs/drivers/AbCip.md | 4 +- .../HardwareGate/HardwareGateGuardTests.cs | 70 +++++++++++++++++++ .../HardwareGate/StringWriteGateTests.cs | 37 +++++++--- 3 files changed, 102 insertions(+), 9 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/HardwareGateGuardTests.cs diff --git a/docs/drivers/AbCip.md b/docs/drivers/AbCip.md index 5c7b8576..23c68e35 100644 --- a/docs/drivers/AbCip.md +++ b/docs/drivers/AbCip.md @@ -180,7 +180,9 @@ The driver has two interchangeable wire transports, selected by one `DriverConfi `BadCommunicationError` (transport `BadConnection`); a keep-alive that gets it recycles the connection before any request fails. Every other status — tag-level errors, other `0x01` extended codes — is left to the request. **Observed on a real CompactLogix (Z-35162, hw5, 2026-10-09): neither code.** A connection left - idle past its 16 s timeout (keep-alive disabled for the test) was closed at the TCP level; the next request + idle 60 s, past its 16 s timeout (keep-alive disabled for the test), was closed at the TCP level somewhere in + that 16–60 s window — the exact moment, and whether the controller sent RST or FIN, were not captured (no + packet capture; EPIPE on the first write only says the close was complete by then); the next request failed on its write in 9 ms (`Broken pipe` → `BadConnection` → `BadCommunicationError`, no CIP or encapsulation status) and the request after it reopened with a fresh Forward Open. That path needs no CIP check — a faulted session always reopens on the next send — and is pinned offline by diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/HardwareGateGuardTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/HardwareGateGuardTests.cs new file mode 100644 index 00000000..ec8c0506 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/HardwareGateGuardTests.cs @@ -0,0 +1,70 @@ +using System.Runtime.CompilerServices; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.HardwareGate; + +/// +/// Offline guard for the hardware-gate folder — deliberately NOT env-gated, so it runs in every build. The folder sits +/// outside the read-only Differential guard because one test in it writes; this pins how far that goes: +/// only may name a write path, its allow-list is exactly the two approved +/// (Z-35162, tag) pairs, and its explicit opt-in and differential-run refusal are present. +/// +public sealed class HardwareGateGuardTests +{ + [Fact] + public void Only_the_string_write_gate_names_a_write_path() + { + // Built by concatenation so this file's own text never matches itself. + string[] forbidden = + [ + "Write" + "Async", "Encode" + "Value", "WriteBit" + "InDInt", "Write" + "Tag", "Write" + "Request", "AbCip" + "Driver", + ]; + var sources = Directory.GetFiles(Path.GetDirectoryName(ThisFile())!, "*.cs"); + sources.ShouldContain(f => Path.GetFileName(f) == nameof(StringWriteGateTests) + ".cs"); + sources.ShouldContain(f => Path.GetFileName(f) == nameof(IdleConnectionEvidenceTests) + ".cs"); + + var hits = (from f in sources + where Path.GetFileName(f) != nameof(StringWriteGateTests) + ".cs" + let text = File.ReadAllText(f) + from token in forbidden + where text.Contains(token, StringComparison.Ordinal) + select $"{Path.GetFileName(f)} names {token}").ToList(); + hits.ShouldBeEmpty(); + + // Positive control: the scan does see the one file that is allowed to write. + var writer = File.ReadAllText(sources.Single(f => Path.GetFileName(f) == nameof(StringWriteGateTests) + ".cs")); + writer.ShouldContain(forbidden[0]); + } + + [Fact] + public void The_write_allow_list_is_exactly_the_two_approved_pairs_on_Z35162() + { + StringWriteGateTests.Approved.ShouldBe( + [ + ("10.209.70.160", "Program:ES03_TemperatureControl._StrTmp"), + ("10.209.70.160", "Program:ES03_TemperatureControl._Prefix"), + ]); + + StringWriteGateTests.IsApproved("ab://10.209.70.160/1,0", "Program:ES03_TemperatureControl._StrTmp").ShouldBeTrue(); + StringWriteGateTests.IsApproved("ab://10.209.70.160:44818/1,0", "Program:ES03_TemperatureControl._Prefix").ShouldBeTrue(); + // Same tag on any other controller, another tag on Z-35162, or a malformed gateway: refused. + StringWriteGateTests.IsApproved("ab://10.209.70.161/1,0", "Program:ES03_TemperatureControl._StrTmp").ShouldBeFalse(); + StringWriteGateTests.IsApproved("ab://10.100.0.35/1,0", "Program:ES03_TemperatureControl._StrTmp").ShouldBeFalse(); + StringWriteGateTests.IsApproved("ab://10.209.70.160/1,0", "test2").ShouldBeFalse(); + StringWriteGateTests.IsApproved("ab://10.209.70.160/1,0", "Program:UN01_Press._Prefix").ShouldBeFalse(); + StringWriteGateTests.IsApproved("10.209.70.160", "Program:ES03_TemperatureControl._StrTmp").ShouldBeFalse(); + } + + [Fact] + public void The_string_write_gate_keeps_its_opt_in_and_refuses_during_a_differential_run() + { + var text = File.ReadAllText(Path.Combine(Path.GetDirectoryName(ThisFile())!, nameof(StringWriteGateTests) + ".cs")); + text.ShouldContain("""GetEnvironmentVariable("AB_HW_WRITE_APPROVED") != "1") Assert.Skip("""); + text.ShouldContain("GetEnvironmentVariable(IntegrationWriteGate.DifferentialEndpointVar) is { Length: > 0 })"); + text.ShouldContain("if (!IsApproved(gateway, tagPath))"); + IntegrationWriteGate.DifferentialEndpointVar.ShouldBe("AB_DIFF_ENDPOINT"); + } + + private static string ThisFile([CallerFilePath] string path = "") => path; +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/StringWriteGateTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/StringWriteGateTests.cs index e6eaa4c7..05822f66 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/StringWriteGateTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/StringWriteGateTests.cs @@ -15,24 +15,43 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.HardwareGate; /// restored byte-exact). /// /// AB_HW_WRITE_GATEWAY — ab://host/1,0; absent ⇒ skip. -/// AB_HW_WRITE_STRING_TAG — must be one of (the brief's allow-list). +/// AB_HW_WRITE_STRING_TAG — with the gateway's host, must be a pair on . /// AB_HW_WRITE_APPROVED=1 — the explicit opt-in. /// AB_HW_WRITE_GAP_SECONDS — snapshot gap, default 30, at least 30. /// AB_HW_WRITE_DRYRUN=1 — stop after the read-only preflight (no write). /// -/// Never set by any other run; (the simulator opt-in) is deliberately not +/// Refused (skipped) while AB_DIFF_ENDPOINT is set (a read-only differential run). Never set by any other +/// run; (the simulator opt-in) is deliberately not /// what enables this. /// [Trait("Category", "LiveIntegration")] public sealed class StringWriteGateTests { - /// The only tags this gate may write: logic-, alias-, trend- and Galaxy-unreferenced (L5X census + raw grep). - private static readonly string[] Approved = + /// + /// The only (controller, tag) pairs this gate may write: Z-35162's logic-, alias-, trend- and Galaxy-unreferenced + /// STRINGs (L5X census + raw grep). Pinned to the host, so the same tag name on another controller is refused. + /// pins this list. + /// + internal static readonly (string Host, string Tag)[] Approved = [ - "Program:ES03_TemperatureControl._StrTmp", - "Program:ES03_TemperatureControl._Prefix", + ("10.209.70.160", "Program:ES03_TemperatureControl._StrTmp"), + ("10.209.70.160", "Program:ES03_TemperatureControl._Prefix"), ]; + /// The host of an ab://host[:port]/path gateway, or null when it is not one. + internal static string? GatewayHost(string gateway) + { + if (!gateway.StartsWith("ab://", StringComparison.OrdinalIgnoreCase)) return null; + var authority = gateway[5..].Split('/', 2)[0]; + var host = authority.Split(':', 2)[0]; + return host.Length == 0 ? null : host; + } + + /// True only for an exact (host, tag) pair on . + internal static bool IsApproved(string gateway, string tag) => + GatewayHost(gateway) is { } host + && Approved.Any(a => string.Equals(a.Host, host, StringComparison.Ordinal) && string.Equals(a.Tag, tag, StringComparison.Ordinal)); + private const string TestValue = "OTOPCUA-TEST"; private const int BuiltInStringBytes = 88; private const int StringCapacity = 82; @@ -42,10 +61,12 @@ public sealed class StringWriteGateTests { var gateway = Environment.GetEnvironmentVariable("AB_HW_WRITE_GATEWAY")?.Trim(); if (string.IsNullOrEmpty(gateway)) Assert.Skip("AB_HW_WRITE_GATEWAY not set."); + if (Environment.GetEnvironmentVariable(IntegrationWriteGate.DifferentialEndpointVar) is { Length: > 0 }) + Assert.Skip($"Write gate refused: {IntegrationWriteGate.DifferentialEndpointVar} is set, so a read-only differential run is in progress."); if (Environment.GetEnvironmentVariable("AB_HW_WRITE_APPROVED") != "1") Assert.Skip("AB_HW_WRITE_APPROVED=1 not set."); var tagPath = Environment.GetEnvironmentVariable("AB_HW_WRITE_STRING_TAG")?.Trim() ?? ""; - if (!Approved.Contains(tagPath, StringComparer.Ordinal)) - throw new InvalidOperationException($"AB_HW_WRITE_STRING_TAG='{tagPath}' is not on the approved list."); + if (!IsApproved(gateway, tagPath)) + throw new InvalidOperationException($"('{gateway}', '{tagPath}') is not an approved (controller, tag) pair."); var gap = 30; if (Environment.GetEnvironmentVariable("AB_HW_WRITE_GAP_SECONDS") is { Length: > 0 } rawGap && (!int.TryParse(rawGap, out gap) || gap < 30 || gap > 300)) From a7c27e978f3c91bdd8b07a9750726902c97c5a54 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 9 Oct 2026 06:41:14 -0400 Subject: [PATCH 3/3] docs(abcip): #592 native array-element write on Z35162 (_SEQ_EP[31], user-approved, restored) Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- CLAUDE.md | 2 +- docs/drivers/AbCip.md | 9 +++++---- .../2026-10-07-native-ab-transport-gate-results.md | 13 +++++++++++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 90c8c729..8297dcb7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1087,7 +1087,7 @@ re-emit a concrete default. The defaults differ per driver (final review, 2026-1 because the driver factory AND the admin-node Test Connect probe both read it. ⚠️ So **the upgrade restart alone moves no-key AbCip drivers to Native**; to stay on libplctag, set the node key — NOT a per-driver key through the old AdminUI, whose typed form drops unknown keys (#576). Evidence: AbCip reads proven on a real - CompactLogix (Z-35162); scalar native writes (DINT/BOOL/REAL) proven on it once (2026-10-09, user-approved one-off, restored) — and native UDT member writes (DINT/BIT/REAL, #611 gate, same day) and a native STRING write (hw5, same day, restored byte-exact); array (element), libplctag member and fragmented writes are `ab_server`-only (no logic-unreferenced array exists on Z-35162 — its `_SEQ_EP` arrays are aliased sequencer step registers). #566: a real Logix drops TCP on a connection idled past its timeout (no 0x0107/0x0203); the faulted session reopens on the next request. + CompactLogix (Z-35162); scalar native writes (DINT/BOOL/REAL) proven on it once (2026-10-09, user-approved one-off, restored) — and native UDT member writes (DINT/BIT/REAL, #611 gate, same day) and a native STRING write (hw5, same day, restored byte-exact); and a native array-element write (`EM01_PowerUnit._SEQ_EP[31]`, user-approved, an element no alias targets, restored; neighbours unchanged); libplctag member and fragmented writes are `ab_server`-only. #566: a real Logix drops TCP on a connection idled past its timeout (no 0x0107/0x0203); the faulted session reopens on the next request. - **AbLegacy: an absent key resolves to `Libplctag` for every family; `Native` is opt-in** (no node key). On a real MicroLogix 1400 (Z-33323, idle, 2026-10-09) native read S/N/F/B/T/C/R/ST/O/I incl. `S:0`, T/C/R sub-elements and status bits, and ST byte order, all correct, and wrote N/F8/B-bit (restored). libplctag (the default) diff --git a/docs/drivers/AbCip.md b/docs/drivers/AbCip.md index 23c68e35..889e161f 100644 --- a/docs/drivers/AbCip.md +++ b/docs/drivers/AbCip.md @@ -249,13 +249,14 @@ The driver has two interchangeable wire transports, selected by one `DriverConfi (DINT `test2`, BOOL `Program:UN01_Press.xxtest`, REAL `Program:ES03_EM03_Zone3._ff_test`: write, read back, restore, all Good), and the #611 gate added **native UDT member writes** (DINT, BIT and REAL members of `_EmptyTemperatureSegment`). hw5 (2026-10-09) added a **native STRING write** (`Program:ES03_TemperatureControl._StrTmp`, through the driver: - write, read back, restored byte-exact). Still ab_server-only: array (element) writes — no logic-unreferenced array - exists on Z-35162 (`_SEQ_EP` is aliased into live sequencer step registers) — libplctag member writes, and fragmented - writes. #566 is answered (the controller drops TCP; see "A target-closed CIP connection"). Mixed STRING/array/scalar + write, read back, restored byte-exact). A user-approved **native array-element write** followed: `Program:EM01_PowerUnit._SEQ_EP[31]` + 0→4242→0 (the array's only alias targets `[1]`, no routine indexes or block-copies it; all 32 elements snapshotted + twice before, after the write and after the restore — only `[31]` changed). Still ab_server-only: whole-array + writes (the driver has none), libplctag member writes, and fragmented writes. #566 is answered (the controller drops TCP; see "A target-closed CIP connection"). Mixed STRING/array/scalar Multiple Service Packet reads (#582) are proven on the press (one 5-read MSP, values equal to single reads). Results: [native-ab-transport-gate-results](../plans/2026-10-07-native-ab-transport-gate-results.md). - **libplctag cannot be deleted yet.** Scalar, UDT-member and STRING native writes are proven on a CompactLogix - (2026-10-09); what remains is #567 (fragmented wide-struct writes), array element writes on hardware, and AbLegacy + (2026-10-09); what remains is #567 (fragmented wide-struct writes), whole-array writes on hardware, and AbLegacy native beyond the MicroLogix 1400 evidence. - UDT **member writes** (#611) are proven on a real CompactLogix on the **native** transport only (2026-10-09); the libplctag member-write path is `ab_server`-only. See [Member writes](#member-writes). diff --git a/docs/plans/2026-10-07-native-ab-transport-gate-results.md b/docs/plans/2026-10-07-native-ab-transport-gate-results.md index faf825ab..4b1a65ff 100644 --- a/docs/plans/2026-10-07-native-ab-transport-gate-results.md +++ b/docs/plans/2026-10-07-native-ab-transport-gate-results.md @@ -784,3 +784,16 @@ whole-array write at the driver layer (array tags are read-only, `DriverArrayWri 128-byte array (or this 88-byte STRING) is a single Write Tag: `LogixTagClient.WriteAsync` fragments only when the request exceeds `MaxRequestBytes` (4000 B on the Large Forward Open this controller grants). A fragmented write needs a value of roughly 4 KB. + +### Native array-element write — 2026-10-09 (user-approved, Z-35162) + +`Program:EM01_PowerUnit._SEQ_EP` (DINT[32]). L5X check: its only alias is `_SEQ_EP01_AutoCycle -> _SEQ_EP[1]`; no routine references `_SEQ_EP` directly (no index, COP or FLL); no cross-program reference. AbCip CLI (native) from master `98c00904`, one element per read, two snapshots 30 s apart before writing. `[1]` = -1 is the idle auto-cycle step. + +``` +SNAP1: 0 -1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 +SNAP2: 0 -1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 +WRITE [31]=4242: Write Program:EM01_PowerUnit._SEQ_EP[31]: 0x00000000 (Good) +AFTER: 0 -1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 4242 +RESTORE [31]=0: Write Program:EM01_PowerUnit._SEQ_EP[31]: 0x00000000 (Good) +FINAL: 0 -1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 +```