From 6e2415bb4c8adc8b1b2b2933a2ec119aad7cba21 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:59:27 -0400 Subject: [PATCH] feat(driver-ablegacy): shared TagConfigJson readers, writable key, Inspect warnings (R2-11) --- ...tagconfig-consolidation-plan.md.tasks.json | 2 +- .../AbLegacyEquipmentTagParser.cs | 40 +++++++++++++-- ...W.OtOpcUa.Driver.AbLegacy.Contracts.csproj | 6 ++- ...LegacyEquipmentTagParserStrictnessTests.cs | 49 +++++++++++++++++++ 4 files changed, 90 insertions(+), 7 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyEquipmentTagParserStrictnessTests.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 32b0a5a3..5fd80f7d 100644 --- a/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json +++ b/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json @@ -16,7 +16,7 @@ { "id": "T12", "subject": "Modbus equipment-tag parser: freeze test, shared readers, honour writable key, Inspect warnings", "status": "completed", "blockedBy": ["T11"] }, { "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": "pending", "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": "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"] }, diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyEquipmentTagParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyEquipmentTagParser.cs index 22b7edbd..12469c9a 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyEquipmentTagParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyEquipmentTagParser.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; @@ -38,7 +39,7 @@ public static class AbLegacyEquipmentTagParser return false; var address = addr.GetString(); if (string.IsNullOrWhiteSpace(address)) return false; - var dataType = ReadEnum(root, "dataType", AbLegacyDataType.Int); + var dataType = TagConfigJson.ReadEnumOrDefault(root, "dataType", AbLegacyDataType.Int); var deviceHostAddress = ReadString(root, "deviceHostAddress"); int? arrayLength = null; if (IsArrayFlag(root)) @@ -47,9 +48,12 @@ public static class AbLegacyEquipmentTagParser if (rawLength >= 1) arrayLength = Math.Min(rawLength, AbLegacyArray.MaxElements); } + // "writable" defaults to true when absent (today's value) — makes read-only equipment tags + // authorable (UNDER-6). + var writable = TagConfigJson.ReadWritable(root); def = new AbLegacyTagDefinition( Name: reference, DeviceHostAddress: deviceHostAddress, Address: address, - DataType: dataType, Writable: true, ArrayLength: arrayLength); + DataType: dataType, Writable: writable, ArrayLength: arrayLength); return true; } catch (JsonException) { return false; } @@ -57,9 +61,35 @@ public static class AbLegacyEquipmentTagParser 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("AbLegacy 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("AbLegacy 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.AbLegacy.Contracts/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts.csproj b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts.csproj index d911061a..9dbcccee 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts.csproj +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts.csproj @@ -5,5 +5,9 @@ enable true - + + + + diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyEquipmentTagParserStrictnessTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyEquipmentTagParserStrictnessTests.cs new file mode 100644 index 00000000..d54019b7 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyEquipmentTagParserStrictnessTests.cs @@ -0,0 +1,49 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests; + +/// R2-11 (05/CONV-2) strictness surface for the AbLegacy equipment-tag parser: lenient runtime +/// freeze, Inspect reports the typo, and the writable key is honoured (default true). +public sealed class AbLegacyEquipmentTagParserStrictnessTests +{ + [Fact] + public void Freeze_typo_dataType_still_defaults_to_Int() + { + AbLegacyEquipmentTagParser.TryParse("{\"address\":\"N7:0\",\"dataType\":\"Intt\"}", out var def) + .ShouldBeTrue(); + def.DataType.ShouldBe(AbLegacyDataType.Int); + } + + [Fact] + public void Inspect_reports_typo_dataType() + { + var warnings = AbLegacyEquipmentTagParser.Inspect("{\"address\":\"N7:0\",\"dataType\":\"Intt\"}"); + warnings.ShouldHaveSingleItem(); + warnings[0].ShouldContain("Intt"); + warnings[0].ShouldContain("dataType"); + } + + [Fact] + public void Inspect_clean_config_has_no_warnings() + { + AbLegacyEquipmentTagParser.Inspect("{\"address\":\"N7:0\",\"dataType\":\"Int\"}").ShouldBeEmpty(); + } + + [Fact] + public void Writable_false_is_honoured() + { + AbLegacyEquipmentTagParser.TryParse("{\"address\":\"N7:0\",\"dataType\":\"Int\",\"writable\":false}", out var def) + .ShouldBeTrue(); + def.Writable.ShouldBeFalse(); + } + + [Fact] + public void Writable_absent_defaults_to_true() + { + AbLegacyEquipmentTagParser.TryParse("{\"address\":\"N7:0\",\"dataType\":\"Int\"}", out var def) + .ShouldBeTrue(); + def.Writable.ShouldBeTrue(); + } +}