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 aef572f3..7d91bbad 100644 --- a/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json +++ b/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json @@ -14,7 +14,7 @@ { "id": "T10", "subject": "Retire OpcUaServer.Tests ExtractTag*Tests (authority moved to Commons.Tests); verify zero 'MUST parse identically' hits", "status": "completed", "blockedBy": ["T3", "T5"] }, { "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": "pending", "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": "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"] }, diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7EquipmentTagParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7EquipmentTagParser.cs index 8c1844e0..320db73b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7EquipmentTagParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7EquipmentTagParser.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.S7; @@ -31,7 +32,7 @@ public static class S7EquipmentTagParser return false; var address = addrEl.GetString(); if (string.IsNullOrWhiteSpace(address)) return false; - var dataType = ReadEnum(root, "dataType", S7DataType.Int16); + var dataType = TagConfigJson.ReadEnumOrDefault(root, "dataType", S7DataType.Int16); var stringLength = ReadInt(root, "stringLength"); // Range-guard applies only to String tags: an S7 string can't exceed 254 chars, and a // negative length is meaningless. For non-String types stringLength is irrelevant and any @@ -42,11 +43,14 @@ public static class S7EquipmentTagParser // that here so the driver's transient def agrees byte-for-byte with the materialised // OPC UA node's ValueRank/ArrayDimensions. Absent / isArray=false ⇒ null (scalar). var arrayCount = ReadArrayCount(root); + // "writable" defaults to true when absent (today's value); node-level authz still governs + // writes. Honouring the key makes read-only equipment tags authorable (UNDER-6). + var writable = TagConfigJson.ReadWritable(root); def = new S7TagDefinition( Name: reference, Address: address, DataType: dataType, - Writable: true, // node-level authz governs writes + Writable: writable, StringLength: stringLength == 0 ? MaxStringLength : stringLength, ArrayCount: arrayCount); return true; @@ -56,9 +60,35 @@ public static class S7EquipmentTagParser 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 (a silent + /// BadNodeIdUnknown at runtime). 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("S7 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("S7 TagConfig is not valid JSON — the tag will not resolve (BadNodeIdUnknown)."); + } + return warnings; + } private static int ReadInt(JsonElement o, string name) => o.TryGetProperty(name, out var e) && e.ValueKind == JsonValueKind.Number diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts.csproj b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts.csproj index 52900ddb..55d40545 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts.csproj +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts.csproj @@ -7,6 +7,11 @@ true - + + + + diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7EquipmentTagParserStrictnessTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7EquipmentTagParserStrictnessTests.cs new file mode 100644 index 00000000..78a46964 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7EquipmentTagParserStrictnessTests.cs @@ -0,0 +1,49 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.S7; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests; + +/// R2-11 (05/CONV-2) strictness surface for the S7 equipment-tag parser: lenient runtime freeze, +/// Inspect reports the typo, and the writable key is honoured (default true). +public sealed class S7EquipmentTagParserStrictnessTests +{ + [Fact] + public void Freeze_typo_dataType_still_defaults_to_Int16() + { + S7EquipmentTagParser.TryParse("{\"address\":\"DB1.DBW0\",\"dataType\":\"Intt16\"}", out var def) + .ShouldBeTrue(); + def.DataType.ShouldBe(S7DataType.Int16); + } + + [Fact] + public void Inspect_reports_typo_dataType() + { + var warnings = S7EquipmentTagParser.Inspect("{\"address\":\"DB1.DBW0\",\"dataType\":\"Intt16\"}"); + warnings.ShouldHaveSingleItem(); + warnings[0].ShouldContain("Intt16"); + warnings[0].ShouldContain("dataType"); + } + + [Fact] + public void Inspect_clean_config_has_no_warnings() + { + S7EquipmentTagParser.Inspect("{\"address\":\"DB1.DBW0\",\"dataType\":\"Int16\"}").ShouldBeEmpty(); + } + + [Fact] + public void Writable_false_is_honoured() + { + S7EquipmentTagParser.TryParse("{\"address\":\"DB1.DBW0\",\"dataType\":\"Int16\",\"writable\":false}", out var def) + .ShouldBeTrue(); + def.Writable.ShouldBeFalse(); + } + + [Fact] + public void Writable_absent_defaults_to_true() + { + S7EquipmentTagParser.TryParse("{\"address\":\"DB1.DBW0\",\"dataType\":\"Int16\"}", out var def) + .ShouldBeTrue(); + def.Writable.ShouldBeTrue(); + } +}