feat(driver-modbus): shared TagConfigJson readers, writable key, Inspect warnings (R2-11)

archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json

@@ -13,7 +13,7 @@
     { "id": "T9",  "subject": "EquipmentNodeWalker.ExtractFullName delegates to TagConfigIntent; repoint tree-wide canonical-parser doc cites", "status": "completed", "blockedBy": ["T2", "T3"] },
     { "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": "pending", "blockedBy": ["T11"] },
+    { "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": "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"] },

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusEquipmentTagParser.cs

@@ -1,4 +1,5 @@
 using System.Text.Json;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
 
 namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus;
 
@@ -29,9 +30,9 @@ public static class ModbusEquipmentTagParser
                 || !addr.TryGetInt32(out var address)
                 || address < 0 || address > ushort.MaxValue)
                 return false;
-            var region = ReadEnum(root, "region", ModbusRegion.HoldingRegisters);
+            var region = TagConfigJson.ReadEnumOrDefault(root, "region", ModbusRegion.HoldingRegisters);
-            var dataType = ReadEnum(root, "dataType", ModbusDataType.Int16);
+            var dataType = TagConfigJson.ReadEnumOrDefault(root, "dataType", ModbusDataType.Int16);
-            var byteOrder = ReadEnum(root, "byteOrder", ModbusByteOrder.BigEndian);
+            var byteOrder = TagConfigJson.ReadEnumOrDefault(root, "byteOrder", ModbusByteOrder.BigEndian);
             var bitIndex = (byte)ReadInt(root, "bitIndex");
             var stringLength = (ushort)ReadInt(root, "stringLength");
             // Guard: String tags require StringLength >= 1. RegisterCount = (StringLength+1)/2,
@@ -51,9 +52,12 @@ public static class ModbusEquipmentTagParser
             var isArray = ReadBool(root, "isArray");
             var arrayLength = ReadInt(root, "arrayLength");
             int? arrayCount = (isArray && arrayLength >= 1) ? arrayLength : null;
+            // "writable" defaults to true when absent (today's value) — makes read-only equipment tags
+            // authorable (UNDER-6). Node-level authz remains the effective write gate.
+            var writable = TagConfigJson.ReadWritable(root);
             def = new ModbusTagDefinition(
                 Name: reference, Region: region, Address: (ushort)address, DataType: dataType,
-                Writable: true, ByteOrder: byteOrder, BitIndex: bitIndex, StringLength: stringLength,
+                Writable: writable, ByteOrder: byteOrder, BitIndex: bitIndex, StringLength: stringLength,
                 ArrayCount: arrayCount);
             return true;
         }
@@ -62,9 +66,44 @@ public static class ModbusEquipmentTagParser
         catch (InvalidOperationException) { return false; }
     }
 
-    private static TEnum ReadEnum<TEnum>(JsonElement o, string name, TEnum fallback) where TEnum : struct, Enum
+    /// <summary>
-        => o.TryGetProperty(name, out var e) && e.ValueKind == JsonValueKind.String
+    ///     Deploy-time inspection of an equipment-tag <c>TagConfig</c> blob (05/CONV-2). Returns
-           && Enum.TryParse<TEnum>(e.GetString(), ignoreCase: true, out var v) ? v : fallback;
+    ///     human-readable warnings for present-but-invalid enum values (<c>region</c> / <c>dataType</c> /
+    ///     <c>byteOrder</c> — which the lenient runtime silently defaults) and for a structurally
+    ///     unparseable TagConfig (which the runtime turns into a silent <c>BadNodeIdUnknown</c>). Empty when
+    ///     the blob is clean or is not an equipment-tag TagConfig object. Never throws.
+    /// </summary>
+    /// <param name="reference">The equipment tag's TagConfig JSON.</param>
+    /// <returns>The warnings; empty when clean.</returns>
+    public static IReadOnlyList<string> Inspect(string reference)
+    {
+        var warnings = new List<string>();
+        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("Modbus TagConfig root is not a JSON object — the tag will not resolve (BadNodeIdUnknown).");
+                return warnings;
+            }
+            foreach (var w in new[]
+            {
+                TagConfigJson.DescribeInvalidEnum<ModbusRegion>(root, "region"),
+                TagConfigJson.DescribeInvalidEnum<ModbusDataType>(root, "dataType"),
+                TagConfigJson.DescribeInvalidEnum<ModbusByteOrder>(root, "byteOrder"),
+            })
+            {
+                if (w is not null) warnings.Add(w);
+            }
+        }
+        catch (JsonException)
+        {
+            warnings.Add("Modbus 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

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts.csproj

@@ -12,6 +12,9 @@
          and was designed for exactly this use — Admin can reference it without transport-layer deps. -->
     <ItemGroup>
         <ProjectReference Include="..\ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing\ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.csproj"/>
+        <!-- R2-11 (05/CONV-2): shared TagConfigJson field readers live in the zero-dependency
+             Core.Abstractions leaf beside EquipmentTagRefResolver. -->
+        <ProjectReference Include="..\..\Core\ZB.MOM.WW.OtOpcUa.Core.Abstractions\ZB.MOM.WW.OtOpcUa.Core.Abstractions.csproj"/>
     </ItemGroup>
 
 </Project>

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusEquipmentTagParserStrictnessTests.cs

@@ -0,0 +1,71 @@
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Driver.Modbus;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests;
+
+/// <summary>
+///     R2-11 (05/CONV-2) strictness surface for the Modbus equipment-tag parser: the runtime stays lenient
+///     (freeze), the new <c>Inspect</c> reports the silently-defaulted typo, and the <c>writable</c> key is
+///     now honoured (default true).
+/// </summary>
+public sealed class ModbusEquipmentTagParserStrictnessTests
+{
+    // ---- Phase-A behaviour freeze: a typo'd enum still silently defaults ----
+
+    [Fact]
+    public void Freeze_typo_dataType_still_defaults_to_Int16()
+    {
+        ModbusEquipmentTagParser.TryParse(
+            "{\"region\":\"HoldingRegisters\",\"address\":1,\"dataType\":\"Intt16\"}", out var def)
+            .ShouldBeTrue();
+        def.DataType.ShouldBe(ModbusDataType.Int16);
+    }
+
+    // ---- Inspect reports the typo ----
+
+    [Fact]
+    public void Inspect_reports_typo_dataType_with_field_and_valid_values()
+    {
+        var warnings = ModbusEquipmentTagParser.Inspect(
+            "{\"region\":\"HoldingRegisters\",\"address\":1,\"dataType\":\"Intt16\"}");
+        warnings.ShouldHaveSingleItem();
+        warnings[0].ShouldContain("Intt16");
+        warnings[0].ShouldContain("dataType");
+        warnings[0].ShouldContain("Int16");
+    }
+
+    [Fact]
+    public void Inspect_clean_config_has_no_warnings()
+    {
+        ModbusEquipmentTagParser.Inspect(
+            "{\"region\":\"HoldingRegisters\",\"address\":1,\"dataType\":\"Int16\"}")
+            .ShouldBeEmpty();
+    }
+
+    [Fact]
+    public void Inspect_malformed_json_warns_unresolvable()
+    {
+        ModbusEquipmentTagParser.Inspect("{not json").ShouldHaveSingleItem();
+    }
+
+    // ---- writable now honoured ----
+
+    [Fact]
+    public void Writable_false_is_honoured()
+    {
+        ModbusEquipmentTagParser.TryParse(
+            "{\"region\":\"HoldingRegisters\",\"address\":1,\"dataType\":\"Int16\",\"writable\":false}", out var def)
+            .ShouldBeTrue();
+        def.Writable.ShouldBeFalse();
+    }
+
+    [Fact]
+    public void Writable_absent_defaults_to_true()
+    {
+        ModbusEquipmentTagParser.TryParse(
+            "{\"region\":\"HoldingRegisters\",\"address\":1,\"dataType\":\"Int16\"}", out var def)
+            .ShouldBeTrue();
+        def.Writable.ShouldBeTrue();
+    }
+}