From d8f3dd739448797bbf34f54fa625d495d7998849 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 14:38:25 -0400 Subject: [PATCH] fix(ablegacy): review fixes - one family-aware address authority in Contracts, deploy gate + editor check, malformed address -> BadNodeIdUnknown warned once, probe normalised, octal/migration docs (#244 #245 #247) Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- docs/drivers/AbLegacy.md | 18 ++++- .../AbLegacyAddress.cs | 39 ++++++++++- .../AbLegacyTagDefinitionFactory.cs | 29 +++++++- .../AbLegacyDriver.cs | 40 ++++++++++- .../Uns/TagEditors/AbLegacyTagConfigModel.cs | 7 +- .../AdminOperations/AdminOperationsActor.cs | 6 +- .../EquipmentTagConfigInspector.cs | 42 ++++++++++++ .../AbLegacyFamilyAddressingTests.cs | 68 +++++++++++++++++++ .../Uns/AbLegacyFamilyAddressGateTests.cs | 55 +++++++++++++++ .../Uns/RawTagConfigGateTests.cs | 2 +- 10 files changed, 291 insertions(+), 15 deletions(-) rename src/Drivers/{ZB.MOM.WW.OtOpcUa.Driver.AbLegacy => ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts}/AbLegacyAddress.cs (85%) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/AbLegacyFamilyAddressGateTests.cs diff --git a/docs/drivers/AbLegacy.md b/docs/drivers/AbLegacy.md index fbe030e6..09b46101 100644 --- a/docs/drivers/AbLegacy.md +++ b/docs/drivers/AbLegacy.md @@ -97,9 +97,21 @@ physical inputs and may reject the write. bit 7 and `I:017/17` is bit 15. Only `I`/`O` on PLC-5 (`OctalIoAddressing` on the family profile) — SLC, MicroLogix and Logix-PCCC stay decimal, and PLC-5 `N`/`B`/... files stay decimal. The record stores decimal and `ToLibplctagName()` **emits decimal**, so the libplctag `name=` string and the string the native transport - re-parses carry identical numbers. ⚠️ *Unverified:* whether libplctag itself reads PLC-5 I/O indices as octal - when handed the raw RSLogix text was not checked (no libplctag source/PLC here); the driver therefore never - hands it octal text. Native PLC-5 is N-file read-only anyway, so I/O never reaches the native wire. + 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. + - ⚠️ **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" + address containing the digits 8 or 9 (`I:018`, `O:1/9`) is no longer valid on PLC-5. Re-author any PLC-5 + I/O tags in RSLogix octal. Such a tag is refused (see below), never silently misread. + - **A malformed / family-invalid address** (also: a function-file letter off MicroLogix) reads and writes + `BadNodeIdUnknown` for that tag only, creates no runtime, leaves driver health alone, and logs **one** + Warning per tag. The same check (`AbLegacyAddress.Validate`) runs in the AdminUI tag editor (family-independent: + rejects only what *no* family accepts) and the deploy gate (`EquipmentTagConfigInspector`, using the device's + `DeviceConfig.PlcFamily`, absent = SLC 500, unrecognised = unknown) — so `I:018` on a PLC-5 device warns at + deploy (an error under `TagConfigValidationMode.Error`), and the editor is never stricter than the gate. - **MicroLogix function files (#245).** `RTC`, `HSC`, `DLS`, `MMI`, `PTO`, `PWM`, `STI`, `EII`, `IOS`, `BHI` (single fixed instance — no file number — with a `.SUBELEMENT`; no bit index) parse on MicroLogix devices only. `AbLegacyFunctionFile.SubElementType` carries the sub-element catalogue (flags → Boolean, words → Int32; unknown diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyAddress.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyAddress.cs similarity index 85% rename from src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyAddress.cs rename to src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyAddress.cs index f4363b8c..aac67c41 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyAddress.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyAddress.cs @@ -49,7 +49,10 @@ public sealed record AbLegacyAddress( /// /// Converts the address to the libplctag library address format. Numeric indices are always - /// emitted decimal (a PLC-5 octal I/O address is normalised at parse time). An indirect + /// emitted decimal (a PLC-5 octal I/O address is normalised at parse time). This is correct, not a + /// double conversion: libplctag's pccc.c (parse_pccc_elem_num / parse_pccc_bit_num) parses + /// decimal numerals only (a FIXME there notes octal is unsupported), so handing it the RSLogix octal text would + /// read the wrong word. An indirect /// address re-emits its bracket form, which neither transport accepts — check /// first. /// @@ -86,6 +89,36 @@ public sealed record AbLegacyAddress( return ParseCore(value.Trim(), profile, allowIndirect: true); } + /// + /// The single authoring / deploy-gate / runtime address check. With a known the + /// address must parse for that family; with an unknown one () it must parse for at + /// least one family, so a caller that cannot see the device never rejects what the runtime would accept. + /// + /// The authored address text. + /// The device's PLC family, or when it is not known. + /// An error message, or when the address is acceptable. + public static string? Validate(string? address, AbLegacyPlcFamily? family) + { + if (string.IsNullOrWhiteSpace(address)) return "Address is required."; + if (family is { } f) + { + if (TryParse(address, f) is not null) return null; + var hint = f == AbLegacyPlcFamily.Plc5 && TryParse(address, AbLegacyPlcFamily.Slc500) is not null + ? " (PLC-5 I:/O: word and bit indices are octal - 8 and 9 are invalid)" + : IsFunctionLetterPrefix(address) ? " (function files are MicroLogix-only)" : ""; + return $"'{address}' is not a valid address for a {f} device{hint}."; + } + foreach (var candidate in Enum.GetValues()) + if (TryParse(address, candidate) is not null) return null; + return $"'{address}' is not a valid PCCC file address for any PLC family."; + } + + private static bool IsFunctionLetterPrefix(string address) + { + var colon = address.IndexOf(':'); + return colon > 0 && IsFunctionFileLetter(address[..colon].Trim().ToUpperInvariant()); + } + private static AbLegacyAddress? ParseCore(string src, AbLegacyPlcFamilyProfile? profile, bool allowIndirect) { // BitIndex: trailing /N outside any bracketed inner address. The numeric parse is deferred until the @@ -254,9 +287,9 @@ public sealed record AbLegacyAddress( /// /// MicroLogix 1100/1400 function-file prefixes — each a single fixed instance with a sub-element - /// catalogue (). + /// catalogue (AbLegacyFunctionFile). /// - internal static bool IsFunctionFileLetter(string letter) => letter switch + public static bool IsFunctionFileLetter(string letter) => letter switch { "RTC" or "HSC" or "DLS" or "MMI" or "PTO" or "PWM" or "STI" or "EII" or "IOS" or "BHI" => true, _ => false, diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyTagDefinitionFactory.cs index 79b48a1b..1bbfa9af 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyTagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyTagDefinitionFactory.cs @@ -1,3 +1,4 @@ +using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.PlcFamilies; using System.Text.Json; using System.Text.Json.Nodes; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; @@ -122,6 +123,12 @@ public static class AbLegacyTagDefinitionFactory /// The RawPath handed to during inspection — the mapper only copies it /// onto and never validates it. /// + private static bool IsIndirectFor(string address, AbLegacyPlcFamily? family) + { + if (family is { } f) return AbLegacyAddress.TryParse(address, f)?.IsIndirect == true; + return Enum.GetValues().Any(c => AbLegacyAddress.TryParse(address, c)?.IsIndirect == true); + } + private const string InspectionRawPath = "(inspect)"; /// @@ -145,7 +152,18 @@ public static class AbLegacyTagDefinitionFactory /// /// The equipment tag's TagConfig JSON. /// The warnings; empty when clean. - public static IReadOnlyList Inspect(string reference) + public static IReadOnlyList Inspect(string reference) => Inspect(reference, family: null); + + /// + /// plus the family-aware address check (, + /// the same authority the driver runtime uses). is the tag's device's PLC family, or + /// when unknown (then only an address no family accepts is flagged). An indirect + /// address parses but the runtime always answers it BadNotSupported, so it is warned about too. + /// + /// The equipment tag's TagConfig JSON. + /// The owning device's PLC family, when known. + /// The warnings; empty when clean. + public static IReadOnlyList Inspect(string reference, AbLegacyPlcFamily? family) { var warnings = new List(); // Gitea #556 item 1: the framing check TRIMS FIRST. FromTagConfig hands the blob straight to @@ -157,7 +175,14 @@ public static class AbLegacyTagDefinitionFactory // The accept gate: whatever the driver reads draws nothing. The RAW reference is handed to the // mapper — the driver reads the stored blob, not a trimmed copy of it. - if (FromTagConfig(reference, InspectionRawPath, out _)) return warnings; + if (FromTagConfig(reference, InspectionRawPath, out var accepted)) + { + if (AbLegacyAddress.Validate(accepted.Address, family) is { } addressError) + warnings.Add($"AbLegacy tag address: {addressError} The driver reads this tag as BadNodeIdUnknown."); + else if (IsIndirectFor(accepted.Address, family)) + warnings.Add($"AbLegacy address '{accepted.Address}' is indirect; indirect addressing is not supported and the tag reads BadNotSupported."); + return warnings; + } try { 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 f6315c78..89b78f40 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs @@ -357,7 +357,14 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover { // Indirect addresses (N7:[N7:0]) parse but are not resolved at runtime (#247) — answer the // refusal before a runtime is created for a name neither transport can encode. - if (AbLegacyAddress.TryParse(def.Address, device.Options.PlcFamily) is { IsIndirect: true }) + var preParsed = AbLegacyAddress.TryParse(def.Address, device.Options.PlcFamily); + if (preParsed is null) + { + WarnMalformedOnce(device, def); + results[i] = new DataValueSnapshot(null, AbLegacyStatusMapper.BadNodeIdUnknown, null, now); + continue; + } + if (preParsed.IsIndirect) { results[i] = new DataValueSnapshot(null, AbLegacyStatusMapper.BadNotSupported, null, now); continue; @@ -465,8 +472,16 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover { var parsed = AbLegacyAddress.TryParse(def.Address, device.Options.PlcFamily); + // A malformed address is a config error, not a comms fault (BadNodeIdUnknown, warned once per tag). + if (parsed is null) + { + WarnMalformedOnce(device, def); + results[i] = new WriteResult(AbLegacyStatusMapper.BadNodeIdUnknown); + continue; + } + // Indirect addressing is parse-only (#247): refuse before any runtime or RMW read is created. - if (parsed is { IsIndirect: true }) + if (parsed.IsIndirect) { results[i] = new WriteResult(AbLegacyStatusMapper.BadNotSupported); continue; @@ -625,7 +640,10 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover Port: state.ParsedAddress.Port, CipPath: state.EffectiveCipPath, LibplctagPlcAttribute: state.Profile.LibplctagPlcAttribute, - TagName: _options.Probe.ProbeAddress!, + // 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() + ?? _options.Probe.ProbeAddress!, Timeout: _options.Probe.Timeout); IAbLegacyTagRuntime? probeRuntime = null; @@ -873,6 +891,19 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover keep(); } + /// + /// A tag whose address does not parse for its device's PLC family (e.g. a PLC-5 octal I/O digit 8/9, a + /// function-file letter off MicroLogix) is a configuration error: answered BadNodeIdUnknown with the + /// handle/health untouched, and warned once per tag so a polled tag does not flood the log. + /// + private void WarnMalformedOnce(DeviceState device, AbLegacyTagDefinition def) + { + if (!device.MalformedAddressLogged.TryAdd(def.Name, 0)) return; + _logger.LogWarning( + "AbLegacy tag address is not valid for the device's PLC family. Driver={DriverInstanceId} Device={DeviceHostAddress} Reference={Reference} Address={Address} Family={Family}", + _driverInstanceId, def.DeviceHostAddress, def.Name, def.Address, device.Options.PlcFamily); + } + /// The create parameters for a tag's own runtime — shared by creation and the write-refusal check, so /// the refusal is asked about exactly the runtime a write would otherwise create. private AbLegacyTagCreateParams CreateParamsFor(DeviceState device, AbLegacyTagDefinition def, AbLegacyAddress parsed) => @@ -1105,6 +1136,9 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover /// public bool FirstNonZeroStatusLogged { get; set; } + /// Tags already warned about for a malformed address (once-per-tag log). + public System.Collections.Concurrent.ConcurrentDictionary MalformedAddressLogged { get; } = new(); + /// /// Disposes all cached tag runtimes. /// diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/AbLegacyTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/AbLegacyTagConfigModel.cs index 06d9d5c6..948438cb 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/AbLegacyTagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/AbLegacyTagConfigModel.cs @@ -68,8 +68,11 @@ public sealed class AbLegacyTagConfigModel if (TagConfigJson.DescribeInvalidEnum(_bag, "dataType") is { } dtError) return dtError; - if (string.IsNullOrWhiteSpace(Address)) - return "Address is required."; + // The editor cannot see the device's PLC family, so it checks only what no family accepts (the same + // AbLegacyAddress authority the deploy gate and runtime use) — never stricter than the gate, which adds the + // family-specific verdict (PLC-5 octal I/O, MicroLogix-only function files). + if (AbLegacyAddress.Validate(Address, family: null) is { } addressError) + return addressError; // DELIBERATELY STRICTER THAN THE RUNTIME, and re-confirmed as such by the Gitea #554 survey — do not // "align" it. PCCC data-file elements are limited to AbLegacyArray.MaxElements (256). The reader diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs index 7c5112a6..afcf2e0a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs @@ -262,12 +262,16 @@ public sealed class AdminOperationsActor : ReceiveActor var driverByDevice = draft.Devices .GroupBy(d => d.DeviceId, StringComparer.Ordinal) .ToDictionary(g => g.Key, g => g.First().DriverInstanceId, StringComparer.Ordinal); + var deviceConfigById = draft.Devices + .GroupBy(d => d.DeviceId, StringComparer.Ordinal) + .ToDictionary(g => g.Key, g => g.First().DeviceConfig, StringComparer.Ordinal); var tagConfigWarnings = new List<(string TagId, string Text)>(); foreach (var t in draft.Tags) { if (!driverByDevice.TryGetValue(t.DeviceId, out var driverInstanceId)) continue; if (!driverTypeById.TryGetValue(driverInstanceId, out var driverType)) continue; - foreach (var w in EquipmentTagConfigInspector.Inspect(driverType, t.TagConfig)) + foreach (var w in EquipmentTagConfigInspector.Inspect( + driverType, t.TagConfig, deviceConfigById.GetValueOrDefault(t.DeviceId))) tagConfigWarnings.Add((t.TagId, $"tag '{t.TagId}': {w}")); } if (tagConfigWarnings.Count > 0) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/EquipmentTagConfigInspector.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/EquipmentTagConfigInspector.cs index 57163a36..aa12b674 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/EquipmentTagConfigInspector.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/EquipmentTagConfigInspector.cs @@ -1,6 +1,7 @@ using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Driver.AbCip; using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; +using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.PlcFamilies; using ZB.MOM.WW.OtOpcUa.Driver.Calculation; using ZB.MOM.WW.OtOpcUa.Driver.FOCAS; using ZB.MOM.WW.OtOpcUa.Driver.Modbus; @@ -78,6 +79,47 @@ public static class EquipmentTagConfigInspector return Inspectors.TryGetValue(driverType, out var inspect) ? inspect(tagConfig) : Array.Empty(); } + /// + /// with the owning device's DeviceConfig, so a driver whose + /// address grammar depends on the device (AbLegacy: the PlcFamily) is checked against the family the + /// runtime will use. Other drivers ignore the device config. A missing family key is the runtime default + /// (SLC 500); an unrecognised value is "unknown" and only the family-independent check applies — the gate + /// is never stricter than the runtime. + /// + /// The bound driver's DriverType. + /// The equipment tag's TagConfig JSON. + /// The owning device's DeviceConfig JSON (may be null). + /// The warnings; empty when clean. + public static IReadOnlyList Inspect(string? driverType, string? tagConfig, string? deviceConfig) + { + if (string.IsNullOrEmpty(driverType) || tagConfig is null) return Array.Empty(); + if (string.Equals(driverType, DriverTypeNames.AbLegacy, StringComparison.OrdinalIgnoreCase)) + return AbLegacyTagDefinitionFactory.Inspect(tagConfig, ReadAbLegacyFamily(deviceConfig)); + return Inspect(driverType, tagConfig); + } + + private static AbLegacyPlcFamily? ReadAbLegacyFamily(string? deviceConfig) + { + if (string.IsNullOrWhiteSpace(deviceConfig)) return AbLegacyPlcFamily.Slc500; + try + { + using var doc = System.Text.Json.JsonDocument.Parse(deviceConfig); + if (doc.RootElement.ValueKind != System.Text.Json.JsonValueKind.Object) return AbLegacyPlcFamily.Slc500; + foreach (var prop in doc.RootElement.EnumerateObject()) + { + if (!string.Equals(prop.Name, "PlcFamily", StringComparison.OrdinalIgnoreCase)) continue; + var raw = prop.Value.ValueKind == System.Text.Json.JsonValueKind.String ? prop.Value.GetString() : null; + if (string.IsNullOrWhiteSpace(raw)) return AbLegacyPlcFamily.Slc500; + return Enum.TryParse(raw, ignoreCase: true, out var f) && Enum.IsDefined(f) ? f : null; + } + return AbLegacyPlcFamily.Slc500; + } + catch (System.Text.Json.JsonException) + { + return null; + } + } + /// Whether the given driver type has a mapped inspector. /// The bound driver's DriverType. /// when an inspector is registered for the driver type. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyFamilyAddressingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyFamilyAddressingTests.cs index 40fdd620..595b4797 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyFamilyAddressingTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyFamilyAddressingTests.cs @@ -249,4 +249,72 @@ public sealed class AbLegacyFamilyAddressingTests ex.Status.ShouldBe(AbStatus.Unsupported); ex.Message.ShouldContain(why, Case.Insensitive); } + + private sealed class CapturingLogger : Microsoft.Extensions.Logging.ILogger + { + public readonly System.Collections.Concurrent.ConcurrentQueue Warnings = new(); + public IDisposable? BeginScope(TState state) where TState : notnull => null; + public bool IsEnabled(Microsoft.Extensions.Logging.LogLevel logLevel) => true; + public void Log(Microsoft.Extensions.Logging.LogLevel level, Microsoft.Extensions.Logging.EventId id, + TState state, Exception? ex, Func f) + { + if (level == Microsoft.Extensions.Logging.LogLevel.Warning) Warnings.Enqueue(f(state, ex)); + } + } + + /// + /// A PLC-5 address that no longer parses (the pre-octal-era decimal 8/9 digit) is a config error: BadNodeIdUnknown on + /// read and write, no runtime created, health untouched, and ONE warning per tag however often it is polled. + /// + [Fact] + public async Task Malformed_address_is_BadNodeIdUnknown_warned_once_and_never_throws() + { + var factory = new FakeAbLegacyTagFactory(); + var logger = new CapturingLogger(); + var drv = new AbLegacyDriver(new AbLegacyDriverOptions + { + Devices = [new AbLegacyDeviceOptions("ab://10.0.0.5/1,0", AbLegacyPlcFamily.Plc5)], + RawTags = AbLegacyRawTags.Entries( + new AbLegacyTagDefinition("Bad", "ab://10.0.0.5/1,0", "I:018", AbLegacyDataType.Int, Writable: true), + new AbLegacyTagDefinition("Good", "ab://10.0.0.5/1,0", "N7:1", AbLegacyDataType.Int)), + Probe = new AbLegacyProbeOptions { Enabled = false }, + }, "drv-1", factory, logger); + await drv.InitializeAsync("{}", CancellationToken.None); + factory.Customise = p => new FakeAbLegacyTag(p) { Value = 1 }; + + var first = await drv.ReadAsync(["Bad", "Good"], CancellationToken.None); + var second = await drv.ReadAsync(["Bad"], CancellationToken.None); + var write = await drv.WriteAsync([new WriteRequest("Bad", 1)], CancellationToken.None); + + first[0].StatusCode.ShouldBe(AbLegacyStatusMapper.BadNodeIdUnknown); + second[0].StatusCode.ShouldBe(AbLegacyStatusMapper.BadNodeIdUnknown); + write[0].StatusCode.ShouldBe(AbLegacyStatusMapper.BadNodeIdUnknown); + first[1].StatusCode.ShouldBe(AbLegacyStatusMapper.Good); // positive control: the refusal is per tag + factory.Tags.Keys.ShouldNotContain(k => k.StartsWith("I:")); + logger.Warnings.Count(w => w.Contains("not valid for the device's PLC family") && w.Contains("I:018")).ShouldBe(1); + } + + /// The connectivity-probe address is normalised like tag paths (PLC-5 octal I/O handed over decimal). + [Fact] + public async Task Probe_address_is_normalised_like_tag_paths() + { + var factory = new FakeAbLegacyTagFactory(); + var drv = new AbLegacyDriver(new AbLegacyDriverOptions + { + Devices = [new AbLegacyDeviceOptions("ab://10.0.0.5/1,0", AbLegacyPlcFamily.Plc5)], + Probe = new AbLegacyProbeOptions + { + Enabled = true, Interval = TimeSpan.FromMilliseconds(50), Timeout = TimeSpan.FromMilliseconds(200), + ProbeAddress = "I:017", + }, + }, "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var deadline = DateTime.UtcNow.AddSeconds(5); + while (!factory.Tags.ContainsKey("I:15") && DateTime.UtcNow < deadline) await Task.Delay(20); + await drv.ShutdownAsync(CancellationToken.None); + + factory.Tags.ContainsKey("I:15").ShouldBeTrue(); + factory.Tags.ContainsKey("I:017").ShouldBeFalse(); + } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/AbLegacyFamilyAddressGateTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/AbLegacyFamilyAddressGateTests.cs new file mode 100644 index 00000000..b0404868 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/AbLegacyFamilyAddressGateTests.cs @@ -0,0 +1,55 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors; +using ZB.MOM.WW.OtOpcUa.ControlPlane.AdminOperations; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns; + +/// #244/#245/#247: one address authority (AbLegacyAddress.Validate) behind the editor and the deploy gate. +public sealed class AbLegacyFamilyAddressGateTests +{ + private static string Cfg(string address) => "{\"address\":\"" + address + "\",\"dataType\":\"Int\"}"; + + [Theory] + [InlineData("{\"PlcFamily\":\"Plc5\"}", "I:018", true)] // octal 8 on PLC-5 + [InlineData("{\"PlcFamily\":\"Plc5\"}", "I:017/07", false)] + [InlineData("{\"PlcFamily\":\"Slc500\"}", "I:018", false)] // control: decimal on SLC + [InlineData("{\"plcFamily\":\"plc5\"}", "I:018", true)] // key + value case-insensitive + [InlineData("{\"PlcFamily\":\"Slc500\"}", "RTC:0.HR", true)] // function file off MicroLogix + [InlineData("{\"PlcFamily\":\"MicroLogix\"}", "RTC:0.HR", false)] + [InlineData(null, "RTC:0.HR", true)] // absent family = runtime default SLC 500 + [InlineData("{\"PlcFamily\":\"Bogus\"}", "I:018", false)] // unknown family: never stricter than the runtime + [InlineData("{\"PlcFamily\":\"Slc500\"}", "Q9:0", true)] + public void Gate_checks_the_address_against_the_device_family(string? deviceConfig, string address, bool warns) + { + var w = EquipmentTagConfigInspector.Inspect(DriverTypeNames.AbLegacy, Cfg(address), deviceConfig); + (w.Count > 0).ShouldBe(warns, string.Join("; ", w)); + } + + [Fact] + public void Gate_warns_about_indirect_addresses_which_never_read() + { + EquipmentTagConfigInspector.Inspect(DriverTypeNames.AbLegacy, Cfg("N7:[N7:0]"), "{\"PlcFamily\":\"Slc500\"}") + .ShouldContain(x => x.Contains("indirect")); + } + + [Theory] + [InlineData("N7:0", null)] + [InlineData("I:018", null)] // family unknown to the editor: another family accepts it + [InlineData("RTC:0.HR", null)] // MicroLogix accepts it + [InlineData("Q9:0", "any PLC family")] + public void Editor_validates_only_the_family_independent_shape(string address, string? error) + { + var result = AbLegacyTagConfigModel.FromJson(Cfg(address)).Validate(); + if (error is null) result.ShouldBeNull(); else result!.ShouldContain(error); + } + + [Fact] + public void Editor_is_never_stricter_than_the_gate() + { + const string address = "Q9:0"; + AbLegacyTagConfigModel.FromJson(Cfg(address)).Validate().ShouldNotBeNull(); + EquipmentTagConfigInspector.Inspect(DriverTypeNames.AbLegacy, Cfg(address), null).ShouldNotBeEmpty(); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawTagConfigGateTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawTagConfigGateTests.cs index 8acd8e63..65930cf0 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawTagConfigGateTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawTagConfigGateTests.cs @@ -346,7 +346,7 @@ public sealed class RawTagConfigGateTests public void A_browse_committed_config_passes_the_gate(string driverType) { var tagConfig = RawBrowseCommitMapper.BuildTagConfig( - driverType, "SomeAddress", BrowseFieldsFor(driverType)); + driverType, driverType == DriverTypeNames.AbLegacy ? "N7:0" : "SomeAddress", BrowseFieldsFor(driverType)); TagConfigValidator.Validate(driverType, tagConfig).ShouldBeNull( $"browse-commit for {driverType} must not author a config the import refuses");