diff --git a/docs/drivers/AbLegacy.md b/docs/drivers/AbLegacy.md index c445a270..88ae97a6 100644 --- a/docs/drivers/AbLegacy.md +++ b/docs/drivers/AbLegacy.md @@ -100,7 +100,7 @@ physical inputs and may reject the write. re-parses carry identical numbers. This is not a double conversion: libplctag's `pccc.c` (`parse_pccc_elem_num` / `parse_pccc_bit_num`) parses decimal numerals only (a FIXME there says octal is unsupported), so the driver must convert. Pinned by `Plc5_io_indices_are_octal` (`I:017/17` -> `I:15/15`). - The connectivity-probe address is normalised the same way. + The connectivity-probe address is normalised the same way — by both the driver's probe loop and the Test Connect probe, through one helper (`AbLegacyProbeAddress.Normalise`, #660); an unparseable Test Connect probe address fails cleanly ("not a valid AbLegacy address") instead of reaching the transport. Normalisation yields the canonical name; the libplctag `S:0` → `S2:0` translation (#659) happens downstream in the runtime, so the two compose. - ⚠️ **Behaviour change / migration.** Before this change a PLC-5 `I:`/`O:` address was passed to libplctag as written, i.e. read as **decimal**. An address copied from RSLogix in octal (e.g. `I:017`) therefore read the wrong word, and an authored decimal such as `I:1/12` now means bit 10 (octal 12). A previously "working" diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs index 4afe4fe7..a352a803 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs @@ -693,7 +693,7 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover LibplctagPlcAttribute: state.Profile.LibplctagPlcAttribute, // Normalised exactly like tag paths (PLC-5 octal I/O -> decimal); an unparseable probe address is passed through // so the transport reports the failure as it always did. - TagName: AbLegacyAddress.TryParse(_options.Probe.ProbeAddress, state.Options.PlcFamily)?.ToLibplctagName() + TagName: AbLegacyProbeAddress.Normalise(_options.Probe.ProbeAddress, state.Options.PlcFamily) ?? _options.Probe.ProbeAddress!, Timeout: _options.Probe.Timeout); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverProbe.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverProbe.cs index d1b05167..6d5eed4b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverProbe.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverProbe.cs @@ -98,12 +98,18 @@ public sealed class AbLegacyDriverProbe : IDriverProbe ?? FirstProbeableTagAddress(opts, firstDevice) ?? "S:0"; + // Normalise through the same helper as the driver's probe loop (#660): PLC-5 octal I/O -> decimal. + // An unparseable address is a clean failure here — the factory would otherwise throw outside the try. + var normalisedName = AbLegacyProbeAddress.Normalise(tagName, firstDevice?.PlcFamily ?? AbLegacyPlcFamily.Slc500); + if (normalisedName is null) + return new(false, $"Probe address '{tagName}' is not a valid AbLegacy address for this PLC family.", null); + var p = new AbLegacyTagCreateParams( Gateway: parsed.Gateway, Port: parsed.Port, CipPath: parsed.CipPath, LibplctagPlcAttribute: profile.LibplctagPlcAttribute, - TagName: tagName, + TagName: normalisedName, Timeout: timeout); var factory = _tagFactory ?? (AbLegacyTransportResolver.Resolve(opts) == AbLegacyTransport.Native diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyProbeAddress.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyProbeAddress.cs new file mode 100644 index 00000000..93f192de --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyProbeAddress.cs @@ -0,0 +1,16 @@ +using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.PlcFamilies; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; + +/// +/// The single authority that turns an operator-authored probe address into the name handed to the tag +/// factory: family-aware parse (PLC-5 I:/O: octal indices become decimal) then +/// — the same normalisation tag paths get. Shared by the +/// driver's probe loop and the Test Connect probe (#660). +/// +internal static class AbLegacyProbeAddress +{ + /// Returns the normalised name, or null when does not parse for the family. + public static string? Normalise(string? address, AbLegacyPlcFamily family) => + AbLegacyAddress.TryParse(address, family)?.ToLibplctagName(); +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyDriverProbeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyDriverProbeTests.cs index 1cf1591b..f58536d7 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyDriverProbeTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyDriverProbeTests.cs @@ -5,6 +5,7 @@ using System.Net; using System.Net.Sockets; using Shouldly; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using Xunit; namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests; @@ -147,4 +148,56 @@ public sealed class AbLegacyDriverProbeTests $"Expected 'timed out' or 'Connect failed' but got: {result.Message}"); result.Latency.ShouldBeNull(); } + + // ---- #660: Test Connect normalises the probe address exactly like the driver's probe loop ---- + + private static async Task<(DriverProbeResult Result, string? SeenName)> ProbeWithFakeAsync(string family, string? probeAddress) + { + var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + try + { + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + var factory = new FakeAbLegacyTagFactory(); + var probe = new AbLegacyDriverProbe(factory); + var addr = probeAddress is null ? "" : $",\"probe\":{{\"probeAddress\":\"{probeAddress}\"}}"; + var json = $"{{\"devices\":[{{\"hostAddress\":\"ab://127.0.0.1:{port}/1,0\",\"plcFamily\":\"{family}\"}}]{addr}}}"; + var result = await probe.ProbeAsync(json, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); + return (result, factory.Tags.Keys.SingleOrDefault()); + } + finally { listener.Stop(); } + } + + [Theory] + [InlineData("Plc5", "I:017", "I:15")] + [InlineData("Plc5", "O:010/07", "O:8/7")] + [InlineData("LogixPccc", "I:017", "I:17")] + [InlineData("Slc500", "I:017", "I:17")] + [InlineData("MicroLogix", "I:017", "I:17")] + [InlineData("Slc500", "N7:0", "N7:0")] + [InlineData("Slc500", null, "S:0")] + public async Task Probe_address_reaches_the_factory_family_normalised(string family, string? configured, string expected) + { + var (result, seen) = await ProbeWithFakeAsync(family, configured); + result.Ok.ShouldBeTrue(result.Message); + seen.ShouldBe(expected); + } + + [Fact] + public async Task Normalised_default_probe_then_composes_with_the_libplctag_translation() + { + var (_, seen) = await ProbeWithFakeAsync("Plc5", "I:017"); + LibplctagLegacyName.Translate(seen!).ShouldBe("I1:15"); + var (_, status) = await ProbeWithFakeAsync("Slc500", null); + LibplctagLegacyName.Translate(status!).ShouldBe("S2:0"); + } + + [Fact] + public async Task Unparseable_probe_address_is_a_clean_failure_not_an_exception() + { + var (result, seen) = await ProbeWithFakeAsync("Slc500", "not-an-address"); + result.Ok.ShouldBeFalse(); + result.Message!.ShouldContain("not-an-address"); + seen.ShouldBeNull(); + } }