fix(drivers): correct 16 wrong OPC UA status-code constants + guard them by reflection (#497)
Every value verified against Opc.Ua.StatusCodes in the pinned SDK (1.5.378.106) by reflection, not by copying siblings. All are client-visible: OPC UA clients branch on status. The three named in #497: - Historian.Gateway SampleMapper "BadNoData" 0x800E0000 -> 0x809B0000 (0x800E0000 is BadServerHalted) - Galaxy "BadTimeout" 0x800B0000 -> 0x800A0000 (0x800B0000 is BadServiceUnsupported) - TwinCAT BadTypeMismatch 0x80730000 -> 0x80740000 (0x80730000 is BadWriteNotSupported) BadTypeMismatch was wrong in three MORE drivers the issue did not name — FOCAS, AbLegacy and AbCip carried the same 0x80730000. Every call site is a FormatException/InvalidCastException conversion failure, so the name was right and the value was wrong in all four. Adding the reflection guard then surfaced nine further defects offline tests had never checked: - Galaxy GoodLocalOverride 0x00D80000 -> 0x00960000 (not a UA code at all) - Galaxy UncertainLastUsableValue 0x40A40000 -> 0x40900000 - Galaxy UncertainSensorNotAccurate 0x408D0000 -> 0x40930000 - Galaxy UncertainEngineeringUnitsExceeded 0x408E0000 -> 0x40940000 - Galaxy UncertainSubNormal 0x408F0000 -> 0x40950000 - TwinCAT BadOutOfService 0x80BE0000 -> 0x808D0000 (was BadProtocolVersionUnsupported) - TwinCAT BadInvalidState 0x80350000 -> 0x80AF0000 (was BadAttributeIdInvalid) - AbCip/AbLegacy GoodMoreData 0x00A70000 -> 0x00A60000 (was GoodCommunicationEvent) - Historian.Gateway GatewayQualityMapper Good_LocalOverride 0x00D80000 -> 0x00960000 Galaxy's whole Uncertain block read as though the OPC DA quality byte could be shifted into the UA substatus position; it cannot — the substatuses are an unrelated enumeration. GatewayQualityMapper already held the correct table, which is what the Galaxy values are now reconciled against. Guard: StatusCodeParityTests (Core.Abstractions.Tests, which already project- references every driver) reflects over every `const uint` in the deployed ZB.MOM.WW.OtOpcUa.Driver.*.dll whose name reads as a status code, and asserts it equals Opc.Ua.StatusCodes.<name>. Discovery is by convention, so a new driver assembly referenced by that project is covered with no edit here. It went red on all nine defects above before the fixes and now checks 109 constants green. Because reflection can only see a NAMED constant, two inline literals were hoisted so the guard can reach them: Galaxy's BadTimeout/Bad into StatusCodeMap, and GatewayQualityMapper's 15-entry table into a new HistorianStatusCodes. An inline `0x800B0000u, // BadTimeout` at a call site is exactly how the Galaxy defect survived. The drivers stay SDK-free by design; only the test project references Opc.Ua.Core, as the oracle. Also corrected: tests that pinned the wrong values (Galaxy StatusCodeMapTests, SampleMapperTests, GatewayQualityMapperTests — all written from the same bad source), a mislabelled comment in DriverInstanceActorTests, and stale constants in two design docs, one of them the unexecuted MTConnect driver design that would have propagated BadNoData's wrong value into a new driver. The CLI's SnapshotFormatter value->name table was checked against the SDK and is correct; no change needed.
This commit is contained in:
Joseph Doherty
@@ -0,0 +1,160 @@
|
||||
using System.Reflection;
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Guards every driver's hard-coded OPC UA status-code constant against
|
||||
/// <see cref="Opc.Ua.StatusCodes"/> — the pinned SDK is the oracle, and a constant whose
|
||||
/// <em>name</em> disagrees with its <em>value</em> fails here.
|
||||
/// <para><b>Why the drivers hard-code these at all.</b> The driver layer is deliberately SDK-free: a
|
||||
/// driver assembly must not drag in the OPC UA stack just to spell a status code, so each one declares
|
||||
/// the handful it needs as bare <c>uint</c> literals. That is a sound layering decision with one
|
||||
/// failure mode — nothing checks the literal against the name — and it is exactly the failure mode
|
||||
/// Gitea #497 found in six places across four drivers.</para>
|
||||
/// <para><b>Discovery is reflection-driven, not a hand-copied list</b> (the same convention
|
||||
/// <see cref="DriverTypeNamesGuardTests"/> uses): the test scans every
|
||||
/// <c>ZB.MOM.WW.OtOpcUa.Driver.*.dll</c> deployed to its output directory for <c>const uint</c> fields
|
||||
/// whose name reads as a status code, so a brand-new driver assembly referenced by this project is
|
||||
/// covered automatically, with no edit here.</para>
|
||||
/// <para><b>An inline literal is invisible to this guard.</b> Reflection can only see a <em>named</em>
|
||||
/// constant, so <c>StatusCode: 0x800B0000u, // BadTimeout</c> written at a call site is unguarded —
|
||||
/// which is how <c>GalaxyDriver</c> shipped <c>BadServiceUnsupported</c> under a <c>BadTimeout</c>
|
||||
/// comment. Hoist a status literal into a named constant rather than writing it at the call site.</para>
|
||||
/// </summary>
|
||||
public sealed class StatusCodeParityTests
|
||||
{
|
||||
/// <summary>
|
||||
/// Name prefixes that mark a <c>uint</c> constant as an OPC UA status code. These are the three
|
||||
/// Part 4 severity categories, so they cover the whole space by construction — a status constant
|
||||
/// that does not start with one of them is not a status constant.
|
||||
/// </summary>
|
||||
private static readonly string[] StatusPrefixes = ["Good", "Uncertain", "Bad"];
|
||||
|
||||
/// <summary>
|
||||
/// Field-name prefixes stripped before the name is looked up on <see cref="Opc.Ua.StatusCodes"/>.
|
||||
/// Drivers vary the spelling (<c>SampleMapper.StatusBadNoData</c> vs
|
||||
/// <c>SqlStatusCodes.BadTimeout</c>); the SDK member is <c>BadNoData</c> either way.
|
||||
/// </summary>
|
||||
private static readonly string[] NamePrefixesToStrip = ["Status"];
|
||||
|
||||
/// <summary>Every <c>Opc.Ua.StatusCodes</c> member, by name — the oracle this test checks against.</summary>
|
||||
private static readonly IReadOnlyDictionary<string, uint> SdkStatusCodes =
|
||||
typeof(Opc.Ua.StatusCodes)
|
||||
.GetFields(BindingFlags.Public | BindingFlags.Static)
|
||||
.Where(f => f.IsLiteral && f.FieldType == typeof(uint))
|
||||
.ToDictionary(f => f.Name, f => (uint)f.GetRawConstantValue()!, StringComparer.Ordinal);
|
||||
|
||||
/// <summary>
|
||||
/// Every status-shaped <c>const uint</c> declared anywhere in the deployed driver assemblies, as
|
||||
/// <c>(assembly, declaring type, field name, SDK member name, declared value)</c>.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Non-public types and fields are included on purpose — <c>SampleMapper.StatusBadNoData</c> is a
|
||||
/// <c>private const</c> on an <c>internal</c> class, and it carried one of the #497 defects. Access
|
||||
/// modifiers say nothing about whether a value reaches an OPC UA client.
|
||||
/// </remarks>
|
||||
public static TheoryData<string, string, string, string, uint> DeclaredStatusConstants()
|
||||
{
|
||||
var data = new TheoryData<string, string, string, string, uint>();
|
||||
|
||||
var binDir = Path.GetDirectoryName(typeof(StatusCodeParityTests).Assembly.Location)!;
|
||||
foreach (var dll in Directory.GetFiles(binDir, "ZB.MOM.WW.OtOpcUa.Driver.*.dll").OrderBy(p => p, StringComparer.Ordinal))
|
||||
{
|
||||
Assembly asm;
|
||||
try { asm = Assembly.LoadFrom(dll); }
|
||||
catch { continue; }
|
||||
|
||||
Type?[] types;
|
||||
try { types = asm.GetTypes(); }
|
||||
catch (ReflectionTypeLoadException ex) { types = ex.Types; }
|
||||
|
||||
foreach (var type in types)
|
||||
{
|
||||
if (type is null) continue;
|
||||
|
||||
var fields = type.GetFields(
|
||||
BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.DeclaredOnly);
|
||||
|
||||
foreach (var field in fields)
|
||||
{
|
||||
if (!field.IsLiteral || field.IsInitOnly || field.FieldType != typeof(uint)) continue;
|
||||
|
||||
var sdkName = StripKnownPrefix(field.Name);
|
||||
if (!StatusPrefixes.Any(p => sdkName.StartsWith(p, StringComparison.Ordinal))) continue;
|
||||
|
||||
data.Add(
|
||||
asm.GetName().Name!,
|
||||
type.FullName ?? type.Name,
|
||||
field.Name,
|
||||
sdkName,
|
||||
(uint)field.GetRawConstantValue()!);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return data;
|
||||
}
|
||||
|
||||
/// <summary>Removes a leading <c>Status</c>-style prefix so the remainder can be looked up on the SDK type.</summary>
|
||||
private static string StripKnownPrefix(string fieldName)
|
||||
{
|
||||
foreach (var prefix in NamePrefixesToStrip)
|
||||
{
|
||||
if (fieldName.Length > prefix.Length && fieldName.StartsWith(prefix, StringComparison.Ordinal))
|
||||
return fieldName[prefix.Length..];
|
||||
}
|
||||
|
||||
return fieldName;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A driver's status constant must carry the value the SDK gives the code it is named after.
|
||||
/// </summary>
|
||||
/// <param name="assembly">The declaring driver assembly (failure-message context).</param>
|
||||
/// <param name="type">The declaring type's full name (failure-message context).</param>
|
||||
/// <param name="field">The constant's name as declared.</param>
|
||||
/// <param name="sdkName">The <see cref="Opc.Ua.StatusCodes"/> member the name resolves to.</param>
|
||||
/// <param name="declared">The value the driver declares.</param>
|
||||
[Theory]
|
||||
[MemberData(nameof(DeclaredStatusConstants))]
|
||||
public void Driver_status_constant_matches_the_pinned_SDK(
|
||||
string assembly, string type, string field, string sdkName, uint declared)
|
||||
{
|
||||
SdkStatusCodes.ShouldContainKey(sdkName,
|
||||
$"{type}.{field} (in {assembly}) reads as an OPC UA status code, but '{sdkName}' is not a member " +
|
||||
"of Opc.Ua.StatusCodes. Either the constant is misnamed, or it is not a status code and should " +
|
||||
"not start with Good/Uncertain/Bad.");
|
||||
|
||||
var expected = SdkStatusCodes[sdkName];
|
||||
declared.ShouldBe(expected,
|
||||
$"{type}.{field} (in {assembly}) is declared 0x{declared:X8}, but Opc.Ua.StatusCodes.{sdkName} " +
|
||||
$"is 0x{expected:X8}" +
|
||||
(SdkStatusCodes.FirstOrDefault(kv => kv.Value == declared) is { Key: { } actual } && actual.Length > 0
|
||||
? $" — 0x{declared:X8} is actually {actual}, which is what OPC UA clients would branch on."
|
||||
: ". The declared value is not any OPC UA status code."));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Discovery must find constants in more than one driver assembly. A zero (or near-zero) here means
|
||||
/// the bin scan broke or the drivers stopped declaring these by convention — either way the theory
|
||||
/// above would pass vacuously, which is the one outcome a guard test must never do quietly.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Discovery_finds_status_constants_across_multiple_driver_assemblies()
|
||||
{
|
||||
var found = DeclaredStatusConstants();
|
||||
found.Count.ShouldBeGreaterThan(20,
|
||||
"the driver bin scan found almost no status constants — the assemblies did not deploy to bin, " +
|
||||
"or the naming convention changed");
|
||||
|
||||
var assemblies = found
|
||||
.Select(row => row.Data.Item1)
|
||||
.Distinct(StringComparer.Ordinal)
|
||||
.ToArray();
|
||||
|
||||
assemblies.Length.ShouldBeGreaterThan(3,
|
||||
"status constants were found in fewer than four driver assemblies: " + string.Join(", ", assemblies));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user