From 4f5bbd253cbd002f23fb21d4f8c34d82ba42526a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 11:00:31 -0400 Subject: [PATCH] feat(driver-twincat): shared TagConfigJson readers, writable key, Inspect warnings (R2-11) --- ...tagconfig-consolidation-plan.md.tasks.json | 2 +- .../TwinCATEquipmentTagParser.cs | 40 +++++++++++++-- ...WW.OtOpcUa.Driver.TwinCAT.Contracts.csproj | 6 ++- ...winCATEquipmentTagParserStrictnessTests.cs | 49 +++++++++++++++++++ 4 files changed, 90 insertions(+), 7 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATEquipmentTagParserStrictnessTests.cs diff --git a/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json b/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json index 5fd80f7d..b25ae8bb 100644 --- a/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json +++ b/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json @@ -17,7 +17,7 @@ { "id": "T13", "subject": "S7 equipment-tag parser: freeze test, shared readers, honour writable key, Inspect warnings (+ amend Contracts banner)", "status": "completed", "blockedBy": ["T11"] }, { "id": "T14", "subject": "AbCip equipment-tag parser: freeze test, shared readers (writable already honoured), Inspect warnings", "status": "completed", "blockedBy": ["T11"] }, { "id": "T15", "subject": "AbLegacy equipment-tag parser: freeze test, shared readers, honour writable key, Inspect warnings", "status": "completed", "blockedBy": ["T11"] }, - { "id": "T16", "subject": "TwinCAT equipment-tag parser: freeze test, shared readers, honour writable key, Inspect warnings", "status": "pending", "blockedBy": ["T11"] }, + { "id": "T16", "subject": "TwinCAT equipment-tag parser: freeze test, shared readers, honour writable key, Inspect warnings", "status": "completed", "blockedBy": ["T11"] }, { "id": "T17", "subject": "FOCAS equipment-tag parser: force Writable:false (05/UNDER-1 correction), shared readers, Inspect warnings (+ amend Contracts banner)", "status": "pending", "blockedBy": ["T11"] }, { "id": "T18", "subject": "FOCAS capability-matrix pre-flight on the equipment-tag _parseRef resolve path (BadNodeIdUnknown on rejection)", "status": "pending", "blockedBy": ["T17"] }, { "id": "T19", "subject": "Modbus probe parses the factory DTO shape (timeoutMs round-trip; OpcUaClient parity rule)", "status": "pending", "blockedBy": [] }, diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATEquipmentTagParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATEquipmentTagParser.cs index 7fd03292..f4d620a4 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATEquipmentTagParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATEquipmentTagParser.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; @@ -28,15 +29,18 @@ public static class TwinCATEquipmentTagParser var symbolPath = symbol.GetString(); if (string.IsNullOrWhiteSpace(symbolPath)) return false; var deviceHostAddress = ReadString(root, "deviceHostAddress"); - var dataType = ReadEnum(root, "dataType", TwinCATDataType.DInt); + var dataType = TagConfigJson.ReadEnumOrDefault(root, "dataType", TwinCATDataType.DInt); // Array intent — same shape the runtime/OPC-UA foundation parses (camelCase // `isArray` bool + `arrayLength` uint). arrayLength is honoured ONLY when isArray // is true AND it is a positive JSON number, so a stale length behind a cleared // isArray never produces an orphan array tag (Phase 4c). var arrayLength = ReadArrayLength(root); + // "writable" defaults to true when absent (today's value) — makes read-only equipment tags + // authorable (UNDER-6). + var writable = TagConfigJson.ReadWritable(root); def = new TwinCATTagDefinition( Name: reference, DeviceHostAddress: deviceHostAddress, SymbolPath: symbolPath, - DataType: dataType, Writable: true, ArrayLength: arrayLength); + DataType: dataType, Writable: writable, ArrayLength: arrayLength); return true; } catch (JsonException) { return false; } @@ -44,9 +48,35 @@ public static class TwinCATEquipmentTagParser catch (InvalidOperationException) { return false; } } - private static TEnum ReadEnum(JsonElement o, string name, TEnum fallback) where TEnum : struct, Enum - => o.TryGetProperty(name, out var e) && e.ValueKind == JsonValueKind.String - && Enum.TryParse(e.GetString(), ignoreCase: true, out var v) ? v : fallback; + /// + /// Deploy-time inspection (05/CONV-2): warns on a present-but-invalid dataType (silently + /// defaulted by the lenient runtime) and on a structurally unparseable TagConfig. Empty when clean + /// or not an equipment-tag object. Never throws. + /// + /// The equipment tag's TagConfig JSON. + /// The warnings; empty when clean. + public static IReadOnlyList Inspect(string reference) + { + var warnings = new List(); + if (string.IsNullOrWhiteSpace(reference) || reference[0] != '{') return warnings; + try + { + using var doc = JsonDocument.Parse(reference); + var root = doc.RootElement; + if (root.ValueKind != JsonValueKind.Object) + { + warnings.Add("TwinCAT TagConfig root is not a JSON object — the tag will not resolve (BadNodeIdUnknown)."); + return warnings; + } + var w = TagConfigJson.DescribeInvalidEnum(root, "dataType"); + if (w is not null) warnings.Add(w); + } + catch (JsonException) + { + warnings.Add("TwinCAT TagConfig is not valid JSON — the tag will not resolve (BadNodeIdUnknown)."); + } + return warnings; + } private static string ReadString(JsonElement o, string name) => o.TryGetProperty(name, out var e) && e.ValueKind == JsonValueKind.String diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts.csproj b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts.csproj index d911061a..9dbcccee 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts.csproj +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts.csproj @@ -5,5 +5,9 @@ enable true - + + + + diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATEquipmentTagParserStrictnessTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATEquipmentTagParserStrictnessTests.cs new file mode 100644 index 00000000..4e298ea2 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATEquipmentTagParserStrictnessTests.cs @@ -0,0 +1,49 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; + +namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests; + +/// R2-11 (05/CONV-2) strictness surface for the TwinCAT equipment-tag parser: lenient runtime +/// freeze, Inspect reports the typo, and the writable key is honoured (default true). +public sealed class TwinCATEquipmentTagParserStrictnessTests +{ + [Fact] + public void Freeze_typo_dataType_still_defaults_to_DInt() + { + TwinCATEquipmentTagParser.TryParse("{\"symbolPath\":\"MAIN.x\",\"dataType\":\"DIntt\"}", out var def) + .ShouldBeTrue(); + def.DataType.ShouldBe(TwinCATDataType.DInt); + } + + [Fact] + public void Inspect_reports_typo_dataType() + { + var warnings = TwinCATEquipmentTagParser.Inspect("{\"symbolPath\":\"MAIN.x\",\"dataType\":\"DIntt\"}"); + warnings.ShouldHaveSingleItem(); + warnings[0].ShouldContain("DIntt"); + warnings[0].ShouldContain("dataType"); + } + + [Fact] + public void Inspect_clean_config_has_no_warnings() + { + TwinCATEquipmentTagParser.Inspect("{\"symbolPath\":\"MAIN.x\",\"dataType\":\"DInt\"}").ShouldBeEmpty(); + } + + [Fact] + public void Writable_false_is_honoured() + { + TwinCATEquipmentTagParser.TryParse("{\"symbolPath\":\"MAIN.x\",\"dataType\":\"DInt\",\"writable\":false}", out var def) + .ShouldBeTrue(); + def.Writable.ShouldBeFalse(); + } + + [Fact] + public void Writable_absent_defaults_to_true() + { + TwinCATEquipmentTagParser.TryParse("{\"symbolPath\":\"MAIN.x\",\"dataType\":\"DInt\"}", out var def) + .ShouldBeTrue(); + def.Writable.ShouldBeTrue(); + } +}