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 7d91bbad..32b0a5a3 100644 --- a/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json +++ b/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json @@ -15,7 +15,7 @@ { "id": "T11", "subject": "TagConfigJson strict-capable readers in Core.Abstractions (TryReadEnum Absent/Valid/Invalid, ReadEnumOrDefault, ReadWritable, DescribeInvalidEnum) (TDD)", "status": "completed", "blockedBy": [] }, { "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": "pending", "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": "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"] }, diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipEquipmentTagParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipEquipmentTagParser.cs index 7a5597d7..a474d0ff 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipEquipmentTagParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipEquipmentTagParser.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; @@ -42,7 +43,7 @@ public static class AbCipEquipmentTagParser // resolves the full tag path (e.g. "Motor.Speed") without enumerating UDT members. // The address space emits a placeholder String variable; UDT member declarations are // not supported in the equipment-tag flow. - var dataType = ReadEnum(root, "dataType", AbCipDataType.DInt); + var dataType = TagConfigJson.ReadEnumOrDefault(root, "dataType", AbCipDataType.DInt); // Review I-1 — an equipment tag is an ARRAY ⟺ isArray:true AND arrayLength >= 1. A // 1-element array (isArray:true, arrayLength:1) is a VALID 1-element array — the // foundation materialises a [1] OPC UA array node — so it must read as an array, not a @@ -50,8 +51,9 @@ public static class AbCipEquipmentTagParser // have a count of 1), so the explicit IsArray flag does. var (isArray, elementCount) = ReadArrayShape(root); // "writable" defaults to true when absent — matches AbCipTagDefinition.Writable default. - var writable = !root.TryGetProperty("writable", out var writableEl) - || writableEl.ValueKind != JsonValueKind.False; + // Now via the shared TagConfigJson.ReadWritable (explicit-false-only), byte-identical to the + // hand-rolled read it replaces. + var writable = TagConfigJson.ReadWritable(root); def = new AbCipTagDefinition( Name: reference, DeviceHostAddress: deviceHostAddress, TagPath: tagPath, DataType: dataType, Writable: writable, ElementCount: elementCount, IsArray: isArray); @@ -83,9 +85,35 @@ public static class AbCipEquipmentTagParser return (false, 1); } - 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("AbCip 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("AbCip 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.AbCip.Contracts/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts.csproj b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts.csproj index d911061a..9dbcccee 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts.csproj +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts.csproj @@ -5,5 +5,9 @@ enable true - + + + + diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipEquipmentTagParserStrictnessTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipEquipmentTagParserStrictnessTests.cs new file mode 100644 index 00000000..c5a84482 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipEquipmentTagParserStrictnessTests.cs @@ -0,0 +1,50 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// R2-11 (05/CONV-2) strictness surface for the AbCip equipment-tag parser: lenient runtime freeze, +/// Inspect reports the typo, and the already-honoured writable key stays bit-identical through +/// the shared reader swap. +public sealed class AbCipEquipmentTagParserStrictnessTests +{ + [Fact] + public void Freeze_typo_dataType_still_defaults_to_DInt() + { + AbCipEquipmentTagParser.TryParse("{\"tagPath\":\"Motor.Speed\",\"dataType\":\"DIntt\"}", out var def) + .ShouldBeTrue(); + def.DataType.ShouldBe(AbCipDataType.DInt); + } + + [Fact] + public void Inspect_reports_typo_dataType() + { + var warnings = AbCipEquipmentTagParser.Inspect("{\"tagPath\":\"Motor.Speed\",\"dataType\":\"DIntt\"}"); + warnings.ShouldHaveSingleItem(); + warnings[0].ShouldContain("DIntt"); + warnings[0].ShouldContain("dataType"); + } + + [Fact] + public void Inspect_clean_config_has_no_warnings() + { + AbCipEquipmentTagParser.Inspect("{\"tagPath\":\"Motor.Speed\",\"dataType\":\"DInt\"}").ShouldBeEmpty(); + } + + [Fact] + public void Writable_false_is_honoured_bit_identical() + { + AbCipEquipmentTagParser.TryParse("{\"tagPath\":\"Motor.Speed\",\"dataType\":\"DInt\",\"writable\":false}", out var def) + .ShouldBeTrue(); + def.Writable.ShouldBeFalse(); + } + + [Fact] + public void Writable_absent_defaults_to_true() + { + AbCipEquipmentTagParser.TryParse("{\"tagPath\":\"Motor.Speed\",\"dataType\":\"DInt\"}", out var def) + .ShouldBeTrue(); + def.Writable.ShouldBeTrue(); + } +}