fix(drivers): flip runtime tag parsers to strict enum parsing (#457)
v2-ci / build (pull_request) Successful in 4m12s
v2-ci / unit-tests (pull_request) Failing after 10m15s

R2-11 Phase C. All six equipment-tag parsers (Modbus/S7/AbCip/AbLegacy/
TwinCAT/Focas) now read enum fields via the new TagConfigJson.TryReadEnumStrict:
absent -> fallback, valid -> parsed, present-but-invalid (typo) -> TryParse
returns false -> EquipmentTagRefResolver.TryResolve false -> driver surfaces
BadNodeIdUnknown, instead of the old lenient path that silently defaulted a
typo to a wrong-width Good.

Modbus flips all three enum fields (region/dataType/byteOrder); the other five
flip dataType. The deploy-time Deployment:TagConfigValidationMode=Error gate is
unchanged and remains the operator pre-flight.

Coverage:
- Six *EquipmentTagParserStrictnessTests inverted from Freeze_typo_* (lenient)
  to Typo_*_rejects_the_tag + Valid_*_still_parses.
- TagConfigJsonTests.TryReadEnumStrict_rejects_only_invalid matrix.
- Driver-level end-to-end proof:
  AbCipEquipmentTagTests.Driver_read_of_a_typod_dataType_ref_surfaces_BadNodeIdUnknown
  drives the real AbCipDriver.ReadAsync through resolve->status.

Golden parity corpus has no typo'd enums, so the flip is a no-op there.
Full solution builds clean; all six driver suites + core + parity + gate green.
This commit is contained in:
Joseph Doherty
2026-07-15 11:28:34 -04:00
parent 57003a23a5
commit e8db0a683f
16 changed files with 193 additions and 38 deletions
@@ -43,7 +43,9 @@ 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 = TagConfigJson.ReadEnumOrDefault(root, "dataType", AbCipDataType.DInt);
// Strict enum read (R2-11 Phase C): a typo'd dataType rejects the tag (→ BadNodeIdUnknown)
// instead of silently defaulting to a wrong-width Good.
if (!TagConfigJson.TryReadEnumStrict(root, "dataType", AbCipDataType.DInt, out var dataType)) return false;
// 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
@@ -39,7 +39,9 @@ public static class AbLegacyEquipmentTagParser
return false;
var address = addr.GetString();
if (string.IsNullOrWhiteSpace(address)) return false;
var dataType = TagConfigJson.ReadEnumOrDefault(root, "dataType", AbLegacyDataType.Int);
// Strict enum read (R2-11 Phase C): a typo'd dataType rejects the tag (→ BadNodeIdUnknown)
// instead of silently defaulting to a wrong-width Good.
if (!TagConfigJson.TryReadEnumStrict(root, "dataType", AbLegacyDataType.Int, out var dataType)) return false;
var deviceHostAddress = ReadString(root, "deviceHostAddress");
int? arrayLength = null;
if (IsArrayFlag(root))
@@ -29,7 +29,9 @@ public static class FocasEquipmentTagParser
return false;
var address = addr.GetString();
if (string.IsNullOrWhiteSpace(address)) return false;
var dataType = TagConfigJson.ReadEnumOrDefault(root, "dataType", FocasDataType.Int32);
// Strict enum read (R2-11 Phase C): a typo'd dataType rejects the tag (→ BadNodeIdUnknown)
// instead of silently defaulting to a wrong-width Good.
if (!TagConfigJson.TryReadEnumStrict(root, "dataType", FocasDataType.Int32, out var dataType)) return false;
var deviceHostAddress = ReadString(root, "deviceHostAddress") ?? "";
def = new FocasTagDefinition(
Name: reference, DeviceHostAddress: deviceHostAddress, Address: address,
@@ -30,9 +30,11 @@ public static class ModbusEquipmentTagParser
|| !addr.TryGetInt32(out var address)
|| address < 0 || address > ushort.MaxValue)
return false;
var region = TagConfigJson.ReadEnumOrDefault(root, "region", ModbusRegion.HoldingRegisters);
var dataType = TagConfigJson.ReadEnumOrDefault(root, "dataType", ModbusDataType.Int16);
var byteOrder = TagConfigJson.ReadEnumOrDefault(root, "byteOrder", ModbusByteOrder.BigEndian);
// Strict enum reads (R2-11 Phase C): a present-but-invalid (typo'd) enum rejects the tag
// (→ BadNodeIdUnknown) instead of silently defaulting to a wrong-width Good.
if (!TagConfigJson.TryReadEnumStrict(root, "region", ModbusRegion.HoldingRegisters, out var region)) return false;
if (!TagConfigJson.TryReadEnumStrict(root, "dataType", ModbusDataType.Int16, out var dataType)) return false;
if (!TagConfigJson.TryReadEnumStrict(root, "byteOrder", ModbusByteOrder.BigEndian, out var byteOrder)) return false;
var bitIndex = (byte)ReadInt(root, "bitIndex");
var stringLength = (ushort)ReadInt(root, "stringLength");
// Guard: String tags require StringLength >= 1. RegisterCount = (StringLength+1)/2,
@@ -32,7 +32,9 @@ public static class S7EquipmentTagParser
return false;
var address = addrEl.GetString();
if (string.IsNullOrWhiteSpace(address)) return false;
var dataType = TagConfigJson.ReadEnumOrDefault(root, "dataType", S7DataType.Int16);
// Strict enum read (R2-11 Phase C): a typo'd dataType rejects the tag (→ BadNodeIdUnknown)
// instead of silently defaulting to a wrong-width Good.
if (!TagConfigJson.TryReadEnumStrict(root, "dataType", S7DataType.Int16, out var dataType)) return false;
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
@@ -29,7 +29,9 @@ public static class TwinCATEquipmentTagParser
var symbolPath = symbol.GetString();
if (string.IsNullOrWhiteSpace(symbolPath)) return false;
var deviceHostAddress = ReadString(root, "deviceHostAddress");
var dataType = TagConfigJson.ReadEnumOrDefault(root, "dataType", TwinCATDataType.DInt);
// Strict enum read (R2-11 Phase C): a typo'd dataType rejects the tag (→ BadNodeIdUnknown)
// instead of silently defaulting to a wrong-width Good.
if (!TagConfigJson.TryReadEnumStrict(root, "dataType", TwinCATDataType.DInt, out var dataType)) return false;
// 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