diff --git a/CLAUDE.md b/CLAUDE.md index 7a2ac620..0d1e228f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1094,8 +1094,9 @@ re-emit a concrete default. The defaults differ per driver (final review, 2026-1 used to fail `S:`/`O:`/`I:` and `R` status bits there (incl. the default probe `S:0`) — its `pccc.c` needs explicit file numbers and has no R status-bit mnemonics; since #659 `LibplctagLegacyName` translates (`S2:`/`O0:`/`I1:`, `R6:0.CON/13`) and both transports read identically on that press. The authored grammar is unchanged. Read-side - only: a libplctag WRITE to a rewritten status bit (R `EN…FD`, `C…UA`) is refused `BadNotSupported` before any runtime - is created (it could never reach the PLC before); libplctag T/C status-bit writes it parses itself are unchanged. + only: a libplctag WRITE to any address the translation changes (`S:`/`O:`/`I:` incl. bit writes, R `EN…FD`, `C…UA`) + is refused `BadNotSupported` before any runtime is created (none could reach the PLC before — no write-surface + widening on the default transport); libplctag T/C status-bit writes it parses itself are unchanged. L/A files, function files, SLC, PLC-5 and `logixpccc` remain unverified natively. Explicit Native on PLC-5/`LogixPccc` logs a once-per-instance Warning. AbLegacy native refuses status-bit sub-element writes (`T4:0.DN`…, `BadNotSupported`). diff --git a/docs/drivers/AbLegacy.md b/docs/drivers/AbLegacy.md index 91eca534..c445a270 100644 --- a/docs/drivers/AbLegacy.md +++ b/docs/drivers/AbLegacy.md @@ -243,13 +243,15 @@ status file when a digit follows it), and its sub-element table has only `con`/` bit libplctag lacks (`R` `EN/EU/DN/EM/ER/UL/IN/FD`, `C` `UA`) → `R6:0.CON/` (control word + bit, bit numbers from the native table). The authored grammar is unchanged; `S2:0`/`O0:0`/`I1:0` stay rejected by the parser (one spelling per address). Re-run on the same idle press 2026-10-09: every address above reads **identical** on both -transports (`S:4` is a free-running clock and differs read to read on each). **The translation is read-side only — -it does not widen the write surface:** before it, a libplctag write to an `R` status bit (or `C…UA`) could never reach -the PLC (tag creation failed), and `R6:0.CON/13` would otherwise turn it into a masked write to the control word. So -on libplctag a write to any status bit the translation rewrites (`R` `EN/EU/DN/EM/ER/UL/IN/FD`, `C` `UA`) is refused -**`BadNotSupported` before a runtime is created** — nothing sent, health unchanged -(`LibplctagLegacyTagRuntime.WriteRefusal`, asked through `IAbLegacyTagFactory.WriteRefusal`, the same plumbing as the -native refusal). T/C status bits libplctag parses itself keep their pre-existing write behaviour (note above). +transports (`S:4` is a free-running clock and differs read to read on each). **The translation is read-side only — it does not widen the write surface:** before it, libplctag could not create a +tag for `S:`/`O:`/`I:` or for an `R`/`C…UA` status bit, so no write to one ever reached the PLC. Now that the +translated names parse, a write would drive a physical output, a status-file word or (masked) a control word. So on +libplctag **every write whose address the translation changes** — `S:`, `O:`, `I:` (word and bit writes; a bit write +such as `O:0/3` is refused ahead of the parent-word read-modify-write) and the rewritten status bits (`R` +`EN/EU/DN/EM/ER/UL/IN/FD`, `C` `UA`) — is refused **`BadNotSupported` before a runtime is created**: nothing read or +sent, health unchanged (`LibplctagLegacyTagRuntime.WriteRefusal`, asked through `IAbLegacyTagFactory.WriteRefusal`, the +same plumbing as the native refusal). Those writes never worked on libplctag, so nothing is lost; the native +transport (opt-in) does write `S`/`O`/`I`. T/C status bits libplctag parses itself keep their pre-existing write behaviour (note above). Not tested on hardware: L and A files (no press has them), function files, PLC-5, SLC, `logixpccc`. Simulator evidence: diff --git a/docs/plans/2026-10-07-native-ab-transport-gate-results.md b/docs/plans/2026-10-07-native-ab-transport-gate-results.md index c337bad4..9ff57893 100644 --- a/docs/plans/2026-10-07-native-ab-transport-gate-results.md +++ b/docs/plans/2026-10-07-native-ab-transport-gate-results.md @@ -700,3 +700,5 @@ ST14:10 String Native value="Z-33323" status=0x00000000 ( ``` Every address reads identically on both transports. `S:4` is the free-running clock, so its value differs read to read on each transport (both Good). `I:0` = -29376 = 0x8D40, and bits `/6` and `/15` read true on both while `/0`, `/1`, `/7` read false — the bit is extracted from the word libplctag read, not defaulted. ab_server has no S/O/I files, so this real-controller read is the only wire evidence; the translation is unit-pinned against a model of the `pccc.c` parser (`LibplctagLegacyNameTests`). + +Caveats: `R6:0.DN` and `R6:0.EN` read `false` on both transports, so this read does not discriminate the control-word bit number (a wrong bit would very likely also read false) — the R bit mapping rests on the native table and the `pccc.c` parser model, not on this wire evidence. Writes: on libplctag every address the translation changes (`S:`/`O:`/`I:`, R/UA status bits) is refused `BadNotSupported` (#659 review), so this fix adds no libplctag write path. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyName.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyName.cs index c68cae96..ec26ae7c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyName.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyName.cs @@ -46,17 +46,18 @@ internal static class LibplctagLegacyName public static string Translate(string canonical) => TranslateCore(canonical).Name; /// - /// True when is a status bit this translator rewrites to control word + bit - /// (R6:0.DN → R6:0.CON/13; R EN/EU/DN/EM/ER/UL/IN/FD, counter UA). A WRITE to one is - /// refused (): before the translation libplctag could not - /// even create such a tag, so a write never reached the PLC, and the rewrite must not turn it into a new masked - /// write to a production controller's control word (the native transport refuses status-bit writes too). + /// True when changes : a file number supplied for + /// S:/O:/I: (incl. bit forms such as O:0/3) or a status bit rewritten to control word + + /// bit (R6:0.DN → R6:0.CON/13; R EN/EU/DN/EM/ER/UL/IN/FD, counter UA). A WRITE to such an + /// address is refused (): before #659 libplctag could not + /// even create these tags, so no write ever reached the PLC, and the read fix must not turn libplctag — the + /// default transport — into a new way to drive outputs, status-file words or control words. /// /// The canonical address. - /// for a rewritten status bit. - public static bool IsRewrittenStatusBit(string canonical) => TranslateCore(canonical).RewrittenStatusBit; + /// when the libplctag name differs from the canonical one. + public static bool IsTranslated(string canonical) => TranslateCore(canonical).Changed; - private static (string Name, bool RewrittenStatusBit) TranslateCore(string canonical) + private static (string Name, bool Changed) TranslateCore(string canonical) { // Family-neutral parse is correct: the driver hands the runtime AbLegacyAddress.ToLibplctagName(), already // decimal-normalised (PLC-5 octal I/O included) — the same reasoning as NativeAbLegacyTagRuntime.ToPcccAddress. @@ -65,6 +66,7 @@ internal static class LibplctagLegacyName var fileNumber = a.FileNumber ?? PcccFileType.ImplicitFileNumber(a.FileLetter); if (fileNumber is null) return (canonical, false); + var fileNumberSupplied = a.FileNumber is null; // S:/O:/I: — libplctag needs S2/O0/I1 var name = $"{a.FileLetter}{fileNumber}:{a.WordNumber}"; if (a.SubElement is { } sub) { @@ -91,6 +93,6 @@ internal static class LibplctagLegacyName } if (a.BitIndex is int b) name += $"/{b}"; - return (name, false); + return (name, fileNumberSupplied); } } 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 bdcef7fc..d070ea96 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs @@ -71,18 +71,20 @@ internal sealed class LibplctagLegacyTagRuntime : IAbLegacyTagRuntime : RunReleasingOrphanAsync(() => _tag.WriteAsync(cancellationToken)); /// - /// The writes the libplctag runtime refuses before the wire, or : a status bit that - /// rewrites to control word + bit (R6:0.DN → R6:0.CON/13, R - /// EN/EU/DN/EM/ER/UL/IN/FD, counter UA). Before #659 libplctag could not create those tags at all, - /// so a write to one never reached the controller; the read-side translation must not open a new masked write - /// to a control word. T/C status bits libplctag itself parses (T4:0.DN, C5:0.CU…) keep their - /// pre-existing behaviour — a masked bit write (documented hazard in docs/drivers/AbLegacy.md). + /// The writes the libplctag runtime refuses before the wire, or : any address whose + /// libplctag name had to change () + /// — file-number-less S:/O:/I: (incl. bit writes such as O:0/3, refused before the + /// parent-word read-modify-write) and the status bits rewritten to control word + bit (R6:0.DN, counter + /// UA). Before #659 libplctag could not create those tags at all, so no write to one ever reached the + /// controller; the read fix must not widen the write surface of the default transport. T/C status bits + /// libplctag parses itself (T4:0.DN, C5:0.CU…) keep their pre-existing behaviour — a masked bit + /// write (documented hazard in docs/drivers/AbLegacy.md). /// /// The tag's create parameters. /// The refusal reason, or . internal static string? WriteRefusal(AbLegacyTagCreateParams p) => - LibplctagLegacyName.IsRewrittenStatusBit(p.TagName) - ? $"Writes to the status bit '{p.TagName}' are not supported on the libplctag transport (libplctag has no mnemonic for it; it is read as control word + bit)." + LibplctagLegacyName.IsTranslated(p.TagName) + ? $"Writes to '{p.TagName}' are not supported on the libplctag transport (its name is translated for libplctag; reads only)." : null; /// diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/LibplctagStatusBitWriteRefusalTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/LibplctagTranslatedWriteRefusalTests.cs similarity index 60% rename from tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/LibplctagStatusBitWriteRefusalTests.cs rename to tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/LibplctagTranslatedWriteRefusalTests.cs index 6166b72a..23099c42 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/LibplctagStatusBitWriteRefusalTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/LibplctagTranslatedWriteRefusalTests.cs @@ -7,14 +7,15 @@ using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.PlcFamilies; namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests; /// -/// #659 review ruling: the libplctag read translation of a status bit libplctag has no mnemonic for -/// (R6:0.DN → R6:0.CON/13) must not widen the WRITE surface. Before the translation libplctag could -/// not create such a tag, so a write never reached the controller; now that the name parses, a write would go out -/// as a masked write to the control word. Such writes are refused BadNotSupported before any runtime is -/// created, health unchanged. T/C status bits libplctag itself parses keep their pre-existing (unrefused) behaviour. +/// #659 review rulings: the libplctag read translation must not widen the WRITE surface of the default transport. +/// Before it, libplctag could not create a tag for S:/O:/I: (no file number) or for an R/UA +/// status bit, so no write to one ever reached the controller; now that the translated names parse, a write would +/// drive an output, a status-file word or (masked) a control word. Every write whose address the translation +/// changes is refused BadNotSupported before any runtime is created, health unchanged. T/C status bits +/// libplctag itself parses keep their pre-existing (unrefused) behaviour. /// [Trait("Category", "Unit")] -public sealed class LibplctagStatusBitWriteRefusalTests +public sealed class LibplctagTranslatedWriteRefusalTests { [Theory] [InlineData("R6:0.EN", true)] @@ -31,19 +32,27 @@ public sealed class LibplctagStatusBitWriteRefusalTests [InlineData("C5:0.DN", false)] [InlineData("R6:0.LEN", false)] [InlineData("R6:0.POS", false)] - [InlineData("S:0", false)] // translated, but not a status bit - [InlineData("O:0/3", false)] - [InlineData("I:0/0", false)] + [InlineData("S:0", true)] // file number supplied by the translation (S2:0) + [InlineData("S:1/5", true)] + [InlineData("S:57", true)] + [InlineData("O:0", true)] + [InlineData("O:0/3", true)] + [InlineData("I:0", true)] + [InlineData("I:0/0", true)] [InlineData("N7:0", false)] + [InlineData("N7:0/3", false)] + [InlineData("F8:0", false)] + [InlineData("ST14:10", false)] + [InlineData("T4:0.PRE", false)] [InlineData("B3:0/4", false)] [InlineData("R6:0.FOO", false)] [InlineData("garbage", false)] - public void The_libplctag_refusal_covers_exactly_the_rewritten_status_bits(string tag, bool refused) + public void The_libplctag_refusal_covers_exactly_the_translated_addresses(string tag, bool refused) { var p = new AbLegacyTagCreateParams("h", 44818, "", "micrologix", tag, TimeSpan.FromSeconds(1)); (LibplctagLegacyTagRuntime.WriteRefusal(p) is not null).ShouldBe(refused); (new LibplctagLegacyTagFactory().WriteRefusal(p) is not null).ShouldBe(refused); - LibplctagLegacyName.IsRewrittenStatusBit(tag).ShouldBe(refused); + LibplctagLegacyName.IsTranslated(tag).ShouldBe(refused); } /// Defence in depth: a runtime for a rewritten status bit refuses the write itself, without touching the @@ -69,14 +78,15 @@ public sealed class LibplctagStatusBitWriteRefusalTests } #pragma warning disable OTOPCUA0001 // direct driver calls pin the driver's own status mapping (no resilience wrapper) - private static async Task<(AbLegacyDriver Driver, CountingLibplctagRefusalFactory Factory)> DriverAsync(string address) + private static async Task<(AbLegacyDriver Driver, CountingLibplctagRefusalFactory Factory)> DriverAsync( + string address, AbLegacyDataType type = AbLegacyDataType.Bit) { const string host = "ab://10.0.0.5/"; var factory = new CountingLibplctagRefusalFactory(); var driver = new AbLegacyDriver(new AbLegacyDriverOptions { Devices = [new AbLegacyDeviceOptions(host, AbLegacyPlcFamily.MicroLogix)], - RawTags = AbLegacyRawTags.Entries([new AbLegacyTagDefinition("Bit", host, address, AbLegacyDataType.Bit, Writable: true)]), + RawTags = AbLegacyRawTags.Entries([new AbLegacyTagDefinition("Bit", host, address, type, Writable: true)]), Timeout = TimeSpan.FromSeconds(1), Probe = new AbLegacyProbeOptions { Enabled = false }, }, "drv-libplctag-statusbit", factory); @@ -84,17 +94,25 @@ public sealed class LibplctagStatusBitWriteRefusalTests return (driver, factory); } + /// Includes bit writes to O/I/S (O:0/3): the refusal runs ahead of the bit read-modify-write, so + /// not even the parent-word runtime (O:0) is created — nothing is read or written. [Theory] - [InlineData("R6:0.DN")] - [InlineData("R6:0.EN")] - [InlineData("C5:0.UA")] - public async Task A_rewritten_status_bit_write_is_BadNotSupported_with_no_runtime_and_health_unchanged(string address) + [InlineData("R6:0.DN", AbLegacyDataType.Bit, true)] + [InlineData("R6:0.EN", AbLegacyDataType.Bit, true)] + [InlineData("C5:0.UA", AbLegacyDataType.Bit, true)] + [InlineData("O:0/3", AbLegacyDataType.Bit, true)] + [InlineData("I:0/0", AbLegacyDataType.Bit, true)] + [InlineData("S:1/5", AbLegacyDataType.Bit, true)] + [InlineData("O:0", AbLegacyDataType.Int, (short)1)] + [InlineData("S:1", AbLegacyDataType.Int, (short)1)] + public async Task A_translated_address_write_is_BadNotSupported_with_no_runtime_and_health_unchanged( + string address, AbLegacyDataType type, object value) { var ct = TestContext.Current.CancellationToken; - var (driver, factory) = await DriverAsync(address); + var (driver, factory) = await DriverAsync(address, type); var health = driver.GetHealth(); - (await driver.WriteAsync([new WriteRequest("Bit", true)], ct)).Single().StatusCode + (await driver.WriteAsync([new WriteRequest("Bit", value)], ct)).Single().StatusCode .ShouldBe(AbLegacyStatusMapper.BadNotSupported); factory.Inner.Tags.ShouldBeEmpty(); @@ -120,5 +138,21 @@ public sealed class LibplctagStatusBitWriteRefusalTests factory.Inner.Tags["T4:0.DN"].WriteCount.ShouldBe(1); await driver.ShutdownAsync(ct); } + + /// Positive control: an untranslated bit write (B3:0/4) still runs the parent-word + /// read-modify-write, so the refusal is not swallowing bit writes in general. + [Fact] + public async Task An_untranslated_bit_write_still_runs_the_parent_word_read_modify_write() + { + var ct = TestContext.Current.CancellationToken; + var (driver, factory) = await DriverAsync("B3:0/4"); + factory.Inner.Customise = p => new FakeAbLegacyTag(p) { Value = (short)0 }; + + (await driver.WriteAsync([new WriteRequest("Bit", true)], ct)).Single().StatusCode + .ShouldBe(AbLegacyStatusMapper.Good); + + factory.Inner.Tags["B3:0"].WriteCount.ShouldBe(1); + await driver.ShutdownAsync(ct); + } #pragma warning restore OTOPCUA0001 }