diff --git a/docs/drivers/AbCip.md b/docs/drivers/AbCip.md index 156837fd..f273e9d5 100644 --- a/docs/drivers/AbCip.md +++ b/docs/drivers/AbCip.md @@ -115,7 +115,10 @@ The driver has two interchangeable wire transports, selected by one `DriverConfi - **Forward Open throttle:** at most one attempt in flight, at least 1 s between starts, bounded by the open budget. - `ConnectionSize` / `Timeout` are honoured per pooled connection, and the **first** driver/device to open - a given device key wins. + a given device key wins. ControlLogix, GuardLogix and (since T21) CompactLogix default to 4002, a Large + Forward Open (0x5B), the same as libplctag. A target that refuses Large gets one regular open at + min(size, 504) inside the same throttled attempt. An explicit `ConnectionSize` in the options or on the + device overrides the family default. - **Routing:** Micro800 is connected with an empty route regardless of `UseConnectedMessaging`. SLC500/MicroLogix/PLC-5 over this driver are direct. `logixpccc` uses UnconnectedSend; its SLC framing is **unverified**. diff --git a/docs/drivers/AbServer-Test-Fixture.md b/docs/drivers/AbServer-Test-Fixture.md index 1b9ec305..8c678967 100644 --- a/docs/drivers/AbServer-Test-Fixture.md +++ b/docs/drivers/AbServer-Test-Fixture.md @@ -116,14 +116,14 @@ Only the `_S`-suffix naming classifier (PR 12, `SecurityClassification.ViewOnly` forced on safety tags) runs. Actual safety-partition write rejection — what happens when a non-safety write lands on a real `1756-L8xS` — is not exercised. -### 5. CompactLogix narrow ConnectionSize cap +### 5. CompactLogix connection size and the regular-open fallback -CompactLogix profile `Notes`: *"ab_server lacks the narrower limit itself."* - -Driver-side `AbCipPlcFamilyProfile` caps `ConnectionSize` at the CompactLogix -value per PR 10, but `ab_server` accepts whatever the client asks for — the -cap's correctness is trusted from its unit test, never stressed against a -simulator that rejects oversized requests. +Since T21 the CompactLogix profile defaults to a **Large** Forward Open (4002), matching what libplctag +opens for `plc=compactlogix`. The old 504 default was cosmetic under libplctag, which never received it, +and cost the native transport ~3× the round trips per poll. A narrower 5069-L3x / pre-v20 target that refuses +the Large open is handled by `CipConnection`'s fallback (a regular open at min(size, 504) in the same +throttled attempt). That fallback is pinned by unit tests against a fake target only: `ab_server` accepts +any size, so a refusing controller is never exercised here. ### 6. BOOL-within-DINT read-modify-write (#181) @@ -168,7 +168,7 @@ skip cleanly + the ab_server Docker fixture runs as usual. | `@tags` Symbol Object walk with `Program:` scope | partial | **yes** | yes | | Add-On Instructions | no | **yes** | yes | | GuardLogix safety-partition write rejection | no | **yes** (Emulate 5580) | yes | -| CompactLogix narrow ConnectionSize enforcement | no | **yes** (5370 firmware) | yes | +| CompactLogix Large-open refusal → regular-open fallback | no | **yes** (5370 firmware) | yes | | EtherNet/IP embedded-switch behaviour | no | no | yes | | Redundant chassis failover (1756-RM) | no | no | yes | | Motion control timing | no | no | yes | @@ -208,7 +208,7 @@ project is authored. | "Do ALMD alarms raise + clear?" | no | yes | **yes** | yes | | "Is Micro800 unconnected-only enforced wire-side?" | partial (--plc=Micro800 boots, but wire rejection untested) | partial | yes | yes (required) | | "Does GuardLogix reject non-safety writes on safety tags?" | no | no | yes (Emulate 5580) | yes | -| "Does CompactLogix refuse oversized ConnectionSize?" | no | partial | yes (5370 firmware) | yes | +| "Does CompactLogix refuse a Large Forward Open?" | no | partial | yes (5370 firmware) | yes | | "Does BOOL-in-DINT RMW race against concurrent writers?" | no | yes | partial | yes (stress) | | "Does EtherNet/IP embedded-switch behave correctly?" | no | no | no | yes (required) | | "Does redundant-chassis failover work?" | no | no | no | yes (required) | diff --git a/docs/plans/2026-10-07-native-ab-transport-gate-results.md b/docs/plans/2026-10-07-native-ab-transport-gate-results.md index c6bf5d33..1489e17a 100644 --- a/docs/plans/2026-10-07-native-ab-transport-gate-results.md +++ b/docs/plans/2026-10-07-native-ab-transport-gate-results.md @@ -199,9 +199,31 @@ ab_server has 7 atomic scalars, so the 100-tag set was **repeated cyclically** ( | CompactLogix (504, the press default) | 7.7 / 10.9 | 22.1 / 25.3 | 6 | **FAIL** | | ControlLogix (4002) | 7.3 / 9.1 | 7.7 / 9.2 | 2 | pass | -The CompactLogix result is a real finding, not a harness fault: native opens a 504-byte connection there, while libplctag -opens a Large one (see the T18 capture above), so native needs 3× the round trips per pass. The T21 press result is -recorded under `## Latency (T21)` and is not filled in here. +The CompactLogix result was a real finding, not a harness fault. Native opened a 504-byte connection there, while libplctag +opened a Large one (see the T18 capture above), so native needed 3× the round trips per pass. + +**Fixed:** the CompactLogix profile now defaults to 4002, so native opens Large like libplctag. A refusal still falls back to +a regular 504 open. Re-run 2026-10-07 23:30–23:32Z against the `compactlogix` compose profile on :44818 (3 distinct atomic +tags, repeated to 100), with a capture in the container's network namespace: + +| Run | libplctag p50 / p95 ms | native p50 / p95 ms | native req/pass | Criterion | +|---|---|---|---|---| +| 1 | — | — | — | aborted in the latency libplctag phase (see below) | +| 2 | 7.6 / 9.6 | 7.5 / 10.8 | 2 | FAIL (limit 10.6) | +| 3 | 7.8 / 9.3 | 8.1 / 9.4 | 2 | PASS (limit 10.2) | +| 4 | 7.7 / 10.0 | 7.8 / 11.7 | 2 | FAIL (limit 11.1) | +| 5 | — | — | — | aborted in the latency libplctag phase | + +The structural gap is gone: p50s are equal, and both transports now send 2 requests per pass. At ~10 ms the p95 margin is about +1 ms, which is within this network path's jitter, so the ab_server verdict varies from run to run. + +Capture: every session opened with **0x5B (Large Forward Open)** and was accepted (0xDB, status 0). That covers 15 sessions: +four per complete invocation and three in the aborted run 5. Each session had one Forward Close (0x4E to Connection Manager 6/1). +There were **zero** 0x4D/0x53/0x4B. Both aborts (runs 1 and 5) were the simulator: libplctag's ~2 KB packed request was split +across TCP segments (1448 + 606), and ab_server answered with FIN. The harness aborted on the first connection-level failure, +as designed. A real controller reassembles TCP; ab_server does not. + +The T21 press result is recorded under `## Latency (T21)` and is not filled in here. ## AdminUI /run (T19) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/PlcFamilies/AbCipPlcFamilyProfile.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/PlcFamilies/AbCipPlcFamilyProfile.cs index acaede0d..5d58e477 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/PlcFamilies/AbCipPlcFamilyProfile.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/PlcFamilies/AbCipPlcFamilyProfile.cs @@ -41,7 +41,12 @@ public sealed record AbCipPlcFamilyProfile( public static readonly AbCipPlcFamilyProfile CompactLogix = new( LibplctagPlcAttribute: "compactlogix", - DefaultConnectionSize: 504, // 5069-L3x narrower buffer; safe baseline that never over-shoots + // Large Forward Open, matching libplctag: `plc=compactlogix` opens Large (0x5B, 4002) and the + // native transport must not pack into a third of the window (T21 latency harness: 504 cost ~3x + // the round trips per poll). A narrower 5069-L3x / pre-v20 target that refuses the Large open + // is covered by CipConnection's fallback — a regular open at min(size, 504) inside the same + // throttled attempt. An explicit ConnectionSize (driver options or per device) still wins. + DefaultConnectionSize: 4002, DefaultCipPath: "1,0", SupportsRequestPacking: true, SupportsConnectedMessaging: true, diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/AbServerProfile.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/AbServerProfile.cs index 066bfd50..28d950d1 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/AbServerProfile.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/AbServerProfile.cs @@ -44,7 +44,7 @@ public static class KnownProfiles public static readonly AbServerProfile CompactLogix = new( Family: AbCipPlcFamily.CompactLogix, ComposeProfile: "compactlogix", - Notes: "ab_server doesn't enforce the narrower ConnectionSize; driver-side profile caps it per PR 10."); + Notes: "Large Forward Open (4002) like libplctag since T21; ab_server accepts any size, so the regular-open fallback is unit-tested only."); /// Gets the Micro800 profile. public static readonly AbServerProfile Micro800 = new( diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverProbeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverProbeTests.cs index 90c4f4cf..db596cc3 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverProbeTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverProbeTests.cs @@ -202,4 +202,26 @@ public sealed class AbCipDriverProbeTests result.Ok.ShouldBeTrue(); result.Message!.ShouldContain("probe tag not found"); } + + /// + /// T21: the probe builds its handle from the family profile, so a CompactLogix device with no + /// ConnectionSize opens Large (4002) like libplctag; an explicit device ConnectionSize still wins. + /// + [Theory] + [InlineData("", 4002)] + [InlineData(",\"connectionSize\":504", 504)] + public async Task CompactLogix_probe_uses_the_profile_default_unless_the_device_overrides_it(string extra, int expected) + { + using var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + var factory = new FakeAbCipTagFactory(); + + await new AbCipDriverProbe(factory).ProbeAsync( + $"{{\"devices\":[{{\"hostAddress\":\"ab://127.0.0.1:{port}/1,0\",\"plcFamily\":\"CompactLogix\"{extra}}}]}}", + TimeSpan.FromSeconds(3), + TestContext.Current.CancellationToken); + + factory.Tags.Values.ShouldHaveSingleItem().CreationParams.ConnectionSize.ShouldBe(expected); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverTests.cs index 235e4390..2562cb06 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverTests.cs @@ -106,7 +106,7 @@ public sealed class AbCipDriverTests AbCipPlcFamilyProfile.Micro800.SupportsRequestPacking.ShouldBeFalse(); AbCipPlcFamilyProfile.Micro800.SupportsConnectedMessaging.ShouldBeFalse(); - AbCipPlcFamilyProfile.CompactLogix.DefaultConnectionSize.ShouldBe(504); + AbCipPlcFamilyProfile.CompactLogix.DefaultConnectionSize.ShouldBe(4002); // Large, as libplctag opens it AbCipPlcFamilyProfile.GuardLogix.LibplctagPlcAttribute.ShouldBe("controllogix"); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipPerDeviceConnectionOptionsTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipPerDeviceConnectionOptionsTests.cs index 78077e41..04b90233 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipPerDeviceConnectionOptionsTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipPerDeviceConnectionOptionsTests.cs @@ -117,6 +117,29 @@ public sealed class AbCipPerDeviceConnectionOptionsTests factory.Tags["Speed"].CreationParams.ConnectionSize.ShouldBe(4002); } + /// + /// T21: a CompactLogix device with no override opens Large (4002), matching libplctag's + /// plc=compactlogix; the per-device override still wins. + /// + [Theory] + [InlineData(null, 4002)] + [InlineData(504, 504)] + public async Task CompactLogix_ConnectionSize_defaults_to_large_unless_overridden(int? overrideSize, int expected) + { + var factory = new FakeAbCipTagFactory(); + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(Device, AbCipPlcFamily.CompactLogix, ConnectionSize: overrideSize)], + RawTags = AbCipRawTags.From(new AbCipTagDefinition("Speed", Device, "Speed", AbCipDataType.DInt)), + Probe = new AbCipProbeOptions { Enabled = false }, + }, "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.ReadAsync(["Speed"], CancellationToken.None); + + factory.Tags["Speed"].CreationParams.ConnectionSize.ShouldBe(expected); + } + /// Verifies that AllowPacking and ConnectionSize round-trip correctly through JSON parsing. [Fact] public void AbCipDriverFactoryExtensions_ParseOptions_round_trips_AllowPacking_and_ConnectionSize() diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipPlcFamilyTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipPlcFamilyTests.cs index 13acd500..f42199b3 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipPlcFamilyTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipPlcFamilyTests.cs @@ -40,22 +40,26 @@ public sealed class AbCipPlcFamilyTests // ---- CompactLogix ---- - /// Verifies that the CompactLogix profile uses a narrower connection size than ControlLogix. + /// + /// Verifies that the CompactLogix profile defaults to a Large Forward Open (4002), matching what + /// libplctag opens for plc=compactlogix. A target that refuses Large falls back to a regular + /// 504 open inside CipConnection (pinned by the Ab.Transport CipConnectionTests fallback tests). + /// [Fact] - public void CompactLogix_profile_uses_narrower_connection_size() + public void CompactLogix_profile_defaults_to_a_large_forward_open_like_libplctag() { var p = AbCipPlcFamilyProfile.CompactLogix; p.LibplctagPlcAttribute.ShouldBe("compactlogix"); - p.DefaultConnectionSize.ShouldBe(504); // 5069-L3x narrow-window safety + p.DefaultConnectionSize.ShouldBe(4002); // T21: 504 cost the native transport ~3x the round trips per poll p.DefaultCipPath.ShouldBe("1,0"); p.SupportsRequestPacking.ShouldBeTrue(); p.SupportsConnectedMessaging.ShouldBeTrue(); p.MaxFragmentBytes.ShouldBe(500); } - /// Verifies that a CompactLogix device initializes with a narrow connection size. + /// Verifies that a CompactLogix device initializes with the Large connection size libplctag opens (T21). [Fact] - public async Task CompactLogix_device_initialises_with_narrow_ConnectionSize() + public async Task CompactLogix_device_initialises_with_large_ConnectionSize() { var drv = new AbCipDriver(new AbCipDriverOptions { @@ -65,7 +69,7 @@ public sealed class AbCipPlcFamilyTests await drv.InitializeAsync("{}", CancellationToken.None); var profile = drv.GetDeviceState("ab://192.168.1.10/1,0")!.Profile; - profile.DefaultConnectionSize.ShouldBeLessThan(AbCipPlcFamilyProfile.ControlLogix.DefaultConnectionSize); + profile.DefaultConnectionSize.ShouldBe(4002); profile.MaxFragmentBytes.ShouldBeLessThan(AbCipPlcFamilyProfile.ControlLogix.MaxFragmentBytes); }