diff --git a/CLAUDE.md b/CLAUDE.md index 1c68f757..ca385748 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1118,8 +1118,9 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa - **TwinCAT (#637):** TIME/TOD publish Float64 ms, DATE/DT a UTC DateTime; a tag authored `UDInt` keeps the raw counter (and keeps historizing — DateTime is not historized). - **AbLegacy (#636):** `AbLegacyAddress` (in `AbLegacy.Contracts`) is family-aware: PLC-5 `I:`/`O:` indices are OCTAL - and are emitted to libplctag as decimal (libplctag's `pccc.c` parses decimal only). MicroLogix function files and - indirect `N7:[N7:0]` addresses parse but read `BadNotSupported`. One check (`AbLegacyAddress.Validate`) backs the + and are emitted to libplctag as decimal (libplctag's `pccc.c` parses decimal only). MicroLogix function files parse + but read `BadNotSupported`. Indirect `N7:[N7:0]` (#646) is resolved by the driver at read/write time (index re-read + every call, never cached; bad index = `BadOutOfRange`; indirect writes are not atomic). One check (`AbLegacyAddress.Validate`) backs the editor (family-independent) and the deploy gate (family from the tag's `DeviceConfig.PlcFamily`); an unparseable address reads `BadNodeIdUnknown` for that tag only. - **OpcUaClient (#635):** OperationLimits are honoured by the SDK session (`SessionClientBatched`) — do not re-add diff --git a/docs/drivers/AbLegacy.md b/docs/drivers/AbLegacy.md index 09b46101..4ce9d5ac 100644 --- a/docs/drivers/AbLegacy.md +++ b/docs/drivers/AbLegacy.md @@ -69,7 +69,7 @@ pieces for driver-side routing (e.g. deciding a bit needs read-modify-write): | `I:0/0` / `O:1/2` / `S:1` | Input / output / status system files (no file number) | | `I:017/07` | **PLC-5 only:** `I:`/`O:` word and bit indices are **octal** (#244); a digit 8/9 is a parse error | | `RTC:0.HR` / `HSC:0.ACC` | **MicroLogix only:** function files RTC, HSC, DLS, MMI, PTO, PWM, STI, EII, IOS, BHI (#245) | -| `N7:[N7:0]` / `N[N7:0]:5` | Indirect (index word read from the PLC) — parsed, **not supported at runtime** (#247) | +| `N7:[N7:0]` / `N[N7:0]:5` | Indirect (index word read from the PLC) — resolved at read/write time (#646) | `AbLegacyDataType` covers the corresponding PCCC types: `Bit`, `Int` (N), `Long` (L), `Float` (F), `AnalogInt` (A), `String` (ST), and the `TimerElement` / @@ -118,11 +118,18 @@ physical inputs and may reject the write. members default to Int32). ⚠️ **Wire-unverified on both transports:** there is no capture and no simulator coverage; the libplctag path passes the name through and the native transport has no file-type code for them, so it refuses the tag as `Unsupported` (`BadNotSupported` to the client; see Wire evidence). -- **Indirect addressing (#247).** `N7:[N7:0]` (word from another address) and `N[N7:0]:5` (file from another - address) parse, one level deep (the inner address must be direct, and carries the family's octal rule). - **Runtime resolution is not implemented** (it needs a pre-read of the index word and per-resolved-address - runtimes): reads and writes of an indirect tag answer `BadNotSupported` before any runtime is created, on both - transports. +- **Indirect addressing (#247 parse, #646 runtime).** `N7:[N7:0]` (word from another address) and `N[N7:0]:5` (file + from another address) parse, one level deep (the inner address must be direct, and carries the family's octal rule). + At runtime the driver reads the inner address first (same runtime factory, so libplctag and Native alike), requires + an integer in range (word/element `0..255`, file `0..999`), composes the concrete address and runs the normal + read/write path against it. **The index is re-read on every read/write - never cached** - and the previous + resolution's handle is released when the index moves. A bad index (negative, too large, float/string source) is + `BadOutOfRange` for that tag only (no health impact); an inner-read failure propagates its own status (a wire + failure also degrades health like any read). **Writes** resolve then write and are **not atomic**: the index can + change between the two PLC accesses. Bit RMW writes (`B3:[N7:0]/4`) resolve first, then RMW the concrete word. + The Native transport never sees the bracket form (it receives the composed name), so N-file indirect stays inside + the wire-proven envelope; other file types are as (un)verified as their direct forms. Live-gated on `ab_server` + by `AbLegacyIndirectAddressingTests` (`N7:0=5`, `N7:5=42`, `N7:[N7:0]` reads 42). ## Configuration @@ -211,7 +218,8 @@ Everything below is against the `ab_server` simulator; no real PCCC controller h | N-file write (`N7:5`) | capture `slc500-write-n7` + native `AbLegacyReadSmokeTests` write/read round trip (SLC 500) | | F-file read (`F8`) | one capture, `slc500-read-f8` (codec parity only; no native F write on a wire) | | S (incl. the default probe `S:0`), B, T, C, R, L, ST, A, O, I | **none** — encoded from the PCCC file-type table | -| MicroLogix function files (RTC/HSC/...), PLC-5 octal I/O, indirect addressing | **none** — parsed only; native refuses function files and indirect names as Unsupported | +| MicroLogix function files (RTC/HSC/...), PLC-5 octal I/O | **none** — parsed only; native refuses function files as Unsupported | +| Indirect addressing (`N7:[N7:0]`) | `ab_server` N-file, both transports (#646) - resolved by the driver, the transport sees the composed address | | T/C/R sub-elements (`.PRE`/`.ACC`/`.LEN`/`.POS`), 6-byte T/C/R element layout | **none** | ### Native known gaps diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyAddress.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyAddress.cs index aac67c41..576cb062 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyAddress.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyAddress.cs @@ -23,7 +23,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; /// L9:0 — long-integer file (SLC 5/05+, 32-bit). /// I:017/07 — PLC-5 only: I/O word + bit indices are octal (#244); stored decimal. /// RTC:0.HR — MicroLogix only: function file (RTC/HSC/DLS/MMI/PTO/PWM/STI/EII/IOS/BHI, #245). -/// N7:[N7:0], N[N7:0]:5 — indirect addressing (#247): parsed, not resolved at runtime. +/// N7:[N7:0], N[N7:0]:5 — indirect addressing (#247 parse, #646 runtime): the driver reads the index first and composes the concrete address. /// /// The libplctag transport takes as its name=... attribute and the /// native transport re-parses the same string, so both are handed identical (decimal) numbers. This parser @@ -42,8 +42,8 @@ public sealed record AbLegacyAddress( /// /// True when the file number or the word number is sourced from another PCCC address evaluated /// at runtime (N7:[N7:0] or N[N7:0]:5). Neither transport can encode the bracket - /// form, and resolving it needs a pre-read of the index word, which is not implemented — the - /// driver answers an indirect tag BadNotSupported. + /// form, so the driver (#646) pre-reads the index, substitutes it, and hands the transport the + /// concrete address; an indirect name never reaches a transport. /// public bool IsIndirect => IndirectFileSource is not null || IndirectWordSource is not null; 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 1bbfa9af..b76a47b4 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 @@ -123,12 +123,6 @@ 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)"; /// @@ -179,8 +173,6 @@ public static class AbLegacyTagDefinitionFactory { 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; } 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 f1ea75e7..80dcc136 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs @@ -388,8 +388,6 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover try { - // 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. var preParsed = AbLegacyAddress.TryParse(def.Address, device.Options.PlcFamily); if (preParsed is null) { @@ -399,8 +397,18 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover } if (preParsed.IsIndirect) { - results[i] = new DataValueSnapshot(null, AbLegacyStatusMapper.BadNotSupported, null, now); - continue; + // Indirect (#646): read the index word(s) NOW (never cached - the index can change between + // polls), compose the concrete address, then run the normal path against it. + var res = await ResolveIndirectAsync(device, def, preParsed, cancellationToken).ConfigureAwait(false); + if (res.Def is null) + { + results[i] = new DataValueSnapshot(null, res.Status, null, now); + if (res.TransportFault) + _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, + $"transport fault resolving indirect address for {reference}"); + continue; + } + def = res.Def; } var runtime = await EnsureTagRuntimeAsync(device, def, cancellationToken).ConfigureAwait(false); @@ -513,11 +521,21 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover continue; } - // Indirect addressing is parse-only (#247): refuse before any runtime or RMW read is created. + // Indirect (#646): resolve the index first (non-atomic with the write - the index may change + // between the two PLC accesses), then write the concrete address through the normal path. if (parsed.IsIndirect) { - results[i] = new WriteResult(AbLegacyStatusMapper.BadNotSupported); - continue; + var res = await ResolveIndirectAsync(device, def, parsed, cancellationToken).ConfigureAwait(false); + if (res.Def is null) + { + results[i] = new WriteResult(res.Status); + if (res.TransportFault) + _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, + $"transport fault resolving indirect address for {w.FullReference}"); + continue; + } + def = res.Def; + parsed = AbLegacyAddress.TryParse(def.Address, device.Options.PlcFamily)!; } // A write the transport refuses outright (native: a T/C/R status-bit sub-element such as T4:0.DN, @@ -841,6 +859,132 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover } } + /// Largest word/element index an indirect source may yield (PCCC element offset is one byte). + private const int MaxIndirectElement = AbLegacyArray.MaxElements - 1; + + /// Largest data-file number an indirect source may yield (PLC-5 allows up to 999). + private const int MaxIndirectFile = 999; + + /// + /// Resolves an indirect address (#646). Reads each index source through the same runtime/transport path + /// (same factory, so libplctag and Native alike), range-checks it, and composes a concrete tag definition + /// whose Name is keyed by the resolved address so the normal runtime cache applies. Nothing about the + /// index is cached: every call re-reads it. A bad index is BadOutOfRange (per-tag, no health impact); + /// an inner-read failure propagates its mapped status (TransportFault true for a wire failure). + /// + private async Task ResolveIndirectAsync( + DeviceState device, AbLegacyTagDefinition def, AbLegacyAddress parsed, CancellationToken ct) + { + var fileNumber = parsed.FileNumber; + var word = parsed.WordNumber; + + if (parsed.IndirectFileSource is { } fileSrc) + { + var r = await ReadIndexAsync(device, fileSrc, ct).ConfigureAwait(false); + if (r.Status != 0) return new IndirectResolution(null, r.Status, r.TransportFault); + if (r.Value is < 0 or > MaxIndirectFile) + return new IndirectResolution(null, AbLegacyStatusMapper.BadOutOfRange, false); + fileNumber = (int)r.Value!; + } + if (parsed.IndirectWordSource is { } wordSrc) + { + var r = await ReadIndexAsync(device, wordSrc, ct).ConfigureAwait(false); + if (r.Status != 0) return new IndirectResolution(null, r.Status, r.TransportFault); + if (r.Value is < 0 or > MaxIndirectElement) + return new IndirectResolution(null, AbLegacyStatusMapper.BadOutOfRange, false); + word = (int)r.Value!; + } + + var concrete = parsed with + { + FileNumber = fileNumber, WordNumber = word, IndirectFileSource = null, IndirectWordSource = null, + }; + var concreteText = concrete.ToLibplctagName(); + var resolvedName = $"{def.Name}=>{concreteText}"; + + // The index moved: drop the previous resolution's handle so a sweeping index does not accumulate one + // libplctag handle per value it ever took. + var previous = device.IndirectLastName.TryGetValue(def.Name, out var prior) ? prior : null; + device.IndirectLastName[def.Name] = resolvedName; + if (previous is not null && !string.Equals(previous, resolvedName, StringComparison.OrdinalIgnoreCase)) + { + var oldLock = device.GetRuntimeLock(previous); + await oldLock.WaitAsync(ct).ConfigureAwait(false); + try { EvictRuntime(device, previous); } finally { oldLock.Release(); } + } + return new IndirectResolution(def with { Name = resolvedName, Address = concreteText }, 0, false); + } + + /// Reads one direct index address as an integer through the shared parent-runtime cache. + private async Task<(long? Value, uint Status, bool TransportFault)> ReadIndexAsync( + DeviceState device, AbLegacyAddress inner, CancellationToken ct) + { + AbLegacyDataType type; + if (inner.BitIndex is not null) type = AbLegacyDataType.Bit; + else if (inner.FileLetter == "L") type = AbLegacyDataType.Long; + else if (inner.FileLetter is "F" or "ST") return (null, AbLegacyStatusMapper.BadOutOfRange, false); + else type = AbLegacyDataType.Int; + + // A bit-addressed index reads its parent word and decodes the bit, like the RMW path does. + var runtimeName = (inner with { BitIndex = null }).ToLibplctagName(); + var gate = device.GetRmwLock(runtimeName); + await gate.WaitAsync(ct).ConfigureAwait(false); + try + { + IAbLegacyTagRuntime runtime; + try + { + runtime = await EnsureParentRuntimeAsync(device, runtimeName, ct).ConfigureAwait(false); + await runtime.ReadAsync(ct).ConfigureAwait(false); + } + catch (OperationCanceledException) { throw; } + catch (AbTransportException ex) when (IsLocalRefusal(ex)) + { + return (null, AbLegacyStatusMapper.BadNotSupported, false); + } + catch + { + EvictParentRuntime(device, runtimeName); + return (null, AbLegacyStatusMapper.BadCommunicationError, true); + } + + var status = runtime.GetStatus(); + if (status != 0) + { + EvictParentRuntime(device, runtimeName); + return (null, AbLegacyStatusMapper.MapStatus(status), true); + } + var raw = runtime.DecodeValue(type, inner.BitIndex); + try + { + return raw switch + { + bool b => (b ? 1L : 0L, 0u, false), + null => (null, AbLegacyStatusMapper.BadOutOfRange, false), + _ => (Convert.ToInt64(raw), 0u, false), + }; + } + catch (Exception ex) when (ex is FormatException or InvalidCastException or OverflowException) + { + return (null, AbLegacyStatusMapper.BadOutOfRange, false); + } + } + finally + { + gate.Release(); + } + } + + private static void EvictParentRuntime(DeviceState device, string name) + { + if (device.ParentRuntimes.TryRemove(name, out var stale)) + { + try { stale.Dispose(); } catch { /* best-effort */ } + } + } + + private sealed record IndirectResolution(AbLegacyTagDefinition? Def, uint Status, bool TransportFault); + private async Task EnsureParentRuntimeAsync( AbLegacyDriver.DeviceState device, string parentName, CancellationToken ct) { @@ -1086,6 +1230,10 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover public System.Collections.Concurrent.ConcurrentDictionary ParentRuntimes { get; } = new(StringComparer.OrdinalIgnoreCase); + /// Indirect tag name to the resolved-address runtime key it last used (#646), so a moved index evicts its stale handle. + public System.Collections.Concurrent.ConcurrentDictionary IndirectLastName { get; } = + new(StringComparer.OrdinalIgnoreCase); + /// /// Per-key creation locks for and . /// A caller holds this before the TryGetValue + Create + InitializeAsync + TryAdd diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyIndirectAddressingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyIndirectAddressingTests.cs new file mode 100644 index 00000000..1b12a781 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyIndirectAddressingTests.cs @@ -0,0 +1,55 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.PlcFamilies; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests; + +/// +/// #646 against ab_server: N7:[N7:0] reads the index word, then the word it names, on both +/// transports. Writes N7:0 and N7:5 (simulator-only, gated by ). +/// +[Collection(AbLegacyServerCollection.Name)] +[Trait("Category", "Integration")] +[Trait("Simulator", "ab_server-PCCC")] +public sealed class AbLegacyIndirectAddressingTests(AbLegacyServerFixture sim) +{ + /// Seeds N7:0=5 and N7:5=42, then reads and writes through N7:[N7:0]. + /// The transport stack the driver runs on. + [AbLegacyTheory] + [MemberData(nameof(TransportMatrix.Transports), MemberType = typeof(TransportMatrix))] + public async Task Slc500_indirect_word_read_and_write_resolve_through_the_index(AbLegacyTransport transport) + { + if (sim.SkipReasonFor(KnownProfiles.Slc500) is { } skip) Assert.Skip(skip); + IntegrationWriteGate.SkipUnlessWritesAllowed(); + + var deviceUri = $"ab://{sim.Host}:{sim.PortFor(KnownProfiles.Slc500)}/{sim.CipPath}"; + await using var drv = new AbLegacyDriver(new AbLegacyDriverOptions + { + Devices = [new AbLegacyDeviceOptions(deviceUri, AbLegacyPlcFamily.Slc500)], + RawTags = AbLegacyRawTags.Entries( + new AbLegacyTagDefinition("Index", deviceUri, "N7:0", AbLegacyDataType.Int, Writable: true), + new AbLegacyTagDefinition("Target", deviceUri, "N7:5", AbLegacyDataType.Int, Writable: true), + new AbLegacyTagDefinition("Indirect", deviceUri, "N7:[N7:0]", AbLegacyDataType.Int, Writable: true)), + Timeout = TimeSpan.FromSeconds(5), + Probe = new AbLegacyProbeOptions { Enabled = false }, + Transport = transport, + }, driverInstanceId: $"ablegacy-indirect-{transport}"); + var ct = TestContext.Current.CancellationToken; + await drv.InitializeAsync("{}", ct); + + var seeded = await drv.WriteAsync( + [new WriteRequest("Index", (short)5), new WriteRequest("Target", (short)42)], ct); + seeded.ShouldAllBe(r => r.StatusCode == AbLegacyStatusMapper.Good); + + var read = (await drv.ReadAsync(["Indirect"], ct)).Single(); + read.StatusCode.ShouldBe(AbLegacyStatusMapper.Good); + Convert.ToInt32(read.Value).ShouldBe(42); + + // Write through the indirection lands on N7:5. + (await drv.WriteAsync([new WriteRequest("Indirect", (short)77)], ct)).Single() + .StatusCode.ShouldBe(AbLegacyStatusMapper.Good); + var direct = (await drv.ReadAsync(["Target"], ct)).Single(); + Convert.ToInt32(direct.Value).ShouldBe(77); + } +} 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 595b4797..58716228 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 @@ -181,28 +181,6 @@ public sealed class AbLegacyFamilyAddressingTests factory.Tags["I:15/15"].ReadCount.ShouldBe(1); } - /// An indirect tag is answered BadNotSupported on read and write, and no runtime is ever created for it. - [Fact] - public async Task Indirect_tag_is_BadNotSupported_and_creates_no_runtime() - { - var (drv, factory) = NewDriver(AbLegacyPlcFamily.Slc500, - new AbLegacyTagDefinition("Ind", "ab://10.0.0.5/1,0", "N7:[N7:0]", AbLegacyDataType.Int, Writable: true), - new AbLegacyTagDefinition("Ok", "ab://10.0.0.5/1,0", "N7:1", AbLegacyDataType.Int, Writable: true)); - await drv.InitializeAsync("{}", CancellationToken.None); - factory.Customise = p => new FakeAbLegacyTag(p) { Value = 5 }; - - var read = await drv.ReadAsync(["Ind", "Ok"], CancellationToken.None); - var write = await drv.WriteAsync( - [new WriteRequest("Ind", 1), new WriteRequest("Ok", 1)], CancellationToken.None); - - read[0].StatusCode.ShouldBe(AbLegacyStatusMapper.BadNotSupported); - write[0].StatusCode.ShouldBe(AbLegacyStatusMapper.BadNotSupported); - // positive control: the neighbouring direct tag in the same batch still works, so the refusal is per-tag - read[1].StatusCode.ShouldBe(AbLegacyStatusMapper.Good); - write[1].StatusCode.ShouldBe(AbLegacyStatusMapper.Good); - factory.Tags.Keys.ShouldNotContain(k => k.Contains('[')); - } - /// A MicroLogix function-file tag reaches the transport under its libplctag name. [Fact] public async Task MicroLogix_function_file_tag_reaches_the_transport() diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyIndirectAddressingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyIndirectAddressingTests.cs new file mode 100644 index 00000000..6baf2165 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyIndirectAddressingTests.cs @@ -0,0 +1,131 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.PlcFamilies; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests; + +/// #646: indirect addresses (N7:[N7:0], N[N7:0]:5) resolve at read/write time. +[Trait("Category", "Unit")] +public sealed class AbLegacyIndirectAddressingTests +{ + private const string Host = "ab://10.0.0.5/1,0"; + + private static async Task<(AbLegacyDriver drv, FakeAbLegacyTagFactory factory)> NewDriverAsync( + params AbLegacyTagDefinition[] tags) + { + var factory = new FakeAbLegacyTagFactory(); + var drv = new AbLegacyDriver(new AbLegacyDriverOptions + { + Devices = [new AbLegacyDeviceOptions(Host, AbLegacyPlcFamily.Slc500)], + RawTags = AbLegacyRawTags.Entries(tags), + Probe = new AbLegacyProbeOptions { Enabled = false }, + }, "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + return (drv, factory); + } + + private static void Seed(FakeAbLegacyTagFactory f, params (string name, object value)[] values) + { + var map = values.ToDictionary(v => v.name, v => v.value, StringComparer.OrdinalIgnoreCase); + f.Customise = p => new FakeAbLegacyTag(p) { Value = map.TryGetValue(p.TagName, out var v) ? v : 0 }; + } + + [Fact] + public async Task Word_indirect_reads_index_then_composed_address() + { + var (drv, f) = await NewDriverAsync(new AbLegacyTagDefinition("Ind", Host, "N7:[N7:0]", AbLegacyDataType.Int)); + Seed(f, ("N7:0", (short)5), ("N7:5", (short)42)); + + var r = (await drv.ReadAsync(["Ind"], CancellationToken.None)).Single(); + + r.StatusCode.ShouldBe(AbLegacyStatusMapper.Good); + r.Value.ShouldBe((short)42); + f.Tags["N7:0"].ReadCount.ShouldBe(1); + f.Tags["N7:5"].ReadCount.ShouldBe(1); + f.Tags.Keys.ShouldNotContain(k => k.Contains('[')); + } + + [Fact] + public async Task File_indirect_resolves_the_file_number() + { + var (drv, f) = await NewDriverAsync(new AbLegacyTagDefinition("Ind", Host, "N[N7:0]:5", AbLegacyDataType.Int)); + Seed(f, ("N7:0", (short)9), ("N9:5", (short)77)); + + var r = (await drv.ReadAsync(["Ind"], CancellationToken.None)).Single(); + + r.StatusCode.ShouldBe(AbLegacyStatusMapper.Good); + r.Value.ShouldBe((short)77); + f.Tags.ContainsKey("N9:5").ShouldBeTrue(); + } + + [Fact] + public async Task Index_is_re_read_every_poll_never_cached() + { + var (drv, f) = await NewDriverAsync(new AbLegacyTagDefinition("Ind", Host, "N7:[N7:0]", AbLegacyDataType.Int)); + Seed(f, ("N7:0", (short)5), ("N7:5", (short)42), ("N7:6", (short)43)); + + (await drv.ReadAsync(["Ind"], CancellationToken.None)).Single().Value.ShouldBe((short)42); + f.Tags["N7:0"].Value = (short)6; + (await drv.ReadAsync(["Ind"], CancellationToken.None)).Single().Value.ShouldBe((short)43); + + f.Tags["N7:0"].ReadCount.ShouldBe(2); + // the stale resolution's handle is released when the index moves + f.Tags["N7:5"].Disposed.ShouldBeTrue(); + } + + [Theory] + [InlineData(-1)] + [InlineData(256)] + [InlineData(30000)] + public async Task Out_of_range_index_is_BadOutOfRange_without_health_impact(int index) + { + var (drv, f) = await NewDriverAsync(new AbLegacyTagDefinition("Ind", Host, "N7:[N7:0]", AbLegacyDataType.Int)); + Seed(f, ("N7:0", (short)index)); + + var r = (await drv.ReadAsync(["Ind"], CancellationToken.None)).Single(); + + r.StatusCode.ShouldBe(AbLegacyStatusMapper.BadOutOfRange); + drv.GetHealth().State.ShouldNotBe(DriverState.Degraded); + f.Tags.Keys.ShouldNotContain(k => k.StartsWith("N7:") && k != "N7:0"); + } + + [Fact] + public async Task Inner_read_failure_propagates_its_status() + { + var (drv, f) = await NewDriverAsync(new AbLegacyTagDefinition("Ind", Host, "N7:[N7:0]", AbLegacyDataType.Int)); + f.Customise = p => new FakeAbLegacyTag(p) { Value = (short)5, Status = p.TagName == "N7:0" ? -14 : 0 }; + + var r = (await drv.ReadAsync(["Ind"], CancellationToken.None)).Single(); + + r.StatusCode.ShouldNotBe(AbLegacyStatusMapper.Good); + r.StatusCode.ShouldNotBe(AbLegacyStatusMapper.BadOutOfRange); + f.Tags.ContainsKey("N7:5").ShouldBeFalse(); + } + + [Fact] + public async Task Write_resolves_index_then_writes_the_composed_address() + { + var (drv, f) = await NewDriverAsync(new AbLegacyTagDefinition("Ind", Host, "N7:[N7:0]", AbLegacyDataType.Int, Writable: true)); + Seed(f, ("N7:0", (short)5)); + + var w = (await drv.WriteAsync([new WriteRequest("Ind", 99)], CancellationToken.None)).Single(); + + w.StatusCode.ShouldBe(AbLegacyStatusMapper.Good); + f.Tags["N7:5"].WriteCount.ShouldBe(1); + f.Tags["N7:5"].Value.ShouldBe((short)99); + f.Tags["N7:0"].WriteCount.ShouldBe(0); + } + + [Fact] + public async Task Write_with_bad_index_is_BadOutOfRange_and_writes_nothing() + { + var (drv, f) = await NewDriverAsync(new AbLegacyTagDefinition("Ind", Host, "N7:[N7:0]", AbLegacyDataType.Int, Writable: true)); + Seed(f, ("N7:0", (short)-3)); + + var w = (await drv.WriteAsync([new WriteRequest("Ind", 1)], CancellationToken.None)).Single(); + + w.StatusCode.ShouldBe(AbLegacyStatusMapper.BadOutOfRange); + f.Tags.Values.ShouldAllBe(t => t.WriteCount == 0); + } +} 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 index b0404868..cd29213f 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/AbLegacyFamilyAddressGateTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/AbLegacyFamilyAddressGateTests.cs @@ -28,10 +28,10 @@ public sealed class AbLegacyFamilyAddressGateTests } [Fact] - public void Gate_warns_about_indirect_addresses_which_never_read() + public void Gate_does_not_warn_about_indirect_addresses_which_the_driver_resolves() { EquipmentTagConfigInspector.Inspect(DriverTypeNames.AbLegacy, Cfg("N7:[N7:0]"), "{\"PlcFamily\":\"Slc500\"}") - .ShouldContain(x => x.Contains("indirect")); + .ShouldBeEmpty(); } [Theory]