fix(abcip): CompactLogix native default connection size matches libplctag's Large Forward Open (found by T21 latency harness)
This commit is contained in:
Joseph Doherty
@@ -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**.
|
||||
|
||||
@@ -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) |
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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.");
|
||||
|
||||
/// <summary>Gets the Micro800 profile.</summary>
|
||||
public static readonly AbServerProfile Micro800 = new(
|
||||
|
||||
@@ -202,4 +202,26 @@ public sealed class AbCipDriverProbeTests
|
||||
result.Ok.ShouldBeTrue();
|
||||
result.Message!.ShouldContain("probe tag not found");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
[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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
|
||||
|
||||
+23
@@ -117,6 +117,29 @@ public sealed class AbCipPerDeviceConnectionOptionsTests
|
||||
factory.Tags["Speed"].CreationParams.ConnectionSize.ShouldBe(4002);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// T21: a CompactLogix device with no override opens Large (4002), matching libplctag's
|
||||
/// <c>plc=compactlogix</c>; the per-device override still wins.
|
||||
/// </summary>
|
||||
[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);
|
||||
}
|
||||
|
||||
/// <summary>Verifies that AllowPacking and ConnectionSize round-trip correctly through JSON parsing.</summary>
|
||||
[Fact]
|
||||
public void AbCipDriverFactoryExtensions_ParseOptions_round_trips_AllowPacking_and_ConnectionSize()
|
||||
|
||||
@@ -40,22 +40,26 @@ public sealed class AbCipPlcFamilyTests
|
||||
|
||||
// ---- CompactLogix ----
|
||||
|
||||
/// <summary>Verifies that the CompactLogix profile uses a narrower connection size than ControlLogix.</summary>
|
||||
/// <summary>
|
||||
/// Verifies that the CompactLogix profile defaults to a Large Forward Open (4002), matching what
|
||||
/// libplctag opens for <c>plc=compactlogix</c>. A target that refuses Large falls back to a regular
|
||||
/// 504 open inside CipConnection (pinned by the Ab.Transport CipConnectionTests fallback tests).
|
||||
/// </summary>
|
||||
[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);
|
||||
}
|
||||
|
||||
/// <summary>Verifies that a CompactLogix device initializes with a narrow connection size.</summary>
|
||||
/// <summary>Verifies that a CompactLogix device initializes with the Large connection size libplctag opens (T21).</summary>
|
||||
[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);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user