diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs index f05b5164..87a630be 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs @@ -122,4 +122,19 @@ public sealed record AbCipTagCreateParams( bool AllowPacking = true, int ConnectionSize = 4002, int ElementCount = 1, - bool IsArray = false); + bool IsArray = false) +{ + /// + /// The value for libplctag's gateway attribute: host:port, or bare host when + /// no usable port was configured (libplctag then applies its own 44818 default). + /// + /// + /// ⚠️ used to be carried here and never consumed (Gitea #534, found in + /// the AbLegacy driver, which had the identical defect). The runtime passed the bare host to + /// libplctag, which silently substitutes 44818 — so an authored port did not merely fail to apply, + /// it redirected the connection to whatever answers on the default port, which on a plant network + /// is another PLC. This driver's own tests never caught it because its fixture sits on 44818, where + /// the bug is invisible by construction. + /// + public string LibplctagGateway => Port > 0 ? $"{Gateway}:{Port}" : Gateway; +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs index 9584fa06..11ef9b26 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs @@ -19,7 +19,9 @@ internal sealed class LibplctagTagRuntime : IAbCipTagRuntime { _tag = new Tag { - Gateway = p.Gateway, + // host:port — see AbLegacyTagCreateParams/AbCipTagCreateParams.LibplctagGateway. + // Passing p.Gateway alone drops the authored port onto libplctag's 44818 default (#534). + Gateway = p.LibplctagGateway, Path = p.CipPath, PlcType = MapPlcType(p.LibplctagPlcAttribute), Protocol = Protocol.ab_eip, diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/IAbLegacyTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/IAbLegacyTagRuntime.cs index 6bc3dfa7..9f4872d6 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/IAbLegacyTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/IAbLegacyTagRuntime.cs @@ -79,4 +79,19 @@ public sealed record AbLegacyTagCreateParams( string LibplctagPlcAttribute, string TagName, TimeSpan Timeout, - int ElementCount = 1); + int ElementCount = 1) +{ + /// + /// The value for libplctag's gateway attribute: host:port, or bare host when + /// no usable port was configured (libplctag then applies its own 44818 default). + /// + /// + /// ⚠️ used to be carried here and never consumed (Gitea #534). The runtime + /// passed the bare host to libplctag, which silently substitutes 44818 — so an authored port was + /// not merely ignored, it redirected the connection to whatever answers on the default port. On the + /// test rig that was a different simulator of a different PLC family, which is how the AbLegacy + /// integration suite failed for years while looking like a fixture problem. On a plant network it + /// would be another PLC. + /// + public string LibplctagGateway => Port > 0 ? $"{Gateway}:{Port}" : Gateway; +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs index 72ce1c57..fb71ce32 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs @@ -18,7 +18,9 @@ internal sealed class LibplctagLegacyTagRuntime : IAbLegacyTagRuntime { _tag = new Tag { - Gateway = p.Gateway, + // host:port — see AbLegacyTagCreateParams/AbCipTagCreateParams.LibplctagGateway. + // Passing p.Gateway alone drops the authored port onto libplctag's 44818 default (#534). + Gateway = p.LibplctagGateway, Path = p.CipPath, PlcType = MapPlcType(p.LibplctagPlcAttribute), Protocol = Protocol.ab_eip, // PCCC-over-EIP; libplctag routes via the PlcType-specific PCCC layer diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipGatewayCompositionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipGatewayCompositionTests.cs new file mode 100644 index 00000000..017e60b0 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipGatewayCompositionTests.cs @@ -0,0 +1,44 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Gitea #534 — the authored EtherNet/IP port must reach libplctag's gateway attribute. +/// +/// +/// This driver carried the identical defect to AbLegacy, where it was found: +/// was on the record and consumed by nobody, so the runtime +/// handed libplctag the bare host and libplctag substituted 44818. It survived here precisely because +/// it is invisible here — every AbCip fixture profile binds 44818, so no test this driver has could +/// distinguish "uses the authored port" from "always uses 44818". Worth remembering as a shape: a +/// fixture that only ever exercises the default value cannot falsify a rule about non-default values, +/// however many tests point at it. +/// +public sealed class AbCipGatewayCompositionTests +{ + private static AbCipTagCreateParams Params(string host, int port) => + new(Gateway: host, Port: port, CipPath: "1,0", LibplctagPlcAttribute: "controllogix", + TagName: "TestDINT", Timeout: TimeSpan.FromSeconds(5)); + + [Fact] + public void The_authored_port_is_appended_to_the_gateway() + { + Params("10.100.0.35", 44900).LibplctagGateway.ShouldBe("10.100.0.35:44900"); + } + + [Fact] + public void The_default_port_is_still_written_explicitly() + { + Params("plc1.plant.local", 44818).LibplctagGateway.ShouldBe("plc1.plant.local:44818"); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void A_missing_port_falls_back_to_the_bare_host(int port) + { + Params("10.100.0.35", port).LibplctagGateway.ShouldBe("10.100.0.35"); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyGatewayCompositionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyGatewayCompositionTests.cs new file mode 100644 index 00000000..062f5360 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyGatewayCompositionTests.cs @@ -0,0 +1,54 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests; + +/// +/// Gitea #534 — the authored EtherNet/IP port must reach libplctag's gateway attribute. +/// +/// +/// +/// was carried on the record and consumed by nobody: +/// the runtime set Tag.Gateway = p.Gateway, the bare host. libplctag does not fail on a +/// portless gateway — it substitutes 44818. So the authored port was not ignored in the harmless +/// sense; the connection was silently *redirected* to whatever answers on the default port. On the +/// test rig that was a different simulator running a different PLC family, which is why the +/// AbLegacy integration suite failed in a way that looked like a missing fixture. On a plant +/// network it is another PLC, and the driver would read someone else's tags and report them Good. +/// +/// +/// The real guard is the integration suite, which now addresses each PCCC family on its own +/// non-default port (44819/44820/44821) and therefore cannot pass unless the port is honoured. +/// These unit tests pin the composition itself so the rule survives on a machine with no fixtures. +/// +/// +public sealed class AbLegacyGatewayCompositionTests +{ + private static AbLegacyTagCreateParams Params(string host, int port) => + new(Gateway: host, Port: port, CipPath: "1,0", LibplctagPlcAttribute: "slc500", + TagName: "N7:0", Timeout: TimeSpan.FromSeconds(5)); + + [Fact] + public void The_authored_port_is_appended_to_the_gateway() + { + Params("10.100.0.35", 44819).LibplctagGateway.ShouldBe("10.100.0.35:44819"); + } + + [Fact] + public void The_default_port_is_still_written_explicitly() + { + // Not an optimisation opportunity: emitting the port unconditionally is what makes the value + // independent of libplctag's own default, so a change to that default cannot move our traffic. + Params("plc1.plant.local", 44818).LibplctagGateway.ShouldBe("plc1.plant.local:44818"); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void A_missing_port_falls_back_to_the_bare_host(int port) + { + // An unset port must not produce "host:0", which libplctag would take literally. + Params("10.100.0.35", port).LibplctagGateway.ShouldBe("10.100.0.35"); + } +}