docs(plans): fold Fable review into the native AB transport plan; add tasks.json
This commit is contained in:
Joseph Doherty
@@ -25,6 +25,17 @@
|
||||
- Forward Close on the last lease release.
|
||||
- **Real presses are touched only in Task 21, read-only, one press at a time.**
|
||||
- **One request in flight per connection.** `EncapsulationClient` serializes requests and `RequestScheduler` has `MaxInFlight = 1` hard-wired. *Deviation from spec §3.1, which allowed a configurable `MaxInFlight`; per YAGNI there is no knob in v1.*
|
||||
- **One `RequestScheduler` per device connection, never per tag.** It lives in the `DeviceSessionPool` entry beside the `CipConnection` and is exposed as `ConnectionLease.Scheduler`. Per-tag schedulers would make MSP coalescing impossible and race the connected sequence counter. That is review finding B1.
|
||||
- **The connected sequence count is assigned inside `EncapsulationClient`'s single-flight gate**, never by a caller (B1).
|
||||
- **Caller cancellation never faults the shared connection.** A poll subscription's CTS is cancelled on every OPC UA unsubscribe (`PollGroupEngine.cs:160`). Faulting the socket on that would drop every tag on the press and force a fresh Forward Open. Instead:
|
||||
- Frames are written uncancellably (they are small and bounded by the socket).
|
||||
- Cancellation during the reply wait abandons only the waiter. The stale reply is skipped by context or sequence on the next exchange.
|
||||
- Only a **timeout**, an I/O error or a malformed frame faults the connection. That is review finding B2.
|
||||
- **Routing is explicit:** `ConnectionOptions.Routing ∈ { Connected, UnconnectedSend, Direct }`.
|
||||
- `Connected` = Forward Open + SendUnitData.
|
||||
- `UnconnectedSend` = SendRRData wrapping 0x52.
|
||||
- `Direct` = SendRRData carrying the CIP request bare, as libplctag does for SLC/MicroLogix/PLC-5 Execute PCCC even with a `1,0` path.
|
||||
- The **captures decide** each family's mode (B3).
|
||||
- **Model tiers:** `mechanical` ⇒ **haiku**, `standard` ⇒ **sonnet**, `frontier` ⇒ **opus**. Each task's metadata carries `model`.
|
||||
- **Parallel execution:** tasks in the same wave touch disjoint files and may run concurrently, **each in its own git worktree** off `feat/native-ab-transport`. They merge back serially in task-id order. Only Task 1 edits `ZB.MOM.WW.OtOpcUa.slnx`. No task edits `Directory.Packages.props`.
|
||||
- **Build gate for every task:** `dotnet build ZB.MOM.WW.OtOpcUa.slnx` with 0 warnings (`TreatWarningsAsErrors` is on for driver projects), and the task's test command green.
|
||||
@@ -44,20 +55,30 @@
|
||||
|
||||
| Wave | Tasks (parallel within a wave) | Notes |
|
||||
|---|---|---|
|
||||
| 0 | T1 | Creates the project, the slnx entries and the shared contracts. Everything depends on it. |
|
||||
| 1 | T2, T3, T4, T5, T6 | T2/T3 are driver seams (disjoint projects); T4 captures golden frames from the docker host; T5/T6 are disjoint transport folders. |
|
||||
| 2 | T7, T8, T11 | T7 needs T4+T6; T8 needs T4; T11 needs T2+T3. |
|
||||
| 3 | T9, T10 | T9 needs T5+T7; T10 needs T7. They live in disjoint files. |
|
||||
| 4 | T12, T13, T14 | T12/T13 are disjoint AbCip files (need T2,T9,T10); T14 is AbLegacy (needs T3,T8,T9,T10). |
|
||||
| 5 | T15, T16 | AbCip switch (needs T12,T13) and AbLegacy switch (needs T14), in disjoint projects. |
|
||||
| 6 | T17, T19 | Integration suites on both transports (needs T15,T16); AdminUI dropdown + `/run` (needs T15,T16). |
|
||||
| 7 | T18, T20 | Differential harness (needs T17: fixes from T17 land in transport code first); docs (needs T19's evidence and the T8/T14 verification status). |
|
||||
| 8 | T21 → T22 | Real-press gate (needs T18), then the default flip (needs T21, T17, T19, T20). Serial. |
|
||||
| 0 | T1 | Creates the project, the slnx entries, the shared contracts and the empty gate-results doc. |
|
||||
| 1 | T2, T3, T4, T6 | T2/T3 are driver seams (disjoint projects); T4 captures golden frames from the docker host; T6 is the path encoders. |
|
||||
| 2 | T5, T7, T8, T11 | T5 needs T4 (capture parity); T7 needs T4+T6; T8 needs T4; T11 needs T2+T3. All disjoint folders. |
|
||||
| 3 | T10 | The scheduler (needs T7). |
|
||||
| 4 | T9 | Connection + pool (needs T5, T7, T10): the pool entry owns the scheduler. |
|
||||
| 5 | T12, T13, T14 | T12/T13 are disjoint AbCip files (need T2, T9); T14 is AbLegacy (needs T3, T8, T9). |
|
||||
| 6 | T15, T16 | AbCip switch (needs T12, T13) and AbLegacy switch (needs T14), in disjoint projects. |
|
||||
| 7 | T17 | Integration suites on both transports (needs T15, T16). It appends to the gate-results doc. |
|
||||
| 8 | T18, T19 | Differential harness (needs T17); AdminUI + `/run` (needs T17, since both append to the gate-results doc, so they are serialized behind T17; T18 and T19 append to **different headed sections** created by T1, which merge cleanly). |
|
||||
| 9 | T20 | Docs (needs T19's evidence and the T8/T14 verification status). |
|
||||
| 10 | T21 → T22 | Real-press gate (needs T18), then the default flip (needs T21, T17, T19, T20). Serial. |
|
||||
|
||||
Dependency list (authoritative; mirrors `.tasks.json` `blockedBy`):
|
||||
T2,T3,T4,T5,T6 ← T1 · T7 ← T4,T6 · T8 ← T4 · T11 ← T2,T3 · T9 ← T5,T7 · T10 ← T7 ·
|
||||
T12,T13 ← T2,T9,T10 · T14 ← T3,T8,T9,T10 · T15 ← T12,T13 · T16 ← T14 · T17,T19 ← T15,T16 ·
|
||||
T18 ← T17 · T20 ← T19 · T21 ← T18 · T22 ← T21,T17,T19,T20
|
||||
T2,T3,T4,T6 ← T1 · T5 ← T1,T4 · T7 ← T4,T6 · T8 ← T4 · T11 ← T2,T3 · T10 ← T7 · T9 ← T5,T7,T10 ·
|
||||
T12,T13 ← T2,T9 · T14 ← T3,T8,T9 · T15 ← T12,T13 · T16 ← T14 · T17 ← T15,T16 ·
|
||||
T18 ← T17 · T19 ← T17 · T20 ← T19 · T21 ← T18 · T22 ← T21,T17,T19,T20
|
||||
|
||||
**Model per task:**
|
||||
|
||||
| Tier | Model | Tasks |
|
||||
|---|---|---|
|
||||
| frontier | opus | T4, T9, T10, T12, T18, T21 |
|
||||
| standard | sonnet | T1, T2, T3, T5, T6, T7, T8, T13, T14, T15, T16, T17, T19, T20 |
|
||||
| mechanical | haiku | T11, T22 |
|
||||
|
||||
---
|
||||
|
||||
@@ -77,13 +98,27 @@ T18 ← T17 · T20 ← T19 · T21 ← T18 · T22 ← T21,T17,T19,T20
|
||||
- Create: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests.csproj`
|
||||
- Create: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/AbStatusCodesTests.cs`
|
||||
- Create: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Wire/CipReplyTests.cs`
|
||||
- Create: `docs/plans/2026-10-07-native-ab-transport-gate-results.md` (skeleton with five empty headed sections, `## ab_server suites (T17)`, `## Differential vs ab_server (T18)`, `## AdminUI /run (T19)`, `## Real press (T21)` and `## Latency (T21)`, so later tasks append to separate sections and merge cleanly)
|
||||
- Modify: `ZB.MOM.WW.OtOpcUa.slnx` (add both projects next to the AbCip entries, lines 41-44 and 112-115)
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/ZB.MOM.WW.OtOpcUa.Driver.AbCip.csproj` and `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.csproj` (add a `ProjectReference` to Ab.Transport)
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- [ ] `dotnet build ZB.MOM.WW.OtOpcUa.slnx` succeeds with 0 warnings and both new projects appear in the slnx.
|
||||
- [ ] `AbStatus.Ok == 0`, and every error member is negative.
|
||||
- [ ] `AbStatusCodes.FromCip` maps CIP general status `0x00→Ok`, `0x04/0x05/0x16→NotFound`, `0x06→Partial`, `0x08→Unsupported`, `0x0F→NotAllowed`, `0x10→DeviceStateConflict`, `0x13→TooSmall`, `0x15→TooLarge`, `0xFF+0x2105→OutOfBounds`, `0xFF+0x2107→TypeMismatch`, anything else → `BadReply`.
|
||||
- [ ] `AbStatusCodes.FromCip` maps CIP general status as follows. It is aligned with the existing `AbCipStatusMapper.MapCipGeneralStatus`, so that existing mapping does not regress (review finding 13).
|
||||
- `0x00`/`0x0B` → `Ok`
|
||||
- `0x04`/`0x05`/`0x16` → `NotFound`
|
||||
- `0x06` → `Partial`
|
||||
- `0x08` → `Unsupported`
|
||||
- `0x0A` → `OutOfBounds`
|
||||
- `0x0C`/`0x10` → `DeviceStateConflict`
|
||||
- `0x0E`/`0x0F` → `NotAllowed`
|
||||
- `0x13` → `TooSmall`
|
||||
- `0x15` → `TooLarge`
|
||||
- `0xFF` + `0x2105` → `OutOfBounds`
|
||||
- `0xFF` + `0x2107` → `TypeMismatch`
|
||||
- everything else, including `0x1E`, → `BadReply`
|
||||
`0x1E` (embedded service error) is only meaningful on an MSP envelope, and the scheduler (T10) handles it before calling `FromCip`.
|
||||
- [ ] `AbStatusCodes.FromPccc` maps STS `0x00→Ok`, `0x10→Unsupported`, `0x20→NotFound`, `0x30→NotAllowed`, `0x40/0x50→DeviceStateConflict`, and `0xF0` maps via EXT STS (`0x06/0x07→NotFound` address problems, else `BadReply`).
|
||||
- [ ] `CipReply.Parse` round-trips a reply with 0, 1 and 2 extended-status words.
|
||||
- [ ] The driver projects reference Ab.Transport, and Ab.Transport references no OtOpcUa project.
|
||||
@@ -139,6 +174,11 @@ public sealed class AbStatusCodesTests
|
||||
[InlineData((byte)0x10, AbStatus.DeviceStateConflict)]
|
||||
[InlineData((byte)0x13, AbStatus.TooSmall)]
|
||||
[InlineData((byte)0x15, AbStatus.TooLarge)]
|
||||
[InlineData((byte)0x0B, AbStatus.Ok)]
|
||||
[InlineData((byte)0x0A, AbStatus.OutOfBounds)]
|
||||
[InlineData((byte)0x0C, AbStatus.DeviceStateConflict)]
|
||||
[InlineData((byte)0x0E, AbStatus.NotAllowed)]
|
||||
[InlineData((byte)0x1E, AbStatus.BadReply)]
|
||||
[InlineData((byte)0x01, AbStatus.BadReply)]
|
||||
public void Cip_general_status_maps(byte general, AbStatus expected) =>
|
||||
AbStatusCodes.FromCip(general, ReadOnlySpan<ushort>.Empty).ShouldBe(expected);
|
||||
@@ -262,12 +302,13 @@ public static class AbStatusCodes
|
||||
{
|
||||
public static AbStatus FromCip(byte general, ReadOnlySpan<ushort> extended) => general switch
|
||||
{
|
||||
0x00 => AbStatus.Ok,
|
||||
0x00 or 0x0B => AbStatus.Ok,
|
||||
0x04 or 0x05 or 0x16 => AbStatus.NotFound,
|
||||
0x06 => AbStatus.Partial,
|
||||
0x08 => AbStatus.Unsupported,
|
||||
0x0F => AbStatus.NotAllowed,
|
||||
0x10 => AbStatus.DeviceStateConflict,
|
||||
0x0A => AbStatus.OutOfBounds,
|
||||
0x0C or 0x10 => AbStatus.DeviceStateConflict,
|
||||
0x0E or 0x0F => AbStatus.NotAllowed,
|
||||
0x13 => AbStatus.TooSmall,
|
||||
0x15 => AbStatus.TooLarge,
|
||||
0xFF when extended.Length > 0 && extended[0] == 0x2105 => AbStatus.OutOfBounds,
|
||||
@@ -401,7 +442,7 @@ Add the two `<Project Path=...>` lines to the slnx. Add `<ProjectReference Inclu
|
||||
- [ ] **Step 6: Commit.**
|
||||
|
||||
```bash
|
||||
git add src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests ZB.MOM.WW.OtOpcUa.slnx src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/ZB.MOM.WW.OtOpcUa.Driver.AbCip.csproj src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.csproj
|
||||
git add docs/plans/2026-10-07-native-ab-transport-gate-results.md src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests ZB.MOM.WW.OtOpcUa.slnx src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/ZB.MOM.WW.OtOpcUa.Driver.AbCip.csproj src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.csproj
|
||||
git commit -m "feat(ab-transport): project skeleton with AbStatus, CIP reply parsing and the ICipChannel seam"
|
||||
```
|
||||
|
||||
@@ -417,7 +458,8 @@ git commit -m "feat(ab-transport): project skeleton with AbStatus, CIP reply par
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipStatusMapper.cs` (replace `MapLibplctagStatus(Status)` / `TryGetStatus(LibPlcTagException)` with `MapStatus(AbStatus)`, `MapStatus(int)`, and `MapException`)
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs` (`IsReachableStatus(AbStatus)`, `IsReachableException(AbTransportException)`; construct runtimes via an `IAbCipTagFactory` parameter that defaults to `new LibplctagTagFactory()`)
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs:1,1186-1191` (catch `AbTransportException`)
|
||||
- Modify (tests): `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverWriteTests.cs`, `AbCipDriverProbeTests.cs`, `AbCipDriverReadTests.cs`, `AbCipStatusMapperTests.cs`, `AbCipDriverCodeReviewRegressionTests.cs`, `AbCipUnknownTagReadTests.cs`, `FakeAbCipTag.cs`
|
||||
- Modify (tests): `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverWriteTests.cs`, `AbCipDriverProbeTests.cs`, `AbCipDriverReadTests.cs`, `AbCipStatusMapperTests.cs`, `AbCipDriverCodeReviewRegressionTests.cs` (line 326 uses `libplctag.Status.ErrorBadConnection`), `AbCipUnknownTagReadTests.cs`, `AbCipLoggingTests.cs` (line 91, same), `FakeAbCipTag.cs`. Run `grep -rln "libplctag" tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests --include=*.cs` and migrate **every** hit except `LibplctagStatusTranslationTests.cs` and the mapper parity oracle.
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs` (its `<see cref="AbCipStatusMapper.MapLibplctagStatus(int)"/>` must become `MapStatus(int)`. A dangling cref is CS1574, which `TreatWarningsAsErrors` turns into a build error.)
|
||||
- Create: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/LibplctagStatusTranslationTests.cs`
|
||||
|
||||
**Acceptance Criteria:**
|
||||
@@ -601,7 +643,7 @@ git commit -m "refactor(abcip): confine libplctag to its adapters behind AbStatu
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyStatusMapper.cs` (`MapStatus(AbStatus|int)`, `MapException`; keep `MapPcccStatus`)
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverProbe.cs` (`IsReachableStatus(AbStatus)`, catch `AbTransportException`, optional `IAbLegacyTagFactory` parameter defaulting to `new LibplctagLegacyTagFactory()`)
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs` (lines ~318-325: `MapLibplctagStatus`→`MapStatus`; log text "transport status"; any `LibPlcTagException` catch → `AbTransportException`)
|
||||
- Modify (tests): `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyEvictOnFailureTests.cs`, `AbLegacyReadWriteTests.cs`, `AbLegacyHostAndStatusTests.cs`, `FakeAbLegacyTag.cs`
|
||||
- Modify (tests): `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyEvictOnFailureTests.cs`, `AbLegacyReadWriteTests.cs` (line 73), `AbLegacyHostAndStatusTests.cs`, `AbLegacyBitRmwTests.cs` (line 232), `AbLegacyArrayTests.cs` (line 115), `FakeAbLegacyTag.cs`. Run `grep -rln "libplctag" tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests --include=*.cs` and migrate every hit except the translation test and the parity oracle. Also grep `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy` for `cref=".*MapLibplctagStatus` and fix any match (CS1574).
|
||||
- Create: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/LibplctagLegacyStatusTranslationTests.cs`
|
||||
|
||||
**Acceptance Criteria:**
|
||||
@@ -678,6 +720,7 @@ git commit -m "refactor(ablegacy): confine libplctag to its adapter behind AbSta
|
||||
- `slc500-read-n7.hex`, `slc500-write-n7.hex`, `slc500-read-f8.hex`, `micrologix-read-n7.hex`, `plc5-read-n7.hex`
|
||||
- [ ] `CaptureFileTests` loads every `.hex` file, parses every frame header, and asserts that each frame's encapsulation length field equals its data length.
|
||||
- [ ] `Captures/README.md` records, for each file: the libplctag version (1.5.2), the ab_server image (`otopcua-ab-server:libplctag-release`), the compose profile, and the libplctag tag string used.
|
||||
- [ ] `Captures/README.md` has a **"Routing observed"** table, one row per family (controllogix, micro800, slc500, micrologix, plc5, plus logixpccc if reachable), with the columns: ForwardOpen yes/no · CIP request wrapped in Unconnected Send yes/no · route bytes · requestor-id bytes (PCCC). Tasks 9, 12 and 14 read their `Routing` choice from this table.
|
||||
|
||||
**Verify:** `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests --filter CaptureFile` → pass, and `ls tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Captures/*.hex | wc -l` → ≥ 17.
|
||||
|
||||
@@ -695,7 +738,7 @@ If `/opt/otopcua-abcip` is missing or stale, rsync first. Read the fixture's `do
|
||||
rsync -av tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Docker/ dohertj2@10.100.0.35:/opt/otopcua-abcip/
|
||||
```
|
||||
|
||||
For AbLegacy, use `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/Docker/` with the profiles `slc500`, `micrologix` and `plc5`. Check its README for the deployed path.
|
||||
For AbLegacy, the stack lives at **`~/otopcua-ablegacy`** on the docker host, not `/opt` (`infra/README.md:41,68`). It is driven by `AB_LEGACY_COMPOSE_PROFILE=<slc500|micrologix|plc5> docker compose up -d`, and its source is `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/Docker/`. **Read that compose file for the published ports**; do not assume `:44818`. For containers that publish other ports, or run concurrently, capture inside the container's network namespace with the netshoot form in Step 2.
|
||||
|
||||
- [ ] **Step 2: Capture server-side** while driving libplctag from the Mac with the existing CLIs (`otopcua-abcip-cli` / `otopcua-ablegacy-cli`; run `dotnet run --project src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli -- --help` for the verbs):
|
||||
|
||||
@@ -782,12 +825,20 @@ git commit -m "test(ab-transport): golden libplctag<->ab_server frame captures f
|
||||
- [ ] `EncapsulationCodec.SendUnitData(session, connId, seq, cip)` produces: command `0x0070`; data = interface handle `00000000`, timeout `00 00`, item count `02 00`, Connected Address item `A1 00 04 00 <connId u32>`, Connected Data item `B1 00 <len+2 u16> <seq u16>` followed by the cip bytes.
|
||||
- [ ] `EncapsulationCodec.ParseReply` returns the header and the CPF item payload. For a connected reply it strips the sequence count. It throws `AbTransportException(BadReply)` when encapsulation status ≠ 0, the length is wrong, or the expected item is missing.
|
||||
- [ ] `EncapsulationClient`:
|
||||
- It connects, registers (and stores the session handle from the reply), and `UnRegisterSession`s on `DisposeAsync`.
|
||||
- It serializes requests with a `SemaphoreSlim(1,1)`.
|
||||
- Each request has a unique 8-byte sender context; a reply whose context does not match is discarded and the client keeps reading.
|
||||
- On timeout it throws `AbTransportException(Timeout)` and **closes the socket**, because an out-of-sync stream cannot be reused.
|
||||
- A socket error throws `AbTransportException(BadConnection)`.
|
||||
- `IsConnected` turns false after any transport fault.
|
||||
- **Lifecycle.** It connects, registers (and stores the session handle from the reply), and `UnRegisterSession`s on `DisposeAsync`.
|
||||
- **Single flight.** It serializes requests with a `SemaphoreSlim(1,1)`.
|
||||
- **Connected sequence (B1).** `SendUnitDataAsync(otConnectionId, cip, timeout, ct)` takes **no sequence argument**. The client owns the connected sequence counter and increments it **inside the gate**, wrapping 65535 → 1.
|
||||
- **Matching replies.**
|
||||
- Unconnected replies match on the 8-byte sender context.
|
||||
- Connected (0x70) replies match on the echoed sequence count.
|
||||
- Any frame that doesn't match (an earlier abandoned request's late reply) is **skipped** and the client keeps reading. It is not treated as an error.
|
||||
- **Caller cancellation (B2).**
|
||||
- The frame write uses `CancellationToken.None`, so a write is never half-sent.
|
||||
- If the *caller's* token is cancelled during the reply wait, the call throws `OperationCanceledException`. The connection is **not** faulted: `IsConnected` stays true, the gate is released, and the late reply is skipped by the next exchange.
|
||||
- **Faults.**
|
||||
- The *timeout* elapsing (no reply within `timeout`) throws `AbTransportException(Timeout)` and **faults** the connection by closing the socket. A target that has stopped answering cannot be trusted.
|
||||
- A socket or I/O error throws `AbTransportException(BadConnection)` and faults the connection.
|
||||
- `IsConnected` turns false after any fault.
|
||||
- [ ] Parity: the bytes `RegisterSession` sends, apart from the sender context, equal the first `>` frame of `Captures/logix-register-session.hex`.
|
||||
- [ ] `FakeEipServer` accepts a script of `Func<byte[] request, byte[]? reply>` steps, records every request it receives, and can be told to stall (to test timeouts) or drop the connection.
|
||||
|
||||
@@ -1044,6 +1095,11 @@ Client tests to write:
|
||||
- `SendRRData_returns_cip_payload`: the handler replies `RRReply(req, [0xCC,0,0,0,0xC4,0,1,0,0,0])`, and the result equals those 10 bytes.
|
||||
- `Concurrent_sends_are_serialized`: fire 5 `SendRRDataAsync` calls concurrently, with a handler that sleeps 20 ms and records the arrival time. Assert the received frames never overlap; each request arrives only after the previous reply was sent.
|
||||
- `Stalled_reply_times_out_and_closes_the_socket`: set `Stall = true` and use a 200 ms timeout. Assert `AbTransportException(Timeout)` and `IsConnected == false`.
|
||||
- `Caller_cancellation_does_not_fault_the_connection` (B2):
|
||||
- Have the handler delay its reply by 300 ms, and cancel the caller's token at 50 ms.
|
||||
- Assert `OperationCanceledException` and `IsConnected == true`.
|
||||
- Then send a second request whose handler answers immediately. Assert it returns **its own** payload: the stale reply arrives first and is skipped.
|
||||
- `Connected_sequence_is_assigned_inside_the_gate`: fire 20 concurrent `SendUnitDataAsync` calls, with a handler that echoes the sequence. Assert the sequences seen on the wire are strictly increasing in arrival order, with no duplicates.
|
||||
- `Reply_with_foreign_context_is_skipped`: the handler first writes a reply carrying a different context, then the correct one. The call returns the correct payload.
|
||||
- `Dispose_sends_UnRegisterSession`: after `DisposeAsync`, `Received` contains a 0x66 frame.
|
||||
|
||||
@@ -1060,6 +1116,7 @@ public sealed class EncapsulationClient : IAsyncDisposable
|
||||
private readonly NetworkStream _stream;
|
||||
private readonly SemaphoreSlim _gate = new(1, 1);
|
||||
private long _ctx;
|
||||
private ushort _seq; // connected sequence count; only touched under _gate
|
||||
private volatile bool _faulted;
|
||||
|
||||
public uint SessionHandle { get; private set; }
|
||||
@@ -1091,45 +1148,56 @@ public sealed class EncapsulationClient : IAsyncDisposable
|
||||
return EncapsulationCodec.ParseReply(frame, expectConnected: false).Cip;
|
||||
}
|
||||
|
||||
public async Task<ReadOnlyMemory<byte>> SendUnitDataAsync(uint otConnectionId, ushort sequence,
|
||||
ReadOnlyMemory<byte> cip, TimeSpan timeout, CancellationToken ct)
|
||||
public async Task<ReadOnlyMemory<byte>> SendUnitDataAsync(uint otConnectionId, ReadOnlyMemory<byte> cip,
|
||||
TimeSpan timeout, CancellationToken ct)
|
||||
{
|
||||
var frame = await ExchangeAsync(ctx => EncapsulationCodec.SendUnitData(SessionHandle, ctx, otConnectionId, sequence, cip.Span), timeout, ct).ConfigureAwait(false);
|
||||
var (_, payload, seq) = EncapsulationCodec.ParseReply(frame, expectConnected: true);
|
||||
if (seq != sequence)
|
||||
throw new AbTransportException(AbStatus.BadReply, $"Connected reply sequence {seq} != request {sequence}.");
|
||||
return payload;
|
||||
// The sequence is chosen inside ExchangeAsync's gate (build runs under it), so concurrent
|
||||
// callers can never put sequences on the wire out of order (review B1).
|
||||
var frame = await ExchangeAsync(ctx =>
|
||||
{
|
||||
_seq = _seq == ushort.MaxValue ? (ushort)1 : (ushort)(_seq + 1);
|
||||
return EncapsulationCodec.SendUnitData(SessionHandle, ctx, otConnectionId, _seq, cip.Span);
|
||||
}, timeout, ct).ConfigureAwait(false);
|
||||
return EncapsulationCodec.ParseReply(frame, expectConnected: true).Cip;
|
||||
}
|
||||
|
||||
/// <summary>Write one frame and read frames until one carries our sender context. Connected
|
||||
/// (0x70) replies are matched on command alone: their sender context is not echoed reliably,
|
||||
/// and the sequence check in SendUnitDataAsync replaces it.</summary>
|
||||
/// <summary>Write one frame, then read frames until the matching reply arrives. Unconnected
|
||||
/// replies match on sender context; connected (0x70) replies match on the echoed sequence.
|
||||
/// Non-matching frames are late replies to abandoned requests and are skipped.
|
||||
/// Caller cancellation abandons the wait WITHOUT faulting the stream (review B2); only the
|
||||
/// timeout, an I/O error or a malformed frame faults it.</summary>
|
||||
private async Task<ReadOnlyMemory<byte>> ExchangeAsync(Func<ulong, byte[]> build, TimeSpan timeout, CancellationToken ct)
|
||||
{
|
||||
if (_faulted) throw new AbTransportException(AbStatus.BadConnection, "Session is faulted; reconnect.");
|
||||
await _gate.WaitAsync(ct).ConfigureAwait(false);
|
||||
using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct);
|
||||
cts.CancelAfter(timeout);
|
||||
using var timeoutCts = new CancellationTokenSource(timeout);
|
||||
using var linked = CancellationTokenSource.CreateLinkedTokenSource(ct, timeoutCts.Token);
|
||||
try
|
||||
{
|
||||
var ctx = (ulong)Interlocked.Increment(ref _ctx);
|
||||
var request = build(ctx);
|
||||
var cmd = (ushort)(request[0] | (request[1] << 8));
|
||||
await _stream.WriteAsync(request, cts.Token).ConfigureAwait(false);
|
||||
var expectSeq = cmd == EncapCommand.SendUnitData ? _seq : (ushort)0; // set by build() above, under the gate
|
||||
await _stream.WriteAsync(request, CancellationToken.None).ConfigureAwait(false);
|
||||
while (true)
|
||||
{
|
||||
var frame = await ReadFrameAsync(cts.Token).ConfigureAwait(false);
|
||||
var frame = await ReadFrameAsync(linked.Token).ConfigureAwait(false);
|
||||
var h = EncapsulationCodec.ReadHeader(frame.Span);
|
||||
if (h.Command != cmd) continue;
|
||||
if (cmd != EncapCommand.SendUnitData && h.SenderContext != ctx) continue;
|
||||
if (cmd == EncapCommand.SendUnitData)
|
||||
{
|
||||
if (EncapsulationCodec.ParseReply(frame, expectConnected: true).Sequence != expectSeq) continue;
|
||||
}
|
||||
else if (h.SenderContext != ctx) continue;
|
||||
return frame;
|
||||
}
|
||||
}
|
||||
catch (OperationCanceledException) when (!ct.IsCancellationRequested)
|
||||
catch (OperationCanceledException) when (timeoutCts.IsCancellationRequested && !ct.IsCancellationRequested)
|
||||
{ Fault(); throw new AbTransportException(AbStatus.Timeout, $"No reply within {timeout.TotalMilliseconds:F0} ms."); }
|
||||
catch (Exception ex) when (ex is IOException or SocketException or ObjectDisposedException)
|
||||
catch (OperationCanceledException) { throw; } // caller cancelled: keep the stream; the late reply is skipped later
|
||||
catch (AbTransportException) { Fault(); throw; }
|
||||
catch (Exception ex) when (ex is IOException or SocketException or ObjectDisposedException or EndOfStreamException)
|
||||
{ Fault(); throw new AbTransportException(AbStatus.BadConnection, ex.Message, ex); }
|
||||
catch (OperationCanceledException) { Fault(); throw; }
|
||||
finally { _gate.Release(); }
|
||||
}
|
||||
|
||||
@@ -1164,7 +1232,7 @@ public sealed class EncapsulationClient : IAsyncDisposable
|
||||
}
|
||||
```
|
||||
|
||||
> Note: cancellation by the caller's token *also* faults the client (an in-flight frame was abandoned), so the stream cannot desynchronize. `ConnectionManager` (Task 9) treats a faulted client as "reconnect", through the rate limiter.
|
||||
> Note: a *caller* cancellation leaves the stream intact. The abandoned reply is skipped when it arrives. Only a timeout or an I/O fault closes the socket, and `CipConnection` (Task 9) then reconnects through the Forward Open throttle. This distinction is press safety: unsubscribing an OPC UA monitored item must not cost the press a new Forward Open.
|
||||
|
||||
- [ ] **Step 6: Run.** `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests --filter FullyQualifiedName~Eip` → all pass.
|
||||
- [ ] **Step 7: Commit.** `git add -A src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Eip tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Eip tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Fakes && git commit -m "feat(ab-transport): EtherNet/IP encapsulation codec and single-flight session client"`
|
||||
@@ -1363,7 +1431,8 @@ Fix the `"9108546573744449 4E54"` spacing in the test by stripping spaces (alrea
|
||||
- O→T RPI `u32`, then O→T params: `u16 0x4200 | size` (regular) or `u32 0x42000000 | size` (large);
|
||||
- T→O RPI, then T→O params (same encoding);
|
||||
- transport/trigger `0xA3`, connection path size in words, connection path = `EncodeRoute(cipPath)` + `MessageRouter`.
|
||||
- [ ] `ParseForwardOpenReply` returns `(OTConnectionId, TOConnectionId, OTApiMicros, TOApiMicros)` on success. On general status `0x01` it surfaces the extended status. `0x0100` = connection in use, and `0x0109` / `0x0111` = size or RPI rejected; the caller (Task 9) uses those last two to fall back.
|
||||
- [ ] `ParseForwardOpenReply` returns `(OTConnectionId, TOConnectionId, OTApiMicros, TOApiMicros)` on success. On a refusal it surfaces the general and extended status.
|
||||
- [ ] `ShouldFallBackToRegularOpen(CipReply)` is true when the reply is general `0x08` (Service Not Supported — pre-v20 Logix and other targets answer a Large Forward Open this way, and libplctag falls back on it too) **or** general `0x01` with extended `0x0109` (invalid connection size). Task 9 uses it.
|
||||
- [ ] `ForwardClose(serial, vendor, origSerial, route)` → `4E 02 20 06 24 01 0A 0E <serial> <vendor> <origSerial> <pathWords> 00 <route + MessageRouter>`.
|
||||
- [ ] **MSP.** `MultipleServicePacket.Build(IReadOnlyList<byte[]> requests)` → `0A 02 20 02 24 01`, count `u16`, then offsets `u16[count]` measured from the start of the count field, then the requests concatenated. `Parse(reply, count)` returns one `CipReply` per embedded reply. `BuildSize(sizes)` returns the exact byte size of an MSP for the given request sizes, without allocating (Task 10 uses it for packing).
|
||||
- [ ] **Read/Write Tag:**
|
||||
@@ -1486,7 +1555,9 @@ public static class ConnectionManagerServices
|
||||
}
|
||||
```
|
||||
|
||||
> The large-open fallback needs the raw extended status, so add `public static bool IsSizeRejection(CipReply r) => r.GeneralStatus == 0x01 && r.ExtendedStatus.Length > 0 && r.ExtendedStatus[0] is 0x0109 or 0x0111;`.
|
||||
> The large-open fallback needs the raw statuses, so add:
|
||||
> `public static bool ShouldFallBackToRegularOpen(CipReply r) => r.GeneralStatus == 0x08 || (r.GeneralStatus == 0x01 && r.ExtendedStatus.Length > 0 && r.ExtendedStatus[0] == 0x0109);`
|
||||
> Test both branches, plus a negative case (`0x01`/`0x0100` "connection in use" must NOT fall back).
|
||||
|
||||
```csharp
|
||||
// Logix/LogixServices.cs
|
||||
@@ -1553,6 +1624,7 @@ Implement `MultipleServicePacket`, `SymbolObjectServices` and `TemplateObjectSer
|
||||
- [ ] `PcccServices.ExecutePccc(requestorVendor, requestorSerial, pccc)` → `4B 02 20 67 24 01 07 <vendor u16> <serial u32> <pccc bytes>`.
|
||||
- [ ] `PcccServices.SlcTypedRead(tns, addr, byteCount)` → PCCC `0F 00 <tns u16> A2 <byteCount u8> <file u8*> <type u8> <element u8*> <sub u8*>`, where `u8*` is a single byte when < 255, else `FF lo hi`.
|
||||
- [ ] `PcccServices.SlcTypedWrite(tns, addr, data)` → `0F 00 <tns> AA <byteCount> <file*> <type> <element*> <sub*> <data>`.
|
||||
- [ ] **Byte-count limit (review finding 12).** `byteCount` is a single byte on the wire. `SlcTypedRead`/`SlcTypedWrite` (and the PLC-5 builders) throw `AbTransportException(TooLarge)` when the requested or supplied byte count exceeds `PcccServices.MaxDataBytes = 236`; a silent `(byte)` wrap would be a corrupt write. Test 236 (ok) and 237 (throws) for both. Task 14 chunks larger arrays.
|
||||
- [ ] `PcccServices.Plc5TypedRead(tns, addr, elements)` → `0F 00 <tns u16> 68 <packetOffset u16 0> <totalTrans u16 elements> <PLC-5 system address> <size u16 elements>`.
|
||||
- The system address is a level-mask byte (bit0 = file level present? **derive from `plc5-read-n7.hex`**) followed by level values (`$N7:0` ⇒ data table section, file 7, element 0), each `u8*` encoded.
|
||||
- `Plc5TypedWrite` is `0F 00 <tns> 67 <offset u16> <total u16> <sysaddr> <type/size param> <data>`.
|
||||
@@ -1625,62 +1697,85 @@ public static class PcccServices
|
||||
|
||||
---
|
||||
|
||||
### Task 9: ConnectionManager + DeviceSessionPool (with press-safety bounds)
|
||||
### Task 9: CipConnection + DeviceSessionPool (routing modes, keep-alive, press-safety bounds)
|
||||
|
||||
**Goal:** Own one EIP session and one CIP connection per device key. Implement `ICipChannel` for connected and UCMM-only modes, and hand out ref-counted leases. Rate-limit Forward Opens so that no failure mode can storm a PLC.
|
||||
**Goal:** Own one EIP session, one CIP connection (when routed `Connected`) and **one `RequestScheduler`** per device key. Hand out ref-counted leases exposing both, keep idle class-3 connections alive, and rate-limit Forward Opens so that no failure mode can storm a PLC.
|
||||
|
||||
**Files:**
|
||||
- Create: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/DeviceKey.cs`
|
||||
- Create: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/ConnectionOptions.cs`
|
||||
- Create: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/ConnectionOptions.cs` (includes the `CipRouting` enum)
|
||||
- Create: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/CipConnection.cs` (implements `ICipChannel`)
|
||||
- Create: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/ForwardOpenThrottle.cs`
|
||||
- Create: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/DeviceSessionPool.cs`
|
||||
- Create: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/DeviceSessionPool.cs` (includes `ConnectionLease`)
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.csproj` (`InternalsVisibleTo` for `ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests`, `ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests`, `ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests`)
|
||||
- Create: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/CipConnectionTests.cs`
|
||||
- Create: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/ForwardOpenThrottleTests.cs`
|
||||
- Create: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/DeviceSessionPoolTests.cs`
|
||||
- Modify: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Fakes/FakeEipServer.cs` (add a `FakeCipTarget` helper that answers Forward Open / Close / Unconnected Send / connected data)
|
||||
- Create: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Fakes/FakeCipTarget.cs` (a `FakeEipServer.Handler` that answers Forward Open / Forward Close / Unconnected Send / Direct / connected data, and counts each)
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- [ ] `DeviceKey(string Host, int Port, string CipPath, string PlcAttribute)` is a record with case-insensitive `Host`/`PlcAttribute` equality.
|
||||
- [ ] `ConnectionOptions`: `Timeout`, `ConnectionSize` (int), `UseConnectedMessaging` (bool), `MinForwardOpenInterval` (default 1 s), `RpiMicros` (default 2 000 000), `TimeoutMultiplier` (default 1 ⇒ ×8; connection expiry = RPI × 8 = 16 s), `VendorId` (`0x1337`), `OriginatorSerial` (random per process).
|
||||
- [ ] **Connected mode:**
|
||||
- First use: TCP connect, RegisterSession, Forward Open. When `ConnectionSize > 511` this is a Large FO (0x5B); otherwise a regular one.
|
||||
- If a large open is rejected with `IsSizeRejection` (0x0109 / 0x0111), it retries **once** with a regular Forward Open at `Math.Min(ConnectionSize, 504)`. That retry does not consume the throttle interval, because it is part of the same open attempt.
|
||||
- Requests go via SendUnitData with a monotonically increasing u16 sequence (wrapping at 65535 → 1).
|
||||
- `MaxRequestBytes = negotiatedSize - 2`.
|
||||
- [ ] **UCMM mode** (`UseConnectedMessaging == false`, or an empty route as on Micro800): requests go via SendRRData, wrapped in an Unconnected Send when the route is non-empty and sent directly otherwise. `MaxRequestBytes = 504 - (route.Length > 0 ? 12 + route.Length : 0)`.
|
||||
- [ ] **Reconnect:** when the `EncapsulationClient` is faulted, the next `SendAsync` reconnects through the throttle. The failing call itself throws its original `AbTransportException`.
|
||||
- [ ] **`ForwardOpenThrottle`** (per `DeviceKey`, shared via the pool):
|
||||
- [ ] **`ConnectionOptions`:**
|
||||
- `Routing` (`CipRouting { Connected, UnconnectedSend, Direct }`) and `AllowPacking` (bool).
|
||||
- `Timeout` and `ConnectionSize` (int).
|
||||
- `MinForwardOpenInterval` (default 1 s).
|
||||
- `RpiMicros` (default 2 000 000) and `TimeoutMultiplier` (default 1 ⇒ ×8, so connection expiry = 16 s).
|
||||
- `KeepAliveInterval` (default = RPI × 8 / 2 = 8 s).
|
||||
- `VendorId` (`0x1337`) and `OriginatorSerial` (random per process).
|
||||
- [ ] **Routing semantics (B3):**
|
||||
- `Connected`:
|
||||
- On first use: TCP connect, RegisterSession, Forward Open. When `ConnectionSize > 511` that is a Large FO (0x5B); otherwise a regular one.
|
||||
- If `ShouldFallBackToRegularOpen` (T7) is true for the reply, it retries **once**, in the same throttled attempt, with a regular open at `Math.Min(ConnectionSize, 504)`.
|
||||
- Requests go via `SendUnitDataAsync`. The client assigns the sequence (T5).
|
||||
- `MaxRequestBytes = negotiatedSize - 2`.
|
||||
- `UnconnectedSend`: requests go via SendRRData wrapped in Unconnected Send 0x52 with the route. `MaxRequestBytes = 504 - (12 + route.Length)`.
|
||||
- `Direct`: requests go via SendRRData carrying the CIP request bare, even when a CIP path is configured; the path is ignored. `MaxRequestBytes = 504`.
|
||||
- [ ] **Reconnect:** when the client is faulted (timeout or I/O, never caller cancellation — see T5), the next `SendAsync` reconnects through the throttle. The failing call itself throws its original exception.
|
||||
- [ ] **Keep-alive (review finding 8, spec §9).** In `Connected` mode, a timer sends Get Attribute Single (`0E 03 20 01 24 01 30 01`: Identity, instance 1, attribute 1 = Vendor ID) through the connection whenever no request has been sent for `KeepAliveInterval`.
|
||||
- A keep-alive failure faults the connection like any other.
|
||||
- Keep-alives count towards `RequestsSent`.
|
||||
- Test with fake time: no keep-alive while traffic flows; exactly one after an idle `KeepAliveInterval`.
|
||||
- [ ] **No null-race (finding 16).** The open path returns the `(client, connection)` pair it established, and `SendAsync` uses that pair rather than re-reading fields after the lock.
|
||||
- [ ] **Clean disposal.** Disposal never disposes a `SemaphoreSlim` that waiters may still be queued on. It sets a `_disposed` flag checked under the lock, and later callers get `AbTransportException(Abort)`.
|
||||
- [ ] **`ForwardOpenThrottle`** (per `DeviceKey`; the pool keeps throttles for the process lifetime, so a reconnect after the last release is still limited):
|
||||
- At most 1 open attempt in flight.
|
||||
- A new attempt within `MinForwardOpenInterval` of the previous **attempt start** waits for the remainder. If the caller's token would expire first, the call fails fast with `AbTransportException(BadConnection, "Forward Open throttled")`.
|
||||
- It uses an injected `TimeProvider` for testability.
|
||||
- [ ] **Storm test:** against a `FakeCipTarget` that refuses every Forward Open, 50 concurrent `SendAsync` callers for 3 s (fake time advanced by 3 s) cause **≤ 4** Forward Open frames: one at t=0, then at most one per second.
|
||||
- An attempt within `MinForwardOpenInterval` of the previous **attempt start** waits for the remainder. If the caller's token fires first, it throws `AbTransportException(BadConnection, "Forward Open throttled")`.
|
||||
- It uses an injected `TimeProvider`.
|
||||
- [ ] **Storm test:** against a `FakeCipTarget` refusing every Forward Open, 50 concurrent `SendAsync` callers over 3 s of fake time produce **≤ 4** Forward Open frames.
|
||||
- [ ] **`DeviceSessionPool`:**
|
||||
- `Acquire(DeviceKey, ConnectionOptions) → ConnectionLease : ICipChannel, IAsyncDisposable`.
|
||||
- Leases for the same key share one `CipConnection`.
|
||||
- The last lease's dispose sends Forward Close (connected mode) and then UnRegisterSession.
|
||||
- A later acquire on the same key builds a new connection.
|
||||
- The pool is thread-safe; concurrent acquire/dispose has a test using 100 parallel tasks.
|
||||
- [ ] `DeviceSessionPool.Shared` is a process-wide static instance used by the native adapters. Tests construct their own pools.
|
||||
- [ ] Diagnostics counters for the gates (Tasks 18 and 21): `internal static int ForwardOpenCount` (incremented once per `OpenAsync` that sends a Forward Open, including the size fallback) and `internal static int RequestsSent` (incremented per `SendAsync`), both via `Interlocked`. Add `InternalsVisibleTo` for `ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests` to the Ab.Transport csproj.
|
||||
- `Acquire(DeviceKey, ConnectionOptions) → ConnectionLease`. `ConnectionLease : IAsyncDisposable` exposes `ICipChannel Channel` and `RequestScheduler Scheduler`.
|
||||
- **All leases for one key share one `CipConnection` and one `RequestScheduler`** (B1). The scheduler is built as `new RequestScheduler(connection, options.AllowPacking)`. A second `Acquire` with different `ConnectionOptions` for the same key reuses the first entry's options; that is logged once and pinned by a test.
|
||||
- The last lease's dispose disposes the scheduler, sends Forward Close (`Connected` only) and then UnRegisterSession.
|
||||
- A later acquire on the same key builds new instances.
|
||||
- Thread-safe: 100 parallel acquire/dispose tasks pass.
|
||||
- [ ] `DeviceSessionPool.Shared` is the process-wide instance; tests build their own.
|
||||
- [ ] **Diagnostics for the gates (Tasks 18 and 21):** `internal static int ForwardOpenCount` (one increment per Forward Open frame sent, including the fallback) and `internal static int RequestsSent` (one per channel send, keep-alives included), both via `Interlocked`.
|
||||
|
||||
**Verify:** `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests --filter "FullyQualifiedName~CipConnection|FullyQualifiedName~ForwardOpenThrottle|FullyQualifiedName~DeviceSessionPool"` → pass.
|
||||
**Verify:** `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests --filter "FullyQualifiedName~CipConnection|FullyQualifiedName~ForwardOpenThrottle|FullyQualifiedName~DeviceSessionPool"` → pass; the storm test passes 20/20 consecutive runs.
|
||||
|
||||
**Steps:**
|
||||
|
||||
- [ ] **Step 1: Write the failing tests.**
|
||||
- The throttle tests use `Microsoft.Extensions.Time.Testing.FakeTimeProvider`. Check `Directory.Packages.props` for `Microsoft.Extensions.TimeProvider.Testing`; if it is not pinned, write a 20-line `ManualTimeProvider : TimeProvider` in `Fakes/` instead. Do not add a package.
|
||||
- Use `ManualTimeProvider : TimeProvider` in `Fakes/` (about 20 lines: settable `UtcNow`, plus timers fired by `Advance(TimeSpan)`). Check `Directory.Packages.props` for `Microsoft.Extensions.TimeProvider.Testing` first; use `FakeTimeProvider` if it is already pinned, and **never add a package**.
|
||||
- Write these tests:
|
||||
- `Second_attempt_inside_interval_waits`
|
||||
- `Attempt_after_interval_proceeds_immediately`
|
||||
- `Only_one_attempt_in_flight`
|
||||
- `Refusing_target_sees_at_most_one_forward_open_per_interval` (the storm test)
|
||||
- `Large_open_size_rejection_falls_back_to_regular_open_once`
|
||||
- `Ucmm_mode_never_sends_forward_open`
|
||||
- `Sequence_wraps_from_65535_to_1`
|
||||
- `Faulted_session_reconnects_on_next_send`
|
||||
- `Refusing_target_sees_at_most_one_forward_open_per_interval`
|
||||
- `Large_open_service_not_supported_falls_back_to_regular_open_once`
|
||||
- `Large_open_bad_size_falls_back_to_regular_open_once`
|
||||
- `Connection_in_use_does_not_fall_back`
|
||||
- `Direct_routing_sends_bare_cip_even_with_a_path`
|
||||
- `UnconnectedSend_routing_wraps_in_0x52`
|
||||
- `Connected_routing_never_sends_bare`
|
||||
- `Caller_cancellation_does_not_reconnect` (cancel mid-request, then send again ⇒ `ForwardOpenCount` unchanged)
|
||||
- `Timeout_reconnects_through_the_throttle`
|
||||
- `Idle_connection_sends_one_keepalive_per_interval`
|
||||
- `Traffic_suppresses_keepalive`
|
||||
- `Leases_for_one_key_share_one_scheduler`
|
||||
- `Last_lease_dispose_sends_forward_close_then_unregister`
|
||||
- `Concurrent_acquire_and_release_is_safe`
|
||||
- `Send_after_dispose_throws_Abort`
|
||||
- [ ] **Step 2: Run and confirm failure.** `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests --filter "FullyQualifiedName~CipConnection|FullyQualifiedName~ForwardOpenThrottle|FullyQualifiedName~DeviceSessionPool"` → compile error.
|
||||
- [ ] **Step 3: Implement the throttle.**
|
||||
|
||||
@@ -1719,7 +1814,9 @@ public sealed class ForwardOpenThrottle(TimeSpan minInterval, TimeProvider time)
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Implement `CipConnection`** (sketch; complete the code):
|
||||
The throttle's semaphore is never disposed: throttles live for the process.
|
||||
|
||||
- [ ] **Step 4: Implement `CipConnection`.**
|
||||
|
||||
```csharp
|
||||
// Cip/CipConnection.cs
|
||||
@@ -1727,97 +1824,131 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip;
|
||||
|
||||
public sealed class CipConnection : ICipChannel, IAsyncDisposable
|
||||
{
|
||||
internal static int ForwardOpenCount;
|
||||
internal static int RequestsSent;
|
||||
|
||||
private sealed record Open(EncapsulationClient Client, ForwardOpenReply? Conn);
|
||||
|
||||
private readonly DeviceKey _key;
|
||||
private readonly ConnectionOptions _o;
|
||||
private readonly ForwardOpenThrottle _throttle;
|
||||
private readonly TimeProvider _time;
|
||||
private readonly byte[] _route;
|
||||
private readonly SemaphoreSlim _state = new(1, 1);
|
||||
private EncapsulationClient? _client;
|
||||
private ForwardOpenReply? _conn;
|
||||
private readonly SemaphoreSlim _state = new(1, 1); // never disposed (see DisposeAsync)
|
||||
private readonly ITimer? _keepAlive;
|
||||
private Open? _open;
|
||||
private int _negotiatedSize;
|
||||
private ushort _seq;
|
||||
private long _lastSendTicks;
|
||||
private bool _disposed;
|
||||
private readonly ushort _connSerial = (ushort)Random.Shared.Next(1, ushort.MaxValue);
|
||||
private readonly uint _toId = (uint)Random.Shared.Next();
|
||||
|
||||
public CipConnection(DeviceKey key, ConnectionOptions o, ForwardOpenThrottle throttle)
|
||||
{ _key = key; _o = o; _throttle = throttle; _route = CipPath.EncodeRoute(key.CipPath); }
|
||||
public CipConnection(DeviceKey key, ConnectionOptions o, ForwardOpenThrottle throttle, TimeProvider time)
|
||||
{
|
||||
_key = key; _o = o; _throttle = throttle; _time = time;
|
||||
_route = CipPath.EncodeRoute(key.CipPath);
|
||||
if (o.Routing == CipRouting.Connected)
|
||||
_keepAlive = time.CreateTimer(_ => _ = KeepAliveAsync(), null, o.KeepAliveInterval, o.KeepAliveInterval);
|
||||
}
|
||||
|
||||
private bool Connected => _o.UseConnectedMessaging && _route.Length > 0;
|
||||
|
||||
public int MaxRequestBytes => Connected
|
||||
? (_negotiatedSize > 0 ? _negotiatedSize : Math.Min(_o.ConnectionSize, 504)) - 2
|
||||
: 504 - (_route.Length > 0 ? 12 + _route.Length : 0);
|
||||
public int MaxRequestBytes => _o.Routing switch
|
||||
{
|
||||
CipRouting.Connected => (_negotiatedSize > 0 ? _negotiatedSize : Math.Min(_o.ConnectionSize, 504)) - 2,
|
||||
CipRouting.UnconnectedSend => 504 - (12 + _route.Length),
|
||||
_ => 504,
|
||||
};
|
||||
|
||||
public async Task<ReadOnlyMemory<byte>> SendAsync(ReadOnlyMemory<byte> cip, CancellationToken ct)
|
||||
{
|
||||
var client = await EnsureOpenAsync(ct).ConfigureAwait(false);
|
||||
if (Connected)
|
||||
var open = await EnsureOpenAsync(ct).ConfigureAwait(false);
|
||||
Interlocked.Increment(ref RequestsSent);
|
||||
Interlocked.Exchange(ref _lastSendTicks, _time.GetTimestamp());
|
||||
return _o.Routing switch
|
||||
{
|
||||
var seq = NextSequence();
|
||||
return await client.SendUnitDataAsync(_conn!.OTConnectionId, seq, cip, _o.Timeout, ct).ConfigureAwait(false);
|
||||
}
|
||||
var payload = _route.Length > 0 ? ConnectionManagerServices.UnconnectedSend(cip.Span, _route) : cip.ToArray();
|
||||
return await client.SendRRDataAsync(payload, _o.Timeout, ct).ConfigureAwait(false);
|
||||
CipRouting.Connected => await open.Client.SendUnitDataAsync(open.Conn!.OTConnectionId, cip, _o.Timeout, ct).ConfigureAwait(false),
|
||||
CipRouting.UnconnectedSend => await open.Client.SendRRDataAsync(ConnectionManagerServices.UnconnectedSend(cip.Span, _route), _o.Timeout, ct).ConfigureAwait(false),
|
||||
_ => await open.Client.SendRRDataAsync(cip, _o.Timeout, ct).ConfigureAwait(false),
|
||||
};
|
||||
}
|
||||
|
||||
private ushort NextSequence() { _seq = _seq == ushort.MaxValue ? (ushort)1 : (ushort)(_seq + 1); return _seq; }
|
||||
|
||||
private async Task<EncapsulationClient> EnsureOpenAsync(CancellationToken ct)
|
||||
private async Task KeepAliveAsync()
|
||||
{
|
||||
if (_client is { IsConnected: true } c && (!Connected || _conn is not null)) return c;
|
||||
if (_disposed || _open is not { Client.IsConnected: true }) return;
|
||||
if (_time.GetElapsedTime(Interlocked.Read(ref _lastSendTicks)) < _o.KeepAliveInterval) return;
|
||||
try { await SendAsync(new byte[] { 0x0E, 0x03, 0x20, 0x01, 0x24, 0x01, 0x30, 0x01 }, CancellationToken.None).ConfigureAwait(false); }
|
||||
catch (AbTransportException) { /* the client is now faulted; the next real send reconnects through the throttle */ }
|
||||
}
|
||||
|
||||
private async Task<Open> EnsureOpenAsync(CancellationToken ct)
|
||||
{
|
||||
if (_open is { } o && o.Client.IsConnected) return o;
|
||||
await _state.WaitAsync(ct).ConfigureAwait(false);
|
||||
try
|
||||
{
|
||||
if (_client is { IsConnected: true } c2 && (!Connected || _conn is not null)) return c2;
|
||||
return await _throttle.RunAsync(OpenAsync, ct).ConfigureAwait(false);
|
||||
if (_disposed) throw new AbTransportException(AbStatus.Abort, "Connection disposed.");
|
||||
if (_open is { } o2 && o2.Client.IsConnected) return o2;
|
||||
return _open = await _throttle.RunAsync(OpenAsync, ct).ConfigureAwait(false);
|
||||
}
|
||||
finally { _state.Release(); }
|
||||
}
|
||||
|
||||
private async Task<EncapsulationClient> OpenAsync(CancellationToken ct)
|
||||
private async Task<Open> OpenAsync(CancellationToken ct)
|
||||
{
|
||||
if (_client is not null) await _client.DisposeAsync().ConfigureAwait(false);
|
||||
_client = null; _conn = null;
|
||||
if (_open is not null) { await _open.Client.DisposeAsync().ConfigureAwait(false); _open = null; }
|
||||
var client = await EncapsulationClient.ConnectAsync(_key.Host, _key.Port, _o.Timeout, ct).ConfigureAwait(false);
|
||||
if (Connected)
|
||||
if (_o.Routing != CipRouting.Connected) return new Open(client, null);
|
||||
try
|
||||
{
|
||||
var large = _o.ConnectionSize > 511;
|
||||
var reply = CipReply.Parse(await client.SendRRDataAsync(ConnectionManagerServices.ForwardOpen(Params(large, _o.ConnectionSize)), _o.Timeout, ct).ConfigureAwait(false));
|
||||
if (large && ConnectionManagerServices.IsSizeRejection(reply))
|
||||
_negotiatedSize = _o.ConnectionSize;
|
||||
var reply = await ForwardOpenAsync(client, large, _negotiatedSize, ct).ConfigureAwait(false);
|
||||
if (large && ConnectionManagerServices.ShouldFallBackToRegularOpen(reply))
|
||||
{
|
||||
_negotiatedSize = Math.Min(_o.ConnectionSize, 504);
|
||||
reply = CipReply.Parse(await client.SendRRDataAsync(ConnectionManagerServices.ForwardOpen(Params(false, _negotiatedSize)), _o.Timeout, ct).ConfigureAwait(false));
|
||||
reply = await ForwardOpenAsync(client, false, _negotiatedSize, ct).ConfigureAwait(false);
|
||||
}
|
||||
else _negotiatedSize = _o.ConnectionSize;
|
||||
try { _conn = ConnectionManagerServices.ParseForwardOpenReply(reply); }
|
||||
catch { await client.DisposeAsync().ConfigureAwait(false); throw; }
|
||||
return new Open(client, ConnectionManagerServices.ParseForwardOpenReply(reply));
|
||||
}
|
||||
return _client = client;
|
||||
catch { await client.DisposeAsync().ConfigureAwait(false); throw; }
|
||||
}
|
||||
|
||||
private ForwardOpenParams Params(bool large, int size) =>
|
||||
new(large, _toId, _connSerial, _o.VendorId, _o.OriginatorSerial, _o.TimeoutMultiplier, _o.RpiMicros, size, _route);
|
||||
private async Task<CipReply> ForwardOpenAsync(EncapsulationClient client, bool large, int size, CancellationToken ct)
|
||||
{
|
||||
Interlocked.Increment(ref ForwardOpenCount);
|
||||
var p = new ForwardOpenParams(large, _toId, _connSerial, _o.VendorId, _o.OriginatorSerial, _o.TimeoutMultiplier, _o.RpiMicros, size, _route);
|
||||
return CipReply.Parse(await client.SendRRDataAsync(ConnectionManagerServices.ForwardOpen(p), _o.Timeout, ct).ConfigureAwait(false));
|
||||
}
|
||||
|
||||
public async ValueTask DisposeAsync()
|
||||
{
|
||||
if (_client is { IsConnected: true } c && _conn is not null)
|
||||
await _state.WaitAsync().ConfigureAwait(false);
|
||||
Open? open;
|
||||
try { _disposed = true; open = _open; _open = null; }
|
||||
finally { _state.Release(); }
|
||||
if (_keepAlive is not null) await _keepAlive.DisposeAsync().ConfigureAwait(false);
|
||||
if (open is null) return;
|
||||
if (open.Conn is not null && open.Client.IsConnected)
|
||||
{
|
||||
try
|
||||
{
|
||||
using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(1));
|
||||
await c.SendRRDataAsync(ConnectionManagerServices.ForwardClose(_connSerial, _o.VendorId, _o.OriginatorSerial, _route), TimeSpan.FromSeconds(1), cts.Token).ConfigureAwait(false);
|
||||
await open.Client.SendRRDataAsync(ConnectionManagerServices.ForwardClose(_connSerial, _o.VendorId, _o.OriginatorSerial, _route), TimeSpan.FromSeconds(1), cts.Token).ConfigureAwait(false);
|
||||
}
|
||||
catch (AbTransportException) { /* PLC expires the connection after RPI × multiplier */ }
|
||||
catch (Exception ex) when (ex is AbTransportException or OperationCanceledException) { /* PLC expires it after RPI × multiplier */ }
|
||||
}
|
||||
if (_client is not null) await _client.DisposeAsync().ConfigureAwait(false);
|
||||
_state.Dispose();
|
||||
await open.Client.DisposeAsync().ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Implement `DeviceSessionPool`.** Use a `Dictionary<DeviceKey, Entry>` guarded by a `lock`, where `Entry` holds `(CipConnection Conn, ForwardOpenThrottle Throttle, int RefCount)`. The throttle outlives the connection: keep throttles in their own dictionary keyed by `DeviceKey` and never remove them, so that a reconnect after the last lease is released is still rate-limited. `ConnectionLease` forwards `ICipChannel` to the entry's connection; its `DisposeAsync` decrements the count and, at zero, removes the entry and disposes the connection outside the lock.
|
||||
- [ ] **Step 6: Run.** `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests` → all pass. Run the storm test 20× (`for i in $(seq 20); do dotnet test --filter Refusing_target ...; done`) to prove it is not flaky.
|
||||
- [ ] **Step 7: Commit.** `git add src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests && git commit -m "feat(ab-transport): per-device CIP connection, session pool and Forward Open throttle"`
|
||||
- [ ] **Step 5: Implement `DeviceSessionPool` + `ConnectionLease`.**
|
||||
- Keep a `Dictionary<DeviceKey, Entry>` under a `lock`, where `Entry = (CipConnection Conn, RequestScheduler Scheduler, ConnectionOptions Options, int RefCount)`.
|
||||
- Keep a separate, never-pruned `Dictionary<DeviceKey, ForwardOpenThrottle>`.
|
||||
- `Acquire` creates the entry on first use (`new CipConnection(key, o, throttle, time)`, `new RequestScheduler(conn, o.AllowPacking)`) and increments `RefCount`.
|
||||
- `ConnectionLease.DisposeAsync` is idempotent (`Interlocked` flag). It decrements the count and, at zero, removes the entry; **outside the lock** it awaits `Scheduler.DisposeAsync()`, then `Conn.DisposeAsync()`.
|
||||
- The pool constructor takes an optional `TimeProvider` (default `TimeProvider.System`).
|
||||
- [ ] **Step 6: Run.** `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests` → all pass, then run `Refusing_target_sees_at_most_one_forward_open_per_interval` 20× to check it is not flaky.
|
||||
- [ ] **Step 7: Commit.** `git add src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests && git commit -m "feat(ab-transport): per-device connection with routing modes, shared scheduler, keep-alive and Forward Open throttle"`
|
||||
|
||||
---
|
||||
|
||||
@@ -1838,9 +1969,17 @@ public sealed class CipConnection : ICipChannel, IAsyncDisposable
|
||||
- [ ] **Boundary tests:** with `MaxRequestBytes = M`, two requests whose MSP size is exactly `M` pack into one frame, and two whose MSP size is `M + 1` go as two frames.
|
||||
- [ ] **A single request larger than `MaxRequestBytes`** is sent bare. Callers must fragment; `LogixTagClient` does.
|
||||
- [ ] **Reply fan-out:** each waiter gets its own embedded `CipReply`, with its own general status, so one failing service in an MSP fails only its own waiter. If the MSP envelope itself fails (non-zero general status other than `0x1E` embedded-service-error), or the channel throws, every waiter in that batch faults with the same `AbTransportException`.
|
||||
- [ ] **Cancellation:** a waiter cancelled while queued is removed and never sent. A waiter cancelled while its batch is in flight gets `OperationCanceledException`, and the batch's other waiters still complete.
|
||||
- [ ] **Cancellation:**
|
||||
- A waiter cancelled while queued is removed and never sent.
|
||||
- A waiter cancelled while its batch is in flight gets `OperationCanceledException`, and the batch's other waiters still complete.
|
||||
- **The channel call always uses the scheduler's own `_stop` token, never a waiter's**, for single sends as well as MSPs (B2). So one caller's cancellation can never abort, or fault, a shared exchange.
|
||||
- Each waiter's `CancellationTokenRegistration` is stored on its `Pending` record and **disposed when the TCS completes** (review finding 10). A test makes 10 000 sends on one long-lived token and asserts no growth: measure with a counting `CancellationTokenSource` subclass, or assert `Pending` registrations are disposed via a test hook.
|
||||
- [ ] **Owner:** one scheduler per device connection, created by `DeviceSessionPool` (T9). This task builds only the class and tests it over `FakeCipChannel`.
|
||||
- [ ] **`LogixTagClient`:**
|
||||
- `ReadAsync(path, elements)` sends Read Tag. On reply status `0x06` (Partial), or when the expected size exceeds `MaxRequestBytes - 8`, it switches to Read Tag Fragmented, starting at offset 0 and advancing by each reply's data length until the status is `0x00`. It concatenates the data and returns `(TypeCode, StructHandle, byte[] Data)`.
|
||||
- `ReadAsync(path, elements)` sends Read Tag.
|
||||
- If the reply status is `0x06` (Partial), it **keeps** that reply's data and continues with Read Tag Fragmented at `offset = bytesReceivedSoFar`, appending each reply's data until a reply has status `0x00`. That avoids double-counting the first chunk (review finding 15).
|
||||
- It returns `(TypeCode, StructHandle, byte[] Data)`. The type code and handle come from the first reply; fragment replies repeat the type header, so strip it from each before appending.
|
||||
- **Reply-size note:** packing bounds only the *request* size, so many large reads packed together can produce a reply that the controller truncates with `0x06` per service. That is correct behaviour (the fragment loop recovers it), not an error.
|
||||
- `WriteAsync(path, type, handle, elements, data)` sends one Write Tag when it fits, otherwise Write Tag Fragmented chunks of `MaxRequestBytes - 12 - path.Length`, aligned down to the element width.
|
||||
- Non-zero final status ⇒ `AbTransportException(reply.Status)`.
|
||||
- [ ] `FakeCipChannel` lets a test set `MaxRequestBytes`, records every request, and answers via a delegate. It is reused by Tasks 12 and 13.
|
||||
@@ -1879,7 +2018,9 @@ Tests to write (each with explicit byte sizes):
|
||||
- `Per_service_error_fails_only_its_waiter`: the MSP reply carries replies with status 0 and status 0x04. Waiter 1 gets Ok, waiter 2 gets a `CipReply` with `GeneralStatus == 0x04`.
|
||||
- `Channel_failure_faults_every_waiter_in_the_batch`.
|
||||
- `Queued_waiter_cancelled_before_send_is_never_sent`.
|
||||
- `Fragmented_read_concatenates_until_status_zero`: the first reply has status 0x06 with 100 bytes, the second 0x06 with 100, the third 0x00 with 40. The result is 240 bytes, and the offsets sent were 0, 100 and 200.
|
||||
- `Fragmented_read_keeps_the_first_partial_and_continues`: the first (Read Tag) reply has status 0x06 with 100 data bytes, and the next two are Read Tag Fragmented replies (0x06 with 100 bytes, then 0x00 with 40). The result is 240 bytes, and the fragmented requests were sent at offsets **100 and 200**.
|
||||
- `Waiter_cancellation_never_reaches_the_channel_token`: the fake channel records the token it received; assert it is never a waiter's token.
|
||||
- `Cancellation_registrations_are_released`.
|
||||
- `Fragmented_write_chunks_align_to_element_width`.
|
||||
|
||||
- [ ] **Step 2: Run and confirm failure.** `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests --filter "FullyQualifiedName~RequestScheduler|FullyQualifiedName~LogixTagClient"` → compile error.
|
||||
@@ -1893,7 +2034,12 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip;
|
||||
|
||||
public sealed class RequestScheduler : IAsyncDisposable
|
||||
{
|
||||
private sealed record Pending(byte[] Request, TaskCompletionSource<CipReply> Tcs, CancellationToken Ct);
|
||||
private sealed class Pending(byte[] request)
|
||||
{
|
||||
public byte[] Request { get; } = request;
|
||||
public TaskCompletionSource<CipReply> Tcs { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||
public CancellationTokenRegistration Registration { get; set; }
|
||||
}
|
||||
|
||||
private readonly ICipChannel _channel;
|
||||
private readonly bool _allowPacking;
|
||||
@@ -1908,15 +2054,20 @@ public sealed class RequestScheduler : IAsyncDisposable
|
||||
|
||||
public Task<CipReply> SendAsync(byte[] request, CancellationToken ct)
|
||||
{
|
||||
var p = new Pending(request, new TaskCompletionSource<CipReply>(TaskCreationOptions.RunContinuationsAsynchronously), ct);
|
||||
var p = new Pending(request);
|
||||
LinkedListNode<Pending> node;
|
||||
lock (_lock) node = _queue.AddLast(p);
|
||||
ct.Register(() =>
|
||||
if (ct.CanBeCanceled)
|
||||
{
|
||||
bool removed;
|
||||
lock (_lock) { removed = node.List is not null; if (removed) _queue.Remove(node); }
|
||||
p.Tcs.TrySetCanceled(ct);
|
||||
});
|
||||
p.Registration = ct.Register(() =>
|
||||
{
|
||||
lock (_lock) { if (node.List is not null) _queue.Remove(node); }
|
||||
p.Tcs.TrySetCanceled(ct);
|
||||
});
|
||||
// Release the registration as soon as the request completes, however it completes (finding 10).
|
||||
_ = p.Tcs.Task.ContinueWith(static (_, state) => ((Pending)state!).Registration.Dispose(), p,
|
||||
CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default);
|
||||
}
|
||||
_signal.Release();
|
||||
return p.Tcs.Task;
|
||||
}
|
||||
@@ -1950,7 +2101,7 @@ public sealed class RequestScheduler : IAsyncDisposable
|
||||
{
|
||||
if (batch.Count == 1)
|
||||
{
|
||||
var raw = await _channel.SendAsync(batch[0].Request, batch[0].Ct).ConfigureAwait(false);
|
||||
var raw = await _channel.SendAsync(batch[0].Request, _stop.Token).ConfigureAwait(false); // never a waiter's token (B2)
|
||||
batch[0].Tcs.TrySetResult(CipReply.Parse(raw));
|
||||
}
|
||||
else
|
||||
@@ -1998,9 +2149,10 @@ public sealed class RequestScheduler : IAsyncDisposable
|
||||
- Create: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/LibplctagConfinementGuardTests.cs`
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- [ ] The test walks up from `AppContext.BaseDirectory` to the directory containing `ZB.MOM.WW.OtOpcUa.slnx`, then scans every `*.cs` under `src/` (excluding `bin`/`obj`) for `using libplctag` or `libplctag.`.
|
||||
- [ ] The test walks up from `AppContext.BaseDirectory` to the directory containing `ZB.MOM.WW.OtOpcUa.slnx`, then scans every `*.cs` under `src/` (excluding `bin`/`obj`) for **code** references: `using libplctag` or `libplctag.(Tag|Status|PlcType|Protocol|LibPlcTagException|NativeImport)`. Prose such as the CLI help string `"via libplctag. Second of six…"` (`AbCip.Cli/Program.cs:9`, `AbLegacy.Cli/Program.cs:8`) must **not** match, or the guard is red from wave 2 until T15/T16 (review finding 6).
|
||||
- [ ] Allowed files: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/Libplctag*.cs` and `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/Libplctag*.cs` only. Comments and XML docs are ignored by stripping `//` line comments before matching.
|
||||
- [ ] The failure message lists each offending file and line.
|
||||
- [ ] A second fact, `Prose_mentions_of_libplctag_do_not_trip_the_guard`, runs the regex against the literal `"…via libplctag. Second of six driver CLIs"` and asserts no match.
|
||||
- [ ] **Falsifiability check:** temporarily add `using libplctag;` to `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs`, run the test and see it go red, then revert. Record that you did this in the commit message.
|
||||
|
||||
**Verify:** `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests --filter LibplctagConfinementGuard` → pass.
|
||||
@@ -2017,7 +2169,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests;
|
||||
|
||||
public sealed class LibplctagConfinementGuardTests
|
||||
{
|
||||
private static readonly Regex Reference = new(@"\busing\s+libplctag\b|\blibplctag\.", RegexOptions.Compiled);
|
||||
private static readonly Regex Reference = new(
|
||||
@"\busing\s+libplctag\b|\blibplctag\.(Tag|Status|PlcType|Protocol|LibPlcTagException|NativeImport)\b",
|
||||
RegexOptions.Compiled);
|
||||
|
||||
[Fact]
|
||||
public void Only_the_libplctag_adapters_reference_libplctag()
|
||||
@@ -2061,16 +2215,21 @@ public sealed class LibplctagConfinementGuardTests
|
||||
- Create: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagRuntime.cs`
|
||||
- Create: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagFactory.cs`
|
||||
- Create: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipDevice.cs` (maps `AbCipTagCreateParams` → `DeviceKey` + `ConnectionOptions`; shared with Task 13)
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs` (add trailing optional `AbCipDataType? AuthoredType = null` to `AbCipTagCreateParams`)
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs` (`EnsureTagRuntimeAsync` passes `AuthoredType: def.DataType`)
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs` (add trailing optionals `AbCipDataType? AuthoredType = null, bool UseConnectedMessaging = true` to `AbCipTagCreateParams`)
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs` (pass `UseConnectedMessaging: profile.SupportsConnectedMessaging`)
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs` (`EnsureTagRuntimeAsync` passes `AuthoredType: def.DataType`; `DeviceState`'s create-params builder passes `UseConnectedMessaging: Profile.SupportsConnectedMessaging`)
|
||||
- Create: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTagRuntimeTests.cs`
|
||||
- Modify: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests.csproj` (add a `ProjectReference` to Ab.Transport and `<Compile Include="..\ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests\Fakes\FakeCipChannel.cs" Link="Fakes\FakeCipChannel.cs" />`. Source-linking compiles the fake into this assembly, so it stays `internal`. Also add `InternalsVisibleTo` for `ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests` to the Ab.Transport csproj if the tests touch internal transport types)
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- [ ] `NativeAbCipDevice.Key(p)` = `new DeviceKey(p.Gateway, p.Port > 0 ? p.Port : 44818, p.CipPath, p.LibplctagPlcAttribute)`.
|
||||
- [ ] `NativeAbCipDevice.Options(p)` = `Timeout = p.Timeout`, `ConnectionSize = p.ConnectionSize`, `UseConnectedMessaging = p.LibplctagPlcAttribute != "micro800"`. It is the **only** mapping from family to transport behaviour; it reads the same `LibplctagPlcAttribute` string the family profile already sets.
|
||||
- [ ] `NativeAbCipDevice.Options(p)`:
|
||||
- `Timeout = p.Timeout`, `ConnectionSize = p.ConnectionSize`, `AllowPacking = p.AllowPacking`.
|
||||
- **`Routing`:** if `p.UseConnectedMessaging` is false, use `UnconnectedSend` when `p.CipPath` is non-empty and `Direct` when it is empty. Otherwise use `Connected`.
|
||||
- `UseConnectedMessaging` is a new trailing optional `bool UseConnectedMessaging = true` on `AbCipTagCreateParams`. It is set at every construction site from `AbCipPlcFamilyProfile.SupportsConnectedMessaging`, so the family profile stays the single authority and string-matching `"micro800"` is not needed (review finding 18). The sites are `AbCipDriver` `DeviceState` (around `AbCipDriver.cs:1690-1700`, where `AllowPacking`/`ConnectionSize` are already merged) and `AbCipDriverProbe`; find any others with `grep -rn "new AbCipTagCreateParams" src`.
|
||||
- Cross-check the result against the Task 4 "Routing observed" table for controllogix and micro800, and record any mismatch in the test comment.
|
||||
- [ ] **Initialization and reads:**
|
||||
- `InitializeAsync` acquires the lease, builds a `RequestScheduler(lease, p.AllowPacking)` and a `LogixTagClient`, and encodes the path once. It splits off a trailing bit selector with `CipPath.SplitBitSelector`, which is stored as the runtime's implicit bit.
|
||||
- `InitializeAsync` acquires the lease from `DeviceSessionPool.Shared` and builds a per-runtime `LogixTagClient` over **the lease's shared `Scheduler` and `Channel`**. It must never construct its own `RequestScheduler`: that would defeat MSP coalescing across tags (B1). It encodes the path once, splitting off a trailing bit selector with `CipPath.SplitBitSelector`; that is stored as the runtime's implicit bit.
|
||||
- It performs **one read** to prove the tag exists. That matches libplctag, whose `InitializeAsync` errors for an unknown tag; the driver's 05/STAB-8 logic depends on create throwing `NotFound`.
|
||||
- `ReadAsync` reads `p.ElementCount` elements, stores the data, `TypeCode` and `StructHandle`, and sets status `Ok`. On `AbTransportException` it stores the exception's status **and rethrows**, matching libplctag's exception-on-failure behaviour. `GetSize()` returns the data length.
|
||||
- [ ] **Decode** is byte-identical to `LibplctagTagRuntime` for every `AbCipDataType` (Bool/SInt/USInt/Int/UInt/DInt/UDInt/LInt/ULInt/Real/LReal/String/Dt). Specifically:
|
||||
@@ -2079,15 +2238,18 @@ public sealed class LibplctagConfinementGuardTests
|
||||
- String ⇒ Logix STRING `u32 LEN` + `DATA`, ASCII, `LEN` clamped to the data length − 4.
|
||||
- `DecodeArray` uses stride = `data.Length / count` when it divides evenly, else the static width (the same `ResolveArrayStride` rule; call `LibplctagTagRuntime.ResolveArrayStride` with `elementSize: null` to share it).
|
||||
- Bool arrays are bit-packed (bit `i` of the buffer).
|
||||
- [ ] **Type check:** after a read, if `TypeCode` is atomic and differs from the authored type's Logix code (mapping `AbCipDataType` → `LogixTypeCode`; `Dt → DInt`, `String`/`Structure` exempt), `GetStatus()` returns `(int)AbStatus.TypeMismatch` and `Decode*` returns null. A BOOL read with a bit index against a DINT host is **not** a mismatch.
|
||||
- [ ] **Type check:** after a read, if `TypeCode` is atomic and differs from the authored type's Logix code, `GetStatus()` returns `(int)AbStatus.TypeMismatch` and `Decode*` returns null. The mapping is `AbCipDataType` → `LogixTypeCode`, with `Dt → DInt`; `String`/`Structure` are exempt. Exemptions:
|
||||
- (a) A BOOL read with a bit index against a DINT host is **not** a mismatch.
|
||||
- (b) **Authored `Bool` with returned `0xD3`** (the Logix packed BOOL-array type) is **not** a mismatch (review finding 11).
|
||||
- Tests cover both exemptions, plus a positive mismatch.
|
||||
- [ ] **Encode/write:**
|
||||
- `EncodeValue` fills a managed write buffer with the same encodings as `LibplctagTagRuntime.EncodeValue`, including the same `NotSupportedException` cases.
|
||||
- `WriteAsync` writes with type = the last read `TypeCode` if known, else the authored type's code.
|
||||
- For String, it uses `Struct` + the last `StructHandle`. If none is known it performs a read first; that read happens inside `WriteAsync`, under the driver's existing per-runtime lock.
|
||||
- The STRING write payload is `u32 len` + DATA padded to the capacity learned from the last read's data length − 4.
|
||||
- [ ] `Dispose` disposes the scheduler and the lease (fire-and-forget `DisposeAsync().AsTask()`; log nothing).
|
||||
- [ ] `Dispose` releases the lease, waiting for at most 2 s (`lease.DisposeAsync().AsTask().Wait(TimeSpan.FromSeconds(2))`). It must not fire and forget: an immediate re-init would otherwise open a second session to the press while the old Forward Close is still in flight (review finding 22). It never disposes the shared scheduler; the pool does that on the last release.
|
||||
- [ ] `NativeAbCipTagFactory : IAbCipTagFactory` creates `NativeAbCipTagRuntime`.
|
||||
- [ ] Test seam: `internal static NativeAbCipTagRuntime ForTest(ICipChannel channel, AbCipTagCreateParams p)` builds a runtime over the given channel (no pool lease). Production code always goes through the factory and `DeviceSessionPool.Shared`.
|
||||
- [ ] Test seam: `internal static NativeAbCipTagRuntime ForTest(ICipChannel channel, AbCipTagCreateParams p)` builds a runtime over the given channel with a private `RequestScheduler(channel, p.AllowPacking)` (test-only; no pool lease). Production code always goes through the factory and `DeviceSessionPool.Shared`.
|
||||
- [ ] **Unit tests** (against `FakeCipChannel` injected through an `internal` constructor that takes an `ICipChannel`):
|
||||
- decode parity for each type, using a buffer captured from `LibplctagTagRuntime` semantics (hard-coded bytes);
|
||||
- the TypeMismatch case (REAL returned for a DInt-authored tag);
|
||||
@@ -2157,7 +2319,7 @@ public async Task Unknown_tag_fails_initialize_with_NotFound()
|
||||
- [ ] Unit tests feed the **real controller captures** through the native path. The fake channel serves, as reply data, the bytes contained in `CipTemplateObjectWireCaptureTests` (rev 34.11 press). The test asserts that `CipTemplateObjectDecoder.Decode(nativeBuffer)` equals `Decode(capturedLibplctagBuffer)`, member by member.
|
||||
- The captured libplctag buffer's header **is** the 14-byte header, so the fake replies are built from that header's fields plus the raw bytes after byte 14.
|
||||
- Do the same for `CipSymbolObjectRealControllerCaptureTests` entries: split them across two pages with a `0x06` status between, and assert the decoded tag list is identical.
|
||||
- [ ] Both factories (`NativeAbCipTagEnumeratorFactory`, `NativeAbCipTemplateReaderFactory`) exist and lease from `DeviceSessionPool.Shared` via `NativeAbCipDevice`.
|
||||
- [ ] Both factories (`NativeAbCipTagEnumeratorFactory`, `NativeAbCipTemplateReaderFactory`) exist and lease from `DeviceSessionPool.Shared` via `NativeAbCipDevice`. They send through **the lease's shared `Scheduler`**, never a private one, and dispose the lease (bounded 2 s wait, as in T12) when the enumeration or read completes.
|
||||
|
||||
**Verify:** `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests --filter "FullyQualifiedName~NativeAbCipTagEnumerator|FullyQualifiedName~NativeAbCipTemplateReader|FullyQualifiedName~Discovery"` → pass.
|
||||
|
||||
@@ -2184,8 +2346,11 @@ public async Task Unknown_tag_fails_initialize_with_NotFound()
|
||||
**Acceptance Criteria:**
|
||||
- [ ] **Connection:**
|
||||
- `DeviceKey` is built from `AbLegacyTagCreateParams` the same way as Task 12 (default port 44818).
|
||||
- Use connected messaging **only if** `Captures/slc500-read-n7.hex` shows libplctag opening a Forward Open for the `slc500` profile. Otherwise use UCMM (SendRRData, with Unconnected Send when the route is non-empty). Record which one the capture showed in a comment.
|
||||
- `ConnectionSize = 504`.
|
||||
- **`Routing` comes from the Task 4 "Routing observed" table (B3), per family:**
|
||||
- Expected: `slc500`/`micrologix`/`plc5` ⇒ **`Direct`** (libplctag sends Execute PCCC bare in SendRRData even though the driver forces `EffectiveCipPath = "1,0"`, `AbLegacyDriver.cs:864`); `logixpccc` ⇒ **`Connected`**.
|
||||
- **If the capture shows otherwise, the capture wins.** Encode the choice in one `static CipRouting RoutingFor(string plcAttribute)` with a comment citing the capture file, and pin it with a test per family.
|
||||
- `ConnectionSize = 504`, `AllowPacking = false` (PCCC requests are never MSP-packed).
|
||||
- Send through the lease's shared `Scheduler`, never a private one (B1).
|
||||
- [ ] **Address handling:**
|
||||
- `AbLegacyAddress.TryParse(p.TagName)` ⇒ `PcccAddress(FileLetter, FileNumber, WordNumber, sub)`. The sub-element comes from `ResolveSubElement`. A `BitIndex` is kept for decode only, because reads are word reads.
|
||||
- Byte counts per element: N/A/B/I/O/S = 2, F/L = 4, T/C/R = 6 when no sub-element (the whole element), 2 when a word sub-element is chosen, ST = 84.
|
||||
@@ -2193,6 +2358,7 @@ public async Task Unknown_tag_fails_initialize_with_NotFound()
|
||||
- [ ] **Read and write:**
|
||||
- `ReadAsync` dispatches by `plc` attribute: `slc500`/`micrologix`/`logixpccc` ⇒ `SlcTypedRead`; `plc5` ⇒ `Plc5TypedRead`, or `AbTransportException(Unsupported)` if Task 8 marked PLC-5 unverified.
|
||||
- The TNS increments per request.
|
||||
- **Chunking (review finding 12):** a read or write whose byte count exceeds `PcccServices.MaxDataBytes` (236) is split into consecutive element ranges of at most 236 bytes (118 words, or 59 longs/floats). The element number advances per chunk and the data is concatenated in order. A failure in any chunk fails the whole operation with that chunk's status; a partial write is reported as failed, never as Good. Test a 256-word array read (three chunks: 118 + 118 + 20) and a 130-word write (two chunks).
|
||||
- On failure it stores the status and rethrows.
|
||||
- `InitializeAsync` performs one read, for the same `NotFound`-on-create reason as Task 12.
|
||||
- [ ] **Decode** is identical to `LibplctagLegacyTagRuntime`:
|
||||
@@ -2251,7 +2417,11 @@ private static byte[] PcccReply(ushort tns, byte[] data) =>
|
||||
- [ ] `AbCipTransportFactories.For(t)` returns `(IAbCipTagFactory Tags, IAbCipTagEnumeratorFactory Enumerators, IAbCipTemplateReaderFactory Templates)`: the native trio for `Native`, the libplctag trio for `Libplctag`.
|
||||
- [ ] Explicitly injected factories (tests) still win over the option.
|
||||
- [ ] JSON configs: `"transport": "Native"` ⇒ Native; `"transport": "native"` ⇒ Native (case-insensitive, as with the existing `ParseEnum`); absent ⇒ Libplctag; `"transport": "Bogus"` ⇒ the same failure the existing `ParseEnum` raises for a bad `PlcFamily` (do not invent a new behaviour).
|
||||
- [ ] `ReinitializeAsync` with a changed transport takes effect. `ParseOptions` already feeds reinit; add a test that a reinit with a different transport swaps the factories, or document why the driver instance must be recreated. Check `AbCipDriver.ReinitializeAsync` first and follow what it already does for other options.
|
||||
- [ ] **Reinit does NOT swap transports, by design (review finding 19).** `AbCipDriver.ReinitializeAsync` (`AbCipDriver.cs:401`) is `ShutdownAsync` + `InitializeAsync` over constructor-held factories, so a transport change in the reinit JSON is inert. Do **not** invent factory swapping:
|
||||
- Document in the option's XML doc and in `docs/drivers/AbCip.md` (T20) that changing `transport` takes effect when the driver instance is recreated, which a deploy that changes the driver config does.
|
||||
- Pin the current behaviour with a test, `Reinit_with_a_changed_transport_keeps_the_constructed_transport`, so a future change is deliberate.
|
||||
- Before writing that test, confirm how a deploy applies a changed `DriverConfig`: `grep -rn "ReinitializeAsync\|CreateInstance" src/Server src/Core | grep -i driver`. If deploys recreate the instance, say so in the doc. If they call `ReinitializeAsync`, raise it as a follow-up in the commit message rather than widening this task.
|
||||
- [ ] The **probe** deserializes with its own `JsonStringEnumConverter`, so a bad `"transport"` reports "Config JSON is invalid", not the `ParseEnum` message. That is acceptable; assert that exact behaviour in the probe test rather than the driver's.
|
||||
- [ ] The probe with `Transport: Native` constructs a `NativeAbCipTagRuntime`. Prove it with a test that uses an unreachable `127.0.0.1:1` endpoint and asserts the failure message comes from the native path ("TCP connect … failed"), not libplctag's.
|
||||
- [ ] The CLI `--transport native` reaches `AbCipDriverOptions.Transport`.
|
||||
|
||||
@@ -2311,7 +2481,7 @@ _templateReaderFactory = templateReaderFactory ?? defaults.Templates;
|
||||
- [ ] An injected factory wins over the option.
|
||||
- [ ] The probe with Native uses the native path; prove it with the unreachable-endpoint message test.
|
||||
- [ ] The CLI `--transport native` reaches the options.
|
||||
- [ ] A reinit with a changed transport is handled the same way Task 15 decided.
|
||||
- [ ] A reinit with a changed transport keeps the constructed transport (`AbLegacyDriver` reinit has the same Shutdown+Initialize shape), pinned by the same kind of test and documentation as Task 15.
|
||||
|
||||
**Verify:** `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests --filter AbLegacyTransportSwitch && dotnet test tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests` → pass.
|
||||
|
||||
@@ -2341,13 +2511,14 @@ _templateReaderFactory = templateReaderFactory ?? defaults.Templates;
|
||||
- Create: `docs/plans/2026-10-07-native-ab-transport-gate-results.md` (results table; Tasks 18 and 21 append to it)
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- [ ] `TransportMatrix.Profiles` yields `(profile, transport)` for every `KnownProfiles.All` entry × both transports. The theories switch from `MemberData(nameof(Profiles))` to `MemberData(nameof(TransportMatrix.Profiles), MemberType = typeof(TransportMatrix))`, and options get `Transport = transport`.
|
||||
- [ ] `TransportMatrix.Profiles` yields `(profile, transport)` for every profile × both transports. The theories switch from `MemberData(nameof(Profiles))` to `MemberData(nameof(TransportMatrix.Profiles), MemberType = typeof(TransportMatrix))`, and options get `Transport = transport`.
|
||||
- [ ] **The AbLegacy matrix is its own class** in the AbLegacy integration project, using `AbLegacyTransport`, and it **preserves the existing `AB_LEGACY_COMPOSE_PROFILE` filter** that `AbLegacyReadSmokeTests.Profiles` applies today (`AbLegacyReadSmokeTests.cs:26-35`): it must filter `KnownProfiles.All` by that env var before crossing with transports. The AbCip snippet below does not apply to AbLegacy verbatim (review finding 20).
|
||||
- [ ] Run against `10.100.0.35`, one compose profile at a time:
|
||||
- AbCip: `controllogix`, `compactlogix`, `micro800`, `guardlogix`.
|
||||
- AbLegacy: `slc500`, `micrologix`, `plc5`.
|
||||
- **For each profile, the set of passing tests under `Native` ⊇ the set under `Libplctag`.**
|
||||
- [ ] Any native-only failure is fixed in the transport task's code (reopen that task's files) and is not skipped. The only allowed exception is the PLC-5 `Unsupported` path from Task 8, marked `Skip = "PLC-5 framing unverified — see Task 8"` **only if** Task 8 marked it.
|
||||
- [ ] The results doc records, per profile × transport, the pass/fail/skip counts plus the `ab_server` image tag and the commit.
|
||||
- [ ] The results doc records, under its `## ab_server suites (T17)` heading, per profile × transport, the pass/fail/skip counts plus the `ab_server` image tag and the commit.
|
||||
|
||||
**Verify:** for each profile P: `ssh dohertj2@10.100.0.35 'cd /opt/otopcua-abcip && docker compose --profile P up -d'` then `AB_SERVER_ENDPOINT=10.100.0.35:44818 dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests --logger "trx;LogFileName=P.trx"` → the Native pass set ⊇ the Libplctag pass set (compare the two trx files with `grep -o 'testName="[^"]*" .*outcome="[^"]*"'`).
|
||||
|
||||
@@ -2395,10 +2566,12 @@ public static class TransportMatrix
|
||||
- Assert `GetStatus()` is equal, **except** that native `TypeMismatch` where libplctag says `Ok` is reported as a distinct *expected divergence* (spec §5), listed in the output and not failed.
|
||||
- Assert the raw buffers are equal. To expose the raw bytes, add `internal ReadOnlyMemory<byte> RawBuffer` to both runtimes: libplctag's via `_tag.GetBuffer()`, native's via its stored data. Add `InternalsVisibleTo` for the integration test assembly in the AbCip csproj.
|
||||
- Assert the decoded values are equal.
|
||||
- [ ] **Connection discipline:**
|
||||
- Each transport's runtimes are created and disposed sequentially, never concurrently, so one press sees at most two sessions over the run (one libplctag, one native).
|
||||
- [ ] **Connection discipline (review finding 9):**
|
||||
- Before any native work, the harness acquires one `DeviceSessionPool.Shared.Acquire(key, options)` **anchor lease** for the target, and holds it until the run ends. Per-tag native runtimes therefore share that one connection: the pool never reaches refcount 0, so it never sends Forward Close and re-opens between tags.
|
||||
- libplctag runtimes and native runtimes are never alive at the same time. All libplctag reads run first, and their handles are disposed. Then all native reads run. So the press sees at most two sessions over the run, never concurrently.
|
||||
- With `AB_DIFF_COUNT_OPENS=1`, the native side asserts it made exactly **one** Forward Open over the whole run, using `CipConnection.ForwardOpenCount` from Task 9 (reset it at the start of the run).
|
||||
- [ ] Output: a markdown table (tag, type, libplctag status, native status, equal?) written to `TestContext.Current.TestOutputHelper`.
|
||||
- [ ] Results are appended under the gate-results doc's `## Differential vs ab_server (T18)` heading only.
|
||||
- [ ] Run against `ab_server` (`controllogix` profile) with `AB_DIFF_ENDPOINT=10.100.0.35:44818` ⇒ zero unexpected divergences. Append the results to the gate-results doc.
|
||||
- [ ] **Ordering:** blocked by Task 17, so any transport fixes found by the ab_server suites land before the differential run.
|
||||
|
||||
@@ -2433,7 +2606,7 @@ public static class TransportMatrix
|
||||
4. Read the UNS/raw node with the Client CLI (`dotnet run --project src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI -- read -u opc.tcp://localhost:4840 -n "ns=<raw ns>;s=<RawPath>"`) and get `Good`.
|
||||
5. Confirm from the central node's logs that the native runtime served it. Add one `LogDebug("AbCip transport={Transport}")` at driver init if no log line distinguishes the two.
|
||||
6. Flip back to libplctag, redeploy, and read `Good` again.
|
||||
7. Record the output in the gate-results doc.
|
||||
7. Record the output under the gate-results doc's `## AdminUI /run (T19)` heading only.
|
||||
- [ ] **Inverted-gate check** (memory: "an unchanged observable proves nothing"): with Transport = Native, point the device at a port where no `ab_server` is listening. The read must go **Bad** and the log must show the *native* error text. That proves the switch changes behaviour and isn't inert.
|
||||
|
||||
**Verify:** AdminUI unit tests pass, plus the docker-dev `/run` evidence (CLI output for both transports and the inverted case) is pasted into `docs/plans/2026-10-07-native-ab-transport-gate-results.md`.
|
||||
@@ -2504,15 +2677,16 @@ git checkout "$ORIG"
|
||||
- `AB_DIFF_ENDPOINT=10.209.70.160:44818 AB_DIFF_FAMILY=CompactLogix AB_DIFF_COUNT_OPENS=1 dotnet test …--filter "FullyQualifiedName~Differential"` passes.
|
||||
- Browse parity holds: the same discovered tag set (the 2026-10-07 run saw ~23,412 variables across 80 programs) and identical UDT shapes.
|
||||
- Read parity holds over `AB_DIFF_MAX_TAGS=200`, with zero unexpected divergences. Every `TypeMismatch` expected-divergence is listed by tag name.
|
||||
- Native made exactly **1** Forward Open over the run.
|
||||
- Native made exactly **1** Forward Open over the run. That is achievable only because T18's harness holds an anchor lease for the whole run; if the count is higher, the harness is broken, not the press.
|
||||
- [ ] **Latency comparison** (`PollLatencyComparisonTests`, env-gated by the same variables):
|
||||
- Read a fixed set of 100 atomic tags 30 times per transport, sequentially; transports are never concurrent.
|
||||
- Read a fixed set of 100 atomic tags 30 times per transport, sequentially; transports are never concurrent. The native side runs under an anchor lease, as in T18.
|
||||
- Native passes issue all 100 reads **concurrently** (`Task.WhenAll`), so the shared scheduler can MSP-pack them. libplctag passes do the same; libplctag packs internally.
|
||||
- Record the CIP round-trips per pass (native: `CipConnection.RequestsSent` from Task 9, delta per pass; libplctag: report n/a) and the p50/p95 wall time per pass.
|
||||
- Pass criterion: native p95 ≤ libplctag p95 × 1.10.
|
||||
- [ ] **No writes:**
|
||||
- Evidence: the Task 18 source guard passes.
|
||||
- `grep -c WriteAsync` over both differential test files returns 0.
|
||||
- [ ] **Recorded results:** the results doc carries the raw `dotnet test` output, the divergence table, the latency table, the Forward Open count, the date/time (UTC) of the run and the press ID.
|
||||
- [ ] **Recorded results:** under the `## Real press (T21)` and `## Latency (T21)` headings, the results doc carries the raw `dotnet test` output, the divergence table, the latency table, the Forward Open count, the date/time (UTC) of the run and the press ID.
|
||||
- [ ] **Failure handling:** if any criterion fails, the task stays open. Fix the code (with a pinning test), re-run the ab_server suites (Task 17) and the press run. **Do not** relax the criterion.
|
||||
|
||||
**Verify:** `AB_DIFF_ENDPOINT=10.209.70.160:44818 AB_DIFF_FAMILY=CompactLogix AB_DIFF_COUNT_OPENS=1 dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests --filter "FullyQualifiedName~Differential" --logger "console;verbosity=detailed"` → passed, 0 unexpected divergences, `ForwardOpenCount=1`, and native p95 ≤ 1.10 × libplctag p95.
|
||||
@@ -2581,4 +2755,6 @@ git checkout "$ORIG"
|
||||
- **Deviations from the spec:**
|
||||
- `MaxInFlight` is fixed at 1 (see Global Constraints).
|
||||
- `LibplctagTagEnumerator` is renamed `SymbolListingEnumerator` (T13), because its decode loop is transport-neutral.
|
||||
- `AbCipTagCreateParams` gains `AuthoredType` (T12) for the type check.
|
||||
- `AbCipTagCreateParams` gains `AuthoredType` and `UseConnectedMessaging` (T12).
|
||||
- Explicit `CipRouting` modes (T9): the spec assumed only connected-or-UCMM, but libplctag's PCCC framing needs a third, `Direct`.
|
||||
- **Fable review (2026-10-07)** findings B1–B3 and 4–22 are folded in: the scheduler is per connection, caller cancellation is non-faulting, routing modes, the T5←T4 and T19←T17 dependencies, the narrowed guard regex, the 0x08 Large-FO fallback, keep-alive, the anchor lease, registration disposal, the BOOL 0xD3 exemption, PCCC chunking, `FromCip` parity, fixture locations, fragment continuation, the open-pair race, extra test files, the profile-driven routing flag, reinit semantics, the AbLegacy matrix filter, per-task models, and the bounded lease dispose.
|
||||
|
||||
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user