fix(dcl): coerce scalar OPC UA writes to the node DataType (#58)

Codec-encoded scalars ("True", "42.5") reached RealOpcUaClient.WriteValuesAsync as strings and were sent as String Variants, so a Boolean/Double node answered BadTypeMismatch (0x80740000). RealOpcUaClient now reads each written node's DataType attribute once per session (batched, cached, cleared on connect/disconnect; derived types and enumerations resolved via the supertype chain) and converts non-null scalar values with the new OpcUaWriteCoercion table: Boolean, SByte..UInt64, Float, Double, String, DateTime, Guid, all culture-invariant. Overflow, fractional values to integer types and unparseable strings are refused client-side with BadTypeMismatch and a message naming the value and target type - never a rounded value. Arrays, null, unmapped types and nodes whose DataType read fails pass through unchanged. WriteValueAsync now surfaces the outcome's own status code instead of always BadNodeIdUnknown. MxGateway path untouched. Tests: OpcUaWriteCoercionTests (coercion table incl. overflow, invalid and culture-sensitive strings) and OpcUaWriteCoercionLiveTests (RequiresOpcUa: String writes to Double/Boolean/UInt32 opc-plc nodes read back typed; reverting the coercion reproduces 0x80740000). Component-DataConnectionLayer.md updated.
2026-11-06 16:08:33 -05:00
parent 0e89966da6
commit a5c3de63e9
6 changed files with 619 additions and 11 deletions
@@ -0,0 +1,197 @@
+using System.Collections.Generic;
+using System.Globalization;
+using Opc.Ua;
+using ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Adapters;
+
+namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests.Adapters;
+
+/// <summary>
+/// Gitea #58: the scalar write-coercion table <see cref="OpcUaWriteCoercion"/> that
+/// <see cref="RealOpcUaClient.WriteValuesAsync"/> applies before wrapping a value in a
+/// <see cref="Variant"/>. Each success case asserts the exact CLR type (the Variant's built-in
+/// type follows from it); each failure case asserts no value is produced.
+/// </summary>
+[Trait("Category", "Unit")]
+public class OpcUaWriteCoercionTests
+{
+    public static TheoryData<object, BuiltInType, object> Successes => new()
+    {
+        // Boolean
+        { "True", BuiltInType.Boolean, true },
+        { "false", BuiltInType.Boolean, false },
+        { " TRUE ", BuiltInType.Boolean, true },
+        { "1", BuiltInType.Boolean, true },
+        { "0", BuiltInType.Boolean, false },
+        { 1, BuiltInType.Boolean, true },
+        { true, BuiltInType.Boolean, true },
+        // Signed integers
+        { "-128", BuiltInType.SByte, (sbyte)-128 },
+        { "32767", BuiltInType.Int16, (short)32767 },
+        { "-42", BuiltInType.Int32, -42 },
+        { "42.0", BuiltInType.Int32, 42 },
+        { 7L, BuiltInType.Int32, 7 },
+        { 3.0, BuiltInType.Int32, 3 },
+        { "-9223372036854775808", BuiltInType.Int64, long.MinValue },
+        { "7", BuiltInType.Enumeration, 7 },
+        // Unsigned integers
+        { "255", BuiltInType.Byte, (byte)255 },
+        { "65535", BuiltInType.UInt16, (ushort)65535 },
+        { "4294967295", BuiltInType.UInt32, uint.MaxValue },
+        { 5, BuiltInType.UInt32, 5u },
+        { "18446744073709551615", BuiltInType.UInt64, ulong.MaxValue },
+        // Floating point (invariant culture: '.' is the decimal separator)
+        { "42.5", BuiltInType.Double, 42.5 },
+        { "-1.5e3", BuiltInType.Double, -1500.0 },
+        { 42, BuiltInType.Double, 42.0 },
+        { 1.25f, BuiltInType.Double, 1.25 },
+        { "Infinity", BuiltInType.Double, double.PositiveInfinity },
+        { "42.5", BuiltInType.Float, 42.5f },
+        { 2.5, BuiltInType.Float, 2.5f },
+        // String
+        { "hello", BuiltInType.String, "hello" },
+        { 42.5, BuiltInType.String, "42.5" },
+        { true, BuiltInType.String, "True" },
+        // Unmapped target types pass through untouched
+        { "abc", BuiltInType.Variant, "abc" },
+        { "abc", BuiltInType.ExtensionObject, "abc" },
+    };
+
+    [Theory]
+    [MemberData(nameof(Successes))]
+    public void Coerces_to_the_node_type(object input, BuiltInType target, object expected)
+    {
+        var result = OpcUaWriteCoercion.Coerce(input, target);
+
+        Assert.True(result.Success, result.Error);
+        Assert.Equal(expected.GetType(), result.Value!.GetType());
+        Assert.Equal(expected, result.Value);
+    }
+
+    public static TheoryData<object, BuiltInType> Failures => new()
+    {
+        // Invalid strings
+        { "yes", BuiltInType.Boolean },
+        { "2", BuiltInType.Boolean },
+        { 2, BuiltInType.Boolean },
+        { "abc", BuiltInType.Int32 },
+        { "", BuiltInType.Int32 },
+        { "abc", BuiltInType.Double },
+        { "not-a-date", BuiltInType.DateTime },
+        { "zzz", BuiltInType.Guid },
+        // Culture: a comma is not a decimal separator, and thousands separators are refused
+        { "42,5", BuiltInType.Double },
+        { "1,000", BuiltInType.Int32 },
+        // Fractional value to an integer type — never rounded
+        { "42.5", BuiltInType.Int32 },
+        { 42.5, BuiltInType.Int64 },
+        { double.NaN, BuiltInType.Int32 },
+        // Overflow
+        { "128", BuiltInType.SByte },
+        { "256", BuiltInType.Byte },
+        { "-1", BuiltInType.Byte },
+        { "32768", BuiltInType.Int16 },
+        { "-1", BuiltInType.UInt16 },
+        { "2147483648", BuiltInType.Int32 },
+        { 4294967296L, BuiltInType.UInt32 },
+        { "-1", BuiltInType.UInt32 },
+        { "9223372036854775808", BuiltInType.Int64 },
+        { "18446744073709551616", BuiltInType.UInt64 },
+        { -1, BuiltInType.UInt64 },
+        { 1e30, BuiltInType.Int64 },
+        { "1e999", BuiltInType.Double },
+        { "1e39", BuiltInType.Float },
+        { 1e39, BuiltInType.Float },
+    };
+
+    [Theory]
+    [MemberData(nameof(Failures))]
+    public void Rejects_values_that_do_not_fit_the_node_type(object input, BuiltInType target)
+    {
+        var result = OpcUaWriteCoercion.Coerce(input, target);
+
+        Assert.False(result.Success);
+        Assert.Null(result.Value);
+        Assert.Contains(target == BuiltInType.Enumeration ? "Int32" : target.ToString(), result.Error);
+    }
+
+    [Fact]
+    public void Error_names_the_value_and_target_type()
+    {
+        var result = OpcUaWriteCoercion.Coerce("300", BuiltInType.Byte);
+
+        Assert.False(result.Success);
+        Assert.Contains("\"300\"", result.Error);
+        Assert.Contains("Byte", result.Error);
+        Assert.Contains("out of range", result.Error);
+    }
+
+    [Fact]
+    public void DateTime_strings_parse_invariantly_as_utc()
+    {
+        var withOffset = OpcUaWriteCoercion.Coerce("2026-11-06T10:00:00+02:00", BuiltInType.DateTime);
+        var noOffset = OpcUaWriteCoercion.Coerce("2026-11-06T08:00:00", BuiltInType.DateTime);
+
+        var expected = new DateTime(2026, 11, 6, 8, 0, 0, DateTimeKind.Utc);
+        Assert.Equal(expected, withOffset.Value);
+        Assert.Equal(DateTimeKind.Utc, ((DateTime)withOffset.Value!).Kind);
+        Assert.Equal(expected, noOffset.Value);
+        Assert.Equal(DateTimeKind.Utc, ((DateTime)noOffset.Value!).Kind);
+    }
+
+    [Fact]
+    public void DateTime_to_string_uses_round_trip_format()
+    {
+        var dt = new DateTime(2026, 11, 6, 8, 0, 0, DateTimeKind.Utc);
+
+        Assert.Equal("2026-11-06T08:00:00.0000000Z", OpcUaWriteCoercion.Coerce(dt, BuiltInType.String).Value);
+    }
+
+    [Fact]
+    public void Parsing_ignores_the_current_culture()
+    {
+        var saved = CultureInfo.CurrentCulture;
+        try
+        {
+            // de-DE uses ',' as the decimal separator; the invariant parse must still read "42.5".
+            CultureInfo.CurrentCulture = new CultureInfo("de-DE");
+            Assert.Equal(42.5, OpcUaWriteCoercion.Coerce("42.5", BuiltInType.Double).Value);
+            Assert.Equal("42.5", OpcUaWriteCoercion.Coerce(42.5, BuiltInType.String).Value);
+        }
+        finally
+        {
+            CultureInfo.CurrentCulture = saved;
+        }
+    }
+
+    [Fact]
+    public void Guid_strings_parse()
+    {
+        var g = Guid.NewGuid();
+        Assert.Equal(g, OpcUaWriteCoercion.Coerce(g.ToString(), BuiltInType.Guid).Value);
+    }
+
+    [Fact]
+    public void Arrays_and_null_pass_through_untouched()
+    {
+        var list = new List<int> { 1, 2 };
+        var array = new[] { "1", "2" };
+
+        Assert.Same(list, OpcUaWriteCoercion.Coerce(list, BuiltInType.Double).Value);
+        Assert.Same(array, OpcUaWriteCoercion.Coerce(array, BuiltInType.Int32).Value);
+        Assert.True(OpcUaWriteCoercion.Coerce(null, BuiltInType.Int32).Success);
+        Assert.Null(OpcUaWriteCoercion.Coerce(null, BuiltInType.Int32).Value);
+        Assert.False(OpcUaWriteCoercion.IsCoercibleScalar(list));
+        Assert.True(OpcUaWriteCoercion.IsCoercibleScalar("abc"));
+    }
+
+    [Fact]
+    public void Coerced_string_becomes_a_typed_variant()
+    {
+        // The #58 failure: new Variant("True") is a String Variant. After coercion it is Boolean.
+        var coerced = OpcUaWriteCoercion.Coerce("True", BuiltInType.Boolean).Value;
+
+        Assert.Equal(BuiltInType.String, new Variant("True").TypeInfo.BuiltInType);
+        Assert.Equal(BuiltInType.Boolean, new Variant(coerced).TypeInfo.BuiltInType);
+        Assert.Equal(ValueRanks.Scalar, new Variant(coerced).TypeInfo.ValueRank);
+    }
+}
@@ -0,0 +1,71 @@
+using ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Adapters;
+
+namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests;
+
+/// <summary>
+/// Gitea #58 live round-trip: a codec-encoded String scalar written to a numeric / Boolean
+/// node of the infra OPC PLC server (<c>opc.tcp://localhost:50000</c>, see
+/// <c>infra/docker-compose.yml</c> + <c>infra/opcua/nodes.json</c>) is coerced to the node's
+/// DataType and reads back as that type. Before the fix the server answered
+/// <c>BadTypeMismatch</c> (0x80740000). Skips when the server is unreachable.
+/// </summary>
+[Trait("Category", "RequiresOpcUa")]
+public class OpcUaWriteCoercionLiveTests
+{
+    private const string EndpointUrl = "opc.tcp://localhost:50000";
+
+    // Writable, non-simulated nodes from infra/opcua/nodes.json (namespace http://microsoft.com/Opc/OpcPlc/).
+    private const string DoubleNode = "nsu=http://microsoft.com/Opc/OpcPlc/;s=Motor.Speed";
+    private const string BooleanNode = "nsu=http://microsoft.com/Opc/OpcPlc/;s=Motor.Running";
+    private const string UInt32Node = "nsu=http://microsoft.com/Opc/OpcPlc/;s=Motor.FaultCode";
+
+    [SkippableFact]
+    public async Task String_writes_to_typed_nodes_are_coerced_and_read_back_typed()
+    {
+        await using var client = new RealOpcUaClient();
+        using var deadline = new CancellationTokenSource(TimeSpan.FromSeconds(30));
+        try
+        {
+            await client.ConnectAsync(EndpointUrl, new OpcUaConnectionOptions(OperationTimeoutMs: 5000), deadline.Token);
+        }
+        catch (Exception ex)
+        {
+            Skip.If(true, $"OPC UA test server unavailable: {ex.Message}");
+            return;
+        }
+
+        // Two distinct values per node so the read-back cannot pass on a stale value.
+        foreach (var (speed, running, fault) in new[] { ("1234.5", "True", "17"), ("42.25", "False", "0") })
+        {
+            var outcomes = await client.WriteValuesAsync(
+                [(DoubleNode, speed), (BooleanNode, running), (UInt32Node, fault)], deadline.Token);
+
+            Assert.All(outcomes, o =>
+            {
+                Assert.Null(o.Error);
+                Assert.Equal(0u, o.StatusCode);
+            });
+
+            var reads = await client.ReadValuesAsync([DoubleNode, BooleanNode, UInt32Node], deadline.Token);
+            Assert.Equal(double.Parse(speed, System.Globalization.CultureInfo.InvariantCulture),
+                Assert.IsType<double>(reads[0].Value));
+            Assert.Equal(bool.Parse(running), Assert.IsType<bool>(reads[1].Value));
+            Assert.Equal(uint.Parse(fault, System.Globalization.CultureInfo.InvariantCulture),
+                Assert.IsType<uint>(reads[2].Value));
+        }
+
+        // A value that does not fit the node type is refused client-side with a clear error,
+        // and the node keeps its previous value.
+        var bad = await client.WriteValuesAsync([(UInt32Node, "-1"), (DoubleNode, "abc")], deadline.Token);
+        Assert.All(bad, o =>
+        {
+            Assert.Equal(Opc.Ua.StatusCodes.BadTypeMismatch, o.StatusCode);
+            Assert.Contains("Cannot convert", o.Error);
+        });
+        var after = await client.ReadValuesAsync([UInt32Node, DoubleNode], deadline.Token);
+        Assert.Equal(0u, after[0].Value);
+        Assert.Equal(42.25, after[1].Value);
+
+        await client.DisconnectAsync();
+    }
+}