feat(modbus): wire Family / MelsecSubFamily / DisableFC23, and the form's timing keys

U1: a tag's addressString (Modicon / mnemonic / DL205 V-memory / MELSEC D-X-Y grammar) now resolves through ModbusAddressParser with the DRIVER's Family and MelsecSubFamily at every initialize and is authoritative; the family-unaware gates (deploy Inspect, AdminUI parse consult) accept what some family resolves. The tag editor gets an address-text field that resolves with the owning driver's family and persists both the string and the resolved region/offset/type fields; a hand edit drops the string. DisableFC23 now gates FC23: by default a 2..121-register holding write is one FC23 write + read-back (a unit answering exception 01, or failing FC23 at the transport, moves to FC16 for the rest of the generation); RTU framing deframes the FC23 reply. Live-checked: the pymodbus fixture implements FC23. Also: the AdminUI driver form serialises ModbusDriverOptions (TimeSpan strings), which the factory DTO never read - every timing field on the form was dead. The DTO now accepts both shapes (*Ms keys win). Refs #741 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
2026-10-22 15:35:58 -04:00
parent 1adc9d65e1
commit 4a3d7a9361
13 changed files with 656 additions and 58 deletions
@@ -533,7 +533,7 @@ public sealed class ModbusDriver
            _skippedTags.BeginRebuild(_options.RawTags.Count);
            foreach (var entry in _options.RawTags)
            {
-                if (ModbusTagDefinitionFactory.FromTagConfig(entry.TagConfig, entry.RawPath, out var def))
+                if (ModbusTagDefinitionFactory.FromTagConfig(entry.TagConfig, entry.RawPath, _options.Family, _options.MelsecSubFamily, out var def))
                    table[entry.RawPath] = def with { WriteIdempotent = entry.WriteIdempotent };
                else
                {
@@ -556,6 +556,7 @@ public sealed class ModbusDriver
                _transport = transport;
                Volatile.Write(ref _tagsByRawPath, table);
                Volatile.Write(ref _refusalWarned, new ConcurrentDictionary<string, byte>(StringComparer.Ordinal));
+                Volatile.Write(ref _fc23Unsupported, new ConcurrentDictionary<byte, byte>());
                WriteHealth(new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null));

                // PR 23: kick off the probe loop once the transport is up. Initial state stays
@@ -1619,6 +1620,7 @@ public sealed class ModbusDriver
                        throw new ModbusWriteRefusedException(StatusBadOutOfRange,
                            $"Write of {qty} registers to {tag.Name} exceeds MaxRegistersPerWrite={writeCap}. " +
                            $"Split the tag (e.g. shorter StringLength or smaller ArrayCount) — partial FC16 chunks would lose atomicity.");
+                    if (await TryWriteViaFc23Async(transport, tag, bytes, qty, ct).ConfigureAwait(false)) return;
                    var pdu = new byte[6 + 1 + bytes.Length];
                    pdu[0] = 0x10;
                    pdu[1] = (byte)(tag.Address >> 8); pdu[2] = (byte)(tag.Address & 0xFF);
@@ -1634,6 +1636,79 @@ public sealed class ModbusDriver
        }
    }

+    /// <summary>FC23's spec ceiling on the write quantity (0x79); larger multi-register writes use FC16.</summary>
+    private const ushort Fc23MaxWriteRegisters = 121;
+
+    /// <summary>
+    ///     Units that refused FC23 (exception 01) or failed it at the transport; their multi-register writes
+    ///     use FC16 for the rest of this driver generation. Replaced (never cleared in place) on re-init.
+    /// </summary>
+    private ConcurrentDictionary<byte, byte> _fc23Unsupported = new();
+
+    /// <summary>
+    ///     Multi-register write via FC23 (Read/Write Multiple Registers): the device writes the block and
+    ///     returns the SAME block read back in one transaction, so the write is confirmed by the device's own
+    ///     post-write registers. Gated by <see cref="ModbusDriverOptions.DisableFC23"/>. A device that does
+    ///     not implement FC23 (MELSEC QJ71MT91, S7 Modbus servers — exception 01) falls back to FC16 for this
+    ///     write and every later one on that unit, which is safe because an Illegal Function wrote nothing.
+    ///     A transport failure on FC23 is reported (its outcome is unknown, so it is not retried as FC16) and
+    ///     also moves the unit to FC16, so a gateway that cannot frame FC23 costs one failed write, not all.
+    ///     A read-back that differs from what was written (PLC logic overwrote it in the same scan, or the
+    ///     device clamped it) is logged once per tag; the write itself was accepted.
+    /// </summary>
+    /// <returns><see langword="true"/> when FC23 performed the write; <see langword="false"/> to use FC16.</returns>
+    private async Task<bool> TryWriteViaFc23Async(
+        IModbusTransport transport, ModbusTagDefinition tag, byte[] bytes, ushort qty, CancellationToken ct)
+    {
+        var unit = ResolveUnitId(tag);
+        var unsupported = Volatile.Read(ref _fc23Unsupported);
+        // Only genuinely multi-register writes: a single register forced onto FC16 (UseFC16ForSingleRegisterWrites)
+        // stays on the function code the operator named.
+        if (_options.DisableFC23 || qty < 2 || qty > Fc23MaxWriteRegisters || unsupported.ContainsKey(unit)) return false;
+
+        var pdu = new byte[10 + 1 + bytes.Length];
+        pdu[0] = 0x17;
+        pdu[1] = (byte)(tag.Address >> 8); pdu[2] = (byte)(tag.Address & 0xFF); // read start
+        pdu[3] = (byte)(qty >> 8); pdu[4] = (byte)(qty & 0xFF);                 // read quantity
+        pdu[5] = (byte)(tag.Address >> 8); pdu[6] = (byte)(tag.Address & 0xFF); // write start
+        pdu[7] = (byte)(qty >> 8); pdu[8] = (byte)(qty & 0xFF);                 // write quantity
+        pdu[9] = (byte)bytes.Length;
+        Buffer.BlockCopy(bytes, 0, pdu, 10, bytes.Length);
+
+        byte[] resp;
+        try
+        {
+            resp = await transport.SendAsync(unit, pdu, ct).ConfigureAwait(false);
+        }
+        catch (ModbusException mex) when (mex.ExceptionCode == 0x01)
+        {
+            if (unsupported.TryAdd(unit, 0))
+                _logger.LogInformation(
+                    "Modbus unit {Unit} does not implement FC23 (Illegal Function); multi-register writes use FC16. Driver={DriverInstanceId}",
+                    unit, _driverInstanceId);
+            return false;
+        }
+        catch (Exception ex) when (ex is not ModbusException && !(ex is OperationCanceledException && ct.IsCancellationRequested))
+        {
+            if (unsupported.TryAdd(unit, 0))
+                _logger.LogWarning(ex,
+                    "Modbus FC23 write to unit {Unit} failed at the transport; later multi-register writes to it use FC16. Driver={DriverInstanceId}",
+                    unit, _driverInstanceId);
+            throw;
+        }
+
+        // resp = [0x17][byteCount][read-back registers…]
+        if (resp.Length < 2 || resp[1] != bytes.Length || resp.Length < 2 + resp[1])
+            throw new InvalidDataException(
+                $"Modbus FC23 response for {tag.Name} malformed: expected {bytes.Length} read-back bytes, got {(resp.Length < 2 ? -1 : resp[1])}");
+        if (!resp.AsSpan(2, bytes.Length).SequenceEqual(bytes)
+            && Volatile.Read(ref _refusalWarned).TryAdd("fc23:" + tag.Name, 0))
+            _logger.LogWarning(
+                "Modbus FC23 read-back of {RawPath} differs from the value written (device logic or clamping changed it); the write was accepted. Driver={DriverInstanceId}",
+                tag.Name, _driverInstanceId);
+        return true;
+    }
+
    /// <summary>
    ///     Encode an array-typed write value into a contiguous byte block by encoding each
    ///     element with the scalar codec. Caller submits IList / Array of the element CLR type.
@@ -78,7 +78,7 @@ public static class ModbusDriverFactoryExtensions
            Host = dto.Host!,
            Port = dto.Port ?? 502,
            UnitId = dto.UnitId ?? 1,
-            Timeout = TimeSpan.FromMilliseconds(dto.TimeoutMs ?? 2_000),
+            Timeout = Span(dto.TimeoutMs, dto.Timeout) ?? TimeSpan.FromSeconds(2),
            MaxRegistersPerRead = dto.MaxRegistersPerRead ?? 125,
            MaxRegistersPerWrite = dto.MaxRegistersPerWrite ?? 123,
            MaxCoilsPerRead = dto.MaxCoilsPerRead ?? 2000,
@@ -94,7 +94,7 @@ public static class ModbusDriverFactoryExtensions
                : ParseEnum<MelsecFamily>(dto.MelsecSubFamily, "<driver-level>", driverInstanceId, "MelsecSubFamily"),
            Transport = dto.Transport is null ? ModbusTransportMode.Tcp
                : ParseEnum<ModbusTransportMode>(dto.Transport, "<driver-level>", driverInstanceId, "Transport"),
-            AutoProhibitReprobeInterval = dto.AutoProhibitReprobeMs is { } reprobeMs ? TimeSpan.FromMilliseconds(reprobeMs) : null,
+            AutoProhibitReprobeInterval = Span(dto.AutoProhibitReprobeMs, dto.AutoProhibitReprobeInterval),
            AutoReconnect = dto.AutoReconnect ?? true,
            // v3: the deploy artifact (Wave C) populates rawTags with the cluster's authored raw Modbus
            // tags. The driver maps each RawTagEntry's TagConfig blob through ModbusTagDefinitionFactory
@@ -104,22 +104,22 @@ public static class ModbusDriverFactoryExtensions
            Probe = new ModbusProbeOptions
            {
                Enabled = dto.Probe?.Enabled ?? true,
-                Interval = TimeSpan.FromMilliseconds(dto.Probe?.IntervalMs ?? 5_000),
-                Timeout = TimeSpan.FromMilliseconds(dto.Probe?.TimeoutMs ?? 2_000),
+                Interval = Span(dto.Probe?.IntervalMs, dto.Probe?.Interval) ?? TimeSpan.FromSeconds(5),
+                Timeout = Span(dto.Probe?.TimeoutMs, dto.Probe?.Timeout) ?? TimeSpan.FromSeconds(2),
                ProbeAddress = dto.Probe?.ProbeAddress ?? 0,
            },
            KeepAlive = dto.KeepAlive is null ? new ModbusKeepAliveOptions() : new ModbusKeepAliveOptions
            {
                Enabled = dto.KeepAlive.Enabled ?? true,
-                Time = TimeSpan.FromMilliseconds(dto.KeepAlive.TimeMs ?? 30_000),
-                Interval = TimeSpan.FromMilliseconds(dto.KeepAlive.IntervalMs ?? 10_000),
+                Time = Span(dto.KeepAlive.TimeMs, dto.KeepAlive.Time) ?? TimeSpan.FromSeconds(30),
+                Interval = Span(dto.KeepAlive.IntervalMs, dto.KeepAlive.Interval) ?? TimeSpan.FromSeconds(10),
                RetryCount = dto.KeepAlive.RetryCount ?? 3,
            },
-            IdleDisconnectTimeout = dto.IdleDisconnectMs is { } ms ? TimeSpan.FromMilliseconds(ms) : null,
+            IdleDisconnectTimeout = Span(dto.IdleDisconnectMs, dto.IdleDisconnectTimeout),
            Reconnect = dto.Reconnect is null ? new ModbusReconnectOptions() : new ModbusReconnectOptions
            {
-                InitialDelay = TimeSpan.FromMilliseconds(dto.Reconnect.InitialDelayMs ?? 0),
-                MaxDelay = TimeSpan.FromMilliseconds(dto.Reconnect.MaxDelayMs ?? 30_000),
+                InitialDelay = Span(dto.Reconnect.InitialDelayMs, dto.Reconnect.InitialDelay) ?? TimeSpan.Zero,
+                MaxDelay = Span(dto.Reconnect.MaxDelayMs, dto.Reconnect.MaxDelay) ?? TimeSpan.FromSeconds(30),
                BackoffMultiplier = dto.Reconnect.BackoffMultiplier ?? 2.0,
            },
        };
@@ -127,6 +127,23 @@ public static class ModbusDriverFactoryExtensions
        return options;
    }

+    /// <summary>
+    ///     One timing knob, accepted in BOTH shapes the config can carry: the <c>*Ms</c> integer keys (CLI,
+    ///     seed data, hand-authored JSON) and the TimeSpan-string keys the AdminUI driver form emits — it
+    ///     serialises <see cref="ModbusDriverOptions"/> itself, so it writes <c>"timeout":"00:00:07"</c>, not
+    ///     <c>timeoutMs</c>. Before this every timing field on that form was silently ignored. The
+    ///     millisecond key wins when both are present; an unparseable TimeSpan string is ignored (the
+    ///     previous behaviour for these keys), never a reason to fail the driver.
+    /// </summary>
+    private static TimeSpan? Span(int? milliseconds, JsonElement? timeSpan)
+    {
+        if (milliseconds is { } ms) return TimeSpan.FromMilliseconds(ms);
+        if (timeSpan is { ValueKind: JsonValueKind.String } e
+            && TimeSpan.TryParse(e.GetString(), System.Globalization.CultureInfo.InvariantCulture, out var parsed))
+            return parsed;
+        return null;
+    }
+
    private static T ParseEnum<T>(string? raw, string? tagName, string driverInstanceId, string field) where T : struct, Enum
    {
        if (string.IsNullOrWhiteSpace(raw))
@@ -156,6 +173,8 @@ public static class ModbusDriverFactoryExtensions
        public byte? UnitId { get; init; }
        /// <summary>Gets or sets the timeout in milliseconds.</summary>
        public int? TimeoutMs { get; init; }
+        /// <summary>Gets or sets the timeout as a TimeSpan string (the AdminUI form's shape).</summary>
+        public JsonElement? Timeout { get; init; }
        /// <summary>Gets or sets the maximum number of registers per read operation.</summary>
        public ushort? MaxRegistersPerRead { get; init; }
        /// <summary>Gets or sets the maximum number of registers per write operation.</summary>
@@ -182,6 +201,8 @@ public static class ModbusDriverFactoryExtensions
        public string? Transport { get; init; }
        /// <summary>Gets or sets the automatic prohibition reprobei interval in milliseconds.</summary>
        public int? AutoProhibitReprobeMs { get; init; }
+        /// <summary>Gets or sets the re-probe interval as a TimeSpan string (the AdminUI form's shape).</summary>
+        public JsonElement? AutoProhibitReprobeInterval { get; init; }
        /// <summary>Gets or sets a value indicating whether automatic reconnection is enabled.</summary>
        public bool? AutoReconnect { get; init; }
        /// <summary>Gets or sets the authored raw tags (RawPath + TagConfig blob + WriteIdempotent) the
@@ -194,6 +215,8 @@ public static class ModbusDriverFactoryExtensions
        public ModbusKeepAliveDto? KeepAlive { get; init; }
        /// <summary>Gets or sets the idle disconnect timeout in milliseconds.</summary>
        public int? IdleDisconnectMs { get; init; }
+        /// <summary>Gets or sets the idle disconnect timeout as a TimeSpan string (the AdminUI form's shape).</summary>
+        public JsonElement? IdleDisconnectTimeout { get; init; }
        /// <summary>Gets or sets the reconnect configuration.</summary>
        public ModbusReconnectDto? Reconnect { get; init; }
    }
@@ -206,6 +229,10 @@ public static class ModbusDriverFactoryExtensions
        public int? TimeMs { get; init; }
        /// <summary>Gets or sets the keep-alive interval in milliseconds.</summary>
        public int? IntervalMs { get; init; }
+        /// <summary>Gets or sets the keep-alive time as a TimeSpan string (the AdminUI form's shape).</summary>
+        public JsonElement? Time { get; init; }
+        /// <summary>Gets or sets the keep-alive interval as a TimeSpan string (the AdminUI form's shape).</summary>
+        public JsonElement? Interval { get; init; }
        /// <summary>Gets or sets the retry count.</summary>
        public int? RetryCount { get; init; }
    }
@@ -216,6 +243,10 @@ public static class ModbusDriverFactoryExtensions
        public int? InitialDelayMs { get; init; }
        /// <summary>Gets or sets the maximum reconnect delay in milliseconds.</summary>
        public int? MaxDelayMs { get; init; }
+        /// <summary>Gets or sets the initial reconnect delay as a TimeSpan string (the AdminUI form's shape).</summary>
+        public JsonElement? InitialDelay { get; init; }
+        /// <summary>Gets or sets the maximum reconnect delay as a TimeSpan string (the AdminUI form's shape).</summary>
+        public JsonElement? MaxDelay { get; init; }
        /// <summary>Gets or sets the backoff multiplier for exponential reconnect delays.</summary>
        public double? BackoffMultiplier { get; init; }
    }
@@ -228,6 +259,10 @@ public static class ModbusDriverFactoryExtensions
        public int? IntervalMs { get; init; }
        /// <summary>Gets or sets the probe timeout in milliseconds.</summary>
        public int? TimeoutMs { get; init; }
+        /// <summary>Gets or sets the probe interval as a TimeSpan string (the AdminUI form's shape).</summary>
+        public JsonElement? Interval { get; init; }
+        /// <summary>Gets or sets the probe timeout as a TimeSpan string (the AdminUI form's shape).</summary>
+        public JsonElement? Timeout { get; init; }
        /// <summary>Gets or sets the probe register address.</summary>
        public ushort? ProbeAddress { get; init; }
    }
@@ -93,9 +93,10 @@ public static class ModbusRtuFraming
            // Exception frame: excCode(1) + CRC(2).
            trailing = 1 + 2;
        }
-        else if (fc is 0x01 or 0x02 or 0x03 or 0x04)
+        else if (fc is 0x01 or 0x02 or 0x03 or 0x04 or 0x17)
        {
-            // Read response: byteCount(1) then byteCount data bytes + CRC(2).
+            // Read response (and FC23 Read/Write Multiple, whose reply is a read): byteCount(1) then
+            // byteCount data bytes + CRC(2).
            var bc = new byte[1];
            await ReadExactlyAsync(stream, bc, ct).ConfigureAwait(false);
            var byteCount = bc[0];
@@ -105,9 +106,9 @@ public static class ModbusRtuFraming
        }
        else
        {
-            // Fixed 4-byte echo: correct for the write FCs this driver emits (05/06/0F/10). A future
-            // variable-length response FC outside 01-04 (e.g. FC23) would be mis-sized here and
-            // surface as a desync — revisit the FC-shape table if the driver starts emitting one.
+            // Fixed 4-byte echo: correct for the write FCs this driver emits (05/06/0F/10). FC23 is
+            // variable-length and handled above; any NEW variable-length response FC must be added there
+            // or it is mis-sized here and surfaces as a desync.
            trailing = 4 + 2;
        }