From 4a3d7a9361f440b51937ecbf7cd18e7667e8b050 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 15:35:58 -0400 Subject: [PATCH] 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 --- .../ModbusDriverOptions.cs | 20 +- .../ModbusTagDefinitionFactory.cs | 148 ++++++++++++--- .../ModbusDriver.cs | 77 +++++++- .../ModbusDriverFactoryExtensions.cs | 53 +++++- .../ModbusRtuFraming.cs | 11 +- .../Drivers/Forms/ModbusDriverForm.razor | 2 +- .../TagEditors/ModbusTagConfigEditor.razor | 49 ++++- .../Uns/TagEditors/ModbusTagConfigModel.cs | 73 ++++++- .../CommsRecoveryTests.cs | 31 +++ .../ModbusDriverTests.cs | 21 ++ .../ModbusFamilyWiringTests.cs | 179 ++++++++++++++++++ .../ModbusRtuFramingTests.cs | 11 ++ .../Uns/ModbusTagConfigModelTests.cs | 39 ++++ 13 files changed, 656 insertions(+), 58 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusFamilyWiringTests.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusDriverOptions.cs index 9d6b5479..4f456585 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusDriverOptions.cs @@ -89,10 +89,13 @@ public sealed class ModbusDriverOptions public bool UseFC16ForSingleRegisterWrites { get; init; } = false; /// - /// Reserved / no-op kill-switch for FC23 (Read/Write Multiple Registers). The - /// driver does not currently emit FC23 — toggling this option has no observable effect - /// today. The slot exists so a future block-read-coalescing enhancement that opts into - /// FC23 can be disabled per-deployment without a code change. Default false. + /// Kill-switch for FC23 (Read/Write Multiple Registers). By default a multi-register holding + /// write (2–121 registers: 32/64-bit values, arrays, strings) is sent as ONE FC23 that writes + /// the block and reads the same block back, so the device confirms what it now holds. A unit + /// that answers FC23 with exception 01 (MELSEC QJ71MT91, S7 Modbus servers) is moved to FC16 + /// automatically, for that write and every later one; one that fails FC23 at the transport is + /// moved too (that one write is reported as a comms failure). Set true to always use + /// FC16. Default false. /// public bool DisableFC23 { get; init; } = false; @@ -128,10 +131,11 @@ public sealed class ModbusDriverOptions public bool DisableReadCoalescing { get; init; } = false; /// - /// PLC family hint that drives the parser's family-native branch. When set to a - /// non-Generic value, address strings using that family's native syntax (DL205 V2000 / - /// MELSEC D100) parse to the right region + offset directly. Defaults to - /// = Modicon-only behaviour preserved by default. + /// PLC family hint that drives the parser's family-native branch. A tag authored with an + /// addressString is resolved with this family at every initialize, so with a non-Generic + /// value that family's native syntax (DL205 V2000 octal / MELSEC D100) parses to the right + /// region + offset. Defaults to (Modicon / mnemonic grammar). + /// Tags authored by region + numeric address are unaffected. /// public ModbusFamily Family { get; init; } = ModbusFamily.Generic; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusTagDefinitionFactory.cs index 86b0a4e9..d34c55ba 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusTagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusTagDefinitionFactory.cs @@ -32,6 +32,24 @@ public static class ModbusTagDefinitionFactory /// The mapped definition when this returns . /// when is a valid Modbus address object. public static bool FromTagConfig(string tagConfig, string rawPath, out ModbusTagDefinition def) + => FromTagConfig(tagConfig, rawPath, ModbusFamily.Generic, MelsecFamily.Q_L_iQR, out def); + + /// + /// Family-aware overload: an addressString key (the Modicon / mnemonic / family-native grammar + /// of — 40001:F:CDAB, DL205 V2000, MELSEC D100) + /// is resolved with the DRIVER's / and is + /// authoritative for region, offset, type, bit, string length, byte order and array count; the + /// editor also persists the resolved region / address keys beside it for display and for + /// readers that predate it. Without an addressString this is exactly the 3-argument overload. + /// + /// The authored equipment-tag TagConfig JSON. + /// The tag's RawPath — becomes the definition's identity (Name). + /// The driver's PLC family (from its DriverConfig). + /// The driver's MELSEC sub-family; consulted only for MELSEC. + /// The mapped definition when this returns . + /// when is a valid Modbus address object. + public static bool FromTagConfig(string tagConfig, string rawPath, ModbusFamily family, MelsecFamily melsecSubFamily, + out ModbusTagDefinition def) { def = null!; if (string.IsNullOrWhiteSpace(tagConfig)) return false; @@ -39,28 +57,60 @@ public static class ModbusTagDefinitionFactory { using var doc = JsonDocument.Parse(tagConfig); var root = doc.RootElement; - if (root.ValueKind != JsonValueKind.Object - || !root.TryGetProperty("region", out _) - || !root.TryGetProperty("address", out var addr)) - return false; - if (addr.ValueKind != JsonValueKind.Number - || !addr.TryGetInt32(out var address) - || address < 0 || address > ushort.MaxValue) - return false; - // Strict enum reads: 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; - // An absent dataType defaults per region: a bit region (Coils / DiscreteInputs) only carries - // Bool, so defaulting it to Int16 would reject every bit tag authored without the key. - if (!TagConfigJson.TryReadEnumStrict(root, "dataType", DefaultDataType(region), out var dataType)) return false; + if (root.ValueKind != JsonValueKind.Object) return false; + + ModbusRegion region; + int address; + ModbusDataType dataType; + ModbusByteOrder byteOrder; + byte bitIndex; + ushort stringLength; + int? arrayCount; + var addressText = ReadAddressString(root, out var addressStringInvalid); + if (addressStringInvalid) return false; + if (addressText is not null) + { + if (!ModbusAddressParser.TryParse(addressText, family, melsecSubFamily, out var parsed, out _) || parsed is null) + return false; + region = parsed.Region; + address = parsed.Offset; + dataType = parsed.DataType; + byteOrder = parsed.ByteOrder; + bitIndex = parsed.Bit ?? 0; + stringLength = parsed.StringLength; + arrayCount = parsed.ArrayCount; + } + else + { + if (!root.TryGetProperty("region", out _) || !root.TryGetProperty("address", out var addr)) + return false; + if (addr.ValueKind != JsonValueKind.Number + || !addr.TryGetInt32(out address) + || address < 0 || address > ushort.MaxValue) + return false; + // Strict enum reads: 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 region)) return false; + // An absent dataType defaults per region: a bit region (Coils / DiscreteInputs) only carries + // Bool, so defaulting it to Int16 would reject every bit tag authored without the key. + if (!TagConfigJson.TryReadEnumStrict(root, "dataType", DefaultDataType(region), out dataType)) return false; + if (!TagConfigJson.TryReadEnumStrict(root, "byteOrder", ModbusByteOrder.BigEndian, out byteOrder)) return false; + bitIndex = (byte)ReadInt(root, "bitIndex"); + stringLength = (ushort)ReadInt(root, "stringLength"); + // isArray / arrayLength — optional keys authored by the typed Modbus tag editor. + // Canonical rule: a tag is an array iff isArray:true AND arrayLength >= 1. + // isArray:false (with any arrayLength) is scalar — the foundation materialises a + // scalar OPC UA node, so returning an array value would cause a shape mismatch. + // A 1-element array (isArray:true, arrayLength:1) IS valid and reads as short[1]. + var isArray = ReadBool(root, "isArray"); + var arrayLength = ReadInt(root, "arrayLength"); + arrayCount = (isArray && arrayLength >= 1) ? arrayLength : null; + } // Region/type compatibility: a bit region carries only Bool, a register region never Bool. // A mismatch used to throw out of the coalescing planner (killing every tag's poll) or // silently return a bool into an Int32 node. if (!IsRegionTypeCompatible(region, dataType)) return false; - if (!TagConfigJson.TryReadEnumStrict(root, "byteOrder", ModbusByteOrder.BigEndian, out var byteOrder)) return false; if (!TagConfigJson.TryReadEnumStrict(root, "stringByteOrder", ModbusStringByteOrder.HighByteFirst, out var stringByteOrder)) return false; - var bitIndex = (byte)ReadInt(root, "bitIndex"); - var stringLength = (ushort)ReadInt(root, "stringLength"); // Guard: String tags require StringLength >= 1. RegisterCount = (StringLength+1)/2, // so StringLength=0 → 0 registers → spec-illegal FC03/FC04 with quantity=0 → PLC // returns exception 03. Reject here so the driver surfaces BadNodeIdUnknown rather @@ -69,14 +119,6 @@ public static class ModbusTagDefinitionFactory // Guard: BitInRegister tags require BitIndex 0–15. Shift by ≥ 16 on an int overflows // a ushort mask silently — reads always return false, writes have no effect. if (dataType == ModbusDataType.BitInRegister && bitIndex > 15) return false; - // isArray / arrayLength — optional keys authored by the typed Modbus tag editor. - // Canonical rule: a tag is an array iff isArray:true AND arrayLength >= 1. - // isArray:false (with any arrayLength) is scalar — the foundation materialises a - // scalar OPC UA node, so returning an array value would cause a shape mismatch. - // A 1-element array (isArray:true, arrayLength:1) IS valid and reads as short[1]. - 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. Node-level authz remains the effective write gate. var writable = TagConfigJson.ReadWritable(root); @@ -100,6 +142,42 @@ public static class ModbusTagDefinitionFactory catch (InvalidOperationException) { return false; } } + /// + /// Family-unaware acceptance, for the gates that do not know the owning driver's family (the deploy + /// gate's , CSV import, the AdminUI parse consult): accepts a blob SOME family + /// resolves, so the gate is never stricter than the driver it guards. Blobs without an + /// addressString behave exactly like . + /// + /// The authored equipment-tag TagConfig JSON. + /// The tag's RawPath. + /// The definition under the first family that resolves it. + /// when any family's driver would serve the tag. + public static bool FromTagConfigAnyFamily(string tagConfig, string rawPath, out ModbusTagDefinition def) + { + foreach (var (family, sub) in AllFamilies) + if (FromTagConfig(tagConfig, rawPath, family, sub, out def)) return true; + def = null!; + return false; + } + + private static readonly (ModbusFamily Family, MelsecFamily Sub)[] AllFamilies = + [ + (ModbusFamily.Generic, MelsecFamily.Q_L_iQR), + (ModbusFamily.DL205, MelsecFamily.Q_L_iQR), + (ModbusFamily.MELSEC, MelsecFamily.Q_L_iQR), + (ModbusFamily.MELSEC, MelsecFamily.F_iQF), + ]; + + /// Reads the optional addressString: null when absent or blank; flags a non-string value. + private static string? ReadAddressString(JsonElement root, out bool invalid) + { + invalid = false; + if (!root.TryGetProperty("addressString", out var e) || e.ValueKind == JsonValueKind.Null) return null; + if (e.ValueKind != JsonValueKind.String) { invalid = true; return null; } + var text = e.GetString(); + return string.IsNullOrWhiteSpace(text) ? null : text.Trim(); + } + /// The data type an authored tag gets when its dataType key is absent. /// The tag's region. /// Bool for Coils / DiscreteInputs, Int16 for the register regions. @@ -194,7 +272,7 @@ public static class ModbusTagDefinitionFactory // The accept gate: whatever the driver reads draws nothing. See the remarks. The RAW reference is // handed to the mapper — the driver reads the stored blob, not a trimmed copy of it. - if (FromTagConfig(reference, InspectionRawPath, out _)) return warnings; + if (FromTagConfigAnyFamily(reference, InspectionRawPath, out _)) return warnings; try { @@ -235,6 +313,24 @@ public static class ModbusTagDefinitionFactory /// The list to append warnings to. private static void DescribeRejection(JsonElement root, List warnings) { + var addressText = ReadAddressString(root, out var addressStringInvalid); + if (addressStringInvalid) + { + warnings.Add("Modbus TagConfig 'addressString' is not a string — the tag will not resolve (BadNodeIdUnknown)."); + return; + } + if (addressText is not null) + { + // The addressString is authoritative; the other address keys are not read at all. + if (!ModbusAddressParser.TryParse(addressText, out _, out var error)) + warnings.Add($"Modbus 'addressString' '{addressText}' does not parse under any PLC family ({error}) — " + + "the tag will not resolve (BadNodeIdUnknown)."); + else + warnings.Add($"Modbus 'addressString' '{addressText}' resolves to an unusable tag (check its type against " + + "the region) — the tag will not resolve (BadNodeIdUnknown)."); + return; + } + if (!root.TryGetProperty("region", out _)) { warnings.Add( diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs index 4aa312d3..2bc8e8ef 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs @@ -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(StringComparer.Ordinal)); + Volatile.Write(ref _fc23Unsupported, new ConcurrentDictionary()); 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 } } + /// FC23's spec ceiling on the write quantity (0x79); larger multi-register writes use FC16. + private const ushort Fc23MaxWriteRegisters = 121; + + /// + /// 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. + /// + private ConcurrentDictionary _fc23Unsupported = new(); + + /// + /// 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 . 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. + /// + /// when FC23 performed the write; to use FC16. + private async Task 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; + } + /// /// 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. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs index 9aa8e0d7..0886124a 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs @@ -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(dto.MelsecSubFamily, "", driverInstanceId, "MelsecSubFamily"), Transport = dto.Transport is null ? ModbusTransportMode.Tcp : ParseEnum(dto.Transport, "", 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; } + /// + /// One timing knob, accepted in BOTH shapes the config can carry: the *Ms integer keys (CLI, + /// seed data, hand-authored JSON) and the TimeSpan-string keys the AdminUI driver form emits — it + /// serialises itself, so it writes "timeout":"00:00:07", not + /// timeoutMs. 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. + /// + 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(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; } /// Gets or sets the timeout in milliseconds. public int? TimeoutMs { get; init; } + /// Gets or sets the timeout as a TimeSpan string (the AdminUI form's shape). + public JsonElement? Timeout { get; init; } /// Gets or sets the maximum number of registers per read operation. public ushort? MaxRegistersPerRead { get; init; } /// Gets or sets the maximum number of registers per write operation. @@ -182,6 +201,8 @@ public static class ModbusDriverFactoryExtensions public string? Transport { get; init; } /// Gets or sets the automatic prohibition reprobei interval in milliseconds. public int? AutoProhibitReprobeMs { get; init; } + /// Gets or sets the re-probe interval as a TimeSpan string (the AdminUI form's shape). + public JsonElement? AutoProhibitReprobeInterval { get; init; } /// Gets or sets a value indicating whether automatic reconnection is enabled. public bool? AutoReconnect { get; init; } /// 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; } /// Gets or sets the idle disconnect timeout in milliseconds. public int? IdleDisconnectMs { get; init; } + /// Gets or sets the idle disconnect timeout as a TimeSpan string (the AdminUI form's shape). + public JsonElement? IdleDisconnectTimeout { get; init; } /// Gets or sets the reconnect configuration. public ModbusReconnectDto? Reconnect { get; init; } } @@ -206,6 +229,10 @@ public static class ModbusDriverFactoryExtensions public int? TimeMs { get; init; } /// Gets or sets the keep-alive interval in milliseconds. public int? IntervalMs { get; init; } + /// Gets or sets the keep-alive time as a TimeSpan string (the AdminUI form's shape). + public JsonElement? Time { get; init; } + /// Gets or sets the keep-alive interval as a TimeSpan string (the AdminUI form's shape). + public JsonElement? Interval { get; init; } /// Gets or sets the retry count. public int? RetryCount { get; init; } } @@ -216,6 +243,10 @@ public static class ModbusDriverFactoryExtensions public int? InitialDelayMs { get; init; } /// Gets or sets the maximum reconnect delay in milliseconds. public int? MaxDelayMs { get; init; } + /// Gets or sets the initial reconnect delay as a TimeSpan string (the AdminUI form's shape). + public JsonElement? InitialDelay { get; init; } + /// Gets or sets the maximum reconnect delay as a TimeSpan string (the AdminUI form's shape). + public JsonElement? MaxDelay { get; init; } /// Gets or sets the backoff multiplier for exponential reconnect delays. public double? BackoffMultiplier { get; init; } } @@ -228,6 +259,10 @@ public static class ModbusDriverFactoryExtensions public int? IntervalMs { get; init; } /// Gets or sets the probe timeout in milliseconds. public int? TimeoutMs { get; init; } + /// Gets or sets the probe interval as a TimeSpan string (the AdminUI form's shape). + public JsonElement? Interval { get; init; } + /// Gets or sets the probe timeout as a TimeSpan string (the AdminUI form's shape). + public JsonElement? Timeout { get; init; } /// Gets or sets the probe register address. public ushort? ProbeAddress { get; init; } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusRtuFraming.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusRtuFraming.cs index b7f20d05..9fd9c682 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusRtuFraming.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusRtuFraming.cs @@ -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; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/ModbusDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/ModbusDriverForm.razor index a46a255e..ce1d77fc 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/ModbusDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/ModbusDriverForm.razor @@ -112,7 +112,7 @@
- +
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/ModbusTagConfigEditor.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/ModbusTagConfigEditor.razor index a3b38dd4..86842227 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/ModbusTagConfigEditor.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/ModbusTagConfigEditor.razor @@ -4,24 +4,38 @@ @using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers.Pickers
+
+
+
+ +
+ @if (_addressError is not null) + { +
@_addressError
+ } + else if (_m.AddressString is not null) + { +
Resolved from @_m.AddressString (family @_family) — the driver re-resolves it with its own family. Editing a field below drops it.
+ }
- @foreach (var v in Enum.GetValues()) { }
-
+
- @foreach (var v in Enum.GetValues()) { }
- @foreach (var v in Enum.GetValues()) { }
-
+
-
+
@@ -66,6 +80,8 @@ if (ConfigJson == _lastConfigJson) { return; } _lastConfigJson = ConfigJson; _m = ModbusTagConfigModel.FromJson(ConfigJson); + _addressText ??= _m.AddressString; + _family = ModbusTagConfigModel.ReadFamily(GetDriverConfigJson()).Family; } private static int ParseInt(object? v, int fallback = 0) => int.TryParse(v?.ToString(), out var i) ? i : fallback; @@ -80,6 +96,25 @@ await ConfigJsonChanged.InvokeAsync(_m.ToJson()); } + private string? _addressText; + private string? _addressError; + private ModbusFamily _family = ModbusFamily.Generic; + + // A hand edit of any resolved field makes those fields authoritative: drop the address string, which the + // driver would otherwise re-resolve over them. + private Task UpdateAddress(Action apply) => Update(() => { _m.ClearAddressString(); _addressError = null; apply(); }); + + // Resolve the typed address with the OWNING driver's family (DL205 octal V-memory, MELSEC D/X/Y) and persist + // both the address string and the resolved region/offset/type fields. + private async Task ResolveAddressAsync() + { + var (family, sub) = ModbusTagConfigModel.ReadFamily(GetDriverConfigJson()); + _family = family; + var error = _m.ResolveAddress(_addressText, family, sub); + _addressError = error; + if (error is null) await ConfigJsonChanged.InvokeAsync(_m.ToJson()); + } + // Parse the builder string (e.g. "4x00001-1") back into Region + Address fields. // Format: {prefix}{offset:00000}-{length} where prefix is 0x/1x/3x/4x. private async Task OnAddressPicked(string address) @@ -94,7 +129,7 @@ var offsetStr = dashIdx >= 0 ? rest[..dashIdx] : rest; if (int.TryParse(offsetStr, out var offset)) { - await Update(() => { _m.Region = region; _m.Address = offset; }); + await UpdateAddress(() => { _m.Region = region; _m.Address = offset; }); } } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/ModbusTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/ModbusTagConfigModel.cs index 29050fc4..3ce00ad8 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/ModbusTagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/ModbusTagConfigModel.cs @@ -30,6 +30,74 @@ public sealed class ModbusTagConfigModel /// key stays absent. public bool? Writable { get; set; } + /// + /// The address in the parser grammar (40001:F:CDAB, DL205 V2000, MELSEC D100) when + /// the tag was authored that way; null for a tag authored by region + numeric address. When + /// present the driver re-resolves it with ITS family at runtime, so it is authoritative — editing any + /// resolved field by hand clears it (). + /// + public string? AddressString { get; set; } + + /// + /// Resolves an address string through with the owning driver's + /// family and copies the result into the region / offset / type / bit / string-length / byte-order / + /// array fields — the "persist the resolved region/offset" half of the family wiring, so the blob is + /// readable on its own and the editor shows what the driver will poll. + /// + /// The address text the operator typed. + /// The driver's PLC family. + /// The driver's MELSEC sub-family. + /// null on success, else the parser's error. + public string? ResolveAddress(string? text, ModbusFamily family, MelsecFamily melsecSubFamily) + { + if (!ModbusAddressParser.TryParse(text, family, melsecSubFamily, out var parsed, out var error) || parsed is null) + return error ?? "Address did not parse."; + AddressString = text!.Trim(); + Region = parsed.Region; + Address = parsed.Offset; + DataType = parsed.DataType; + ByteOrder = parsed.ByteOrder; + BitIndex = parsed.Bit ?? 0; + StringLength = parsed.StringLength; + if (parsed.ArrayCount is { } count) + { + TagConfigJson.Set(_bag, "isArray", true); + TagConfigJson.Set(_bag, "arrayLength", count); + } + else + { + _bag.Remove("isArray"); + _bag.Remove("arrayLength"); + } + return null; + } + + /// Drops the address string so the hand-edited region / address fields are what the driver reads. + public void ClearAddressString() => AddressString = null; + + /// + /// Reads the owning driver's family / melsecSubFamily from its DriverConfig JSON (keys + /// matched case-insensitively, like the driver's DTO binding); Generic / Q_L_iQR when absent or unknown. + /// + /// The driver's DriverConfig JSON. + /// The family pair the driver will resolve address strings with. + public static (ModbusFamily Family, MelsecFamily MelsecSubFamily) ReadFamily(string? driverConfigJson) + { + var family = ModbusFamily.Generic; + var sub = MelsecFamily.Q_L_iQR; + var o = TagConfigJson.ParseOrNew(driverConfigJson); + foreach (var (key, node) in o) + { + var text = node is JsonValue v && v.TryGetValue(out var str) ? str : null; + if (text is null) continue; + if (string.Equals(key, "family", StringComparison.OrdinalIgnoreCase) + && Enum.TryParse(text, ignoreCase: true, out var f)) family = f; + else if (string.Equals(key, "melsecSubFamily", StringComparison.OrdinalIgnoreCase) + && Enum.TryParse(text, ignoreCase: true, out var m)) sub = m; + } + return (family, sub); + } + /// /// Changes the region and keeps the data type compatible with it (the driver refuses a bit region /// with a non-Bool type, and a register region with Bool): switching to Coils / DiscreteInputs selects @@ -69,6 +137,8 @@ public sealed class ModbusTagConfigModel BitIndex = TagConfigJson.GetInt(o, "bitIndex"), StringLength = TagConfigJson.GetInt(o, "stringLength"), Writable = TagConfigJson.GetBoolNullable(o, "writable"), + AddressString = string.IsNullOrWhiteSpace(TagConfigJson.GetString(o, "addressString")) + ? null : TagConfigJson.GetString(o, "addressString"), _bag = o, }; } @@ -85,6 +155,7 @@ public sealed class ModbusTagConfigModel TagConfigJson.Set(_bag, "bitIndex", BitIndex); TagConfigJson.Set(_bag, "stringLength", StringLength); TagConfigJson.Set(_bag, "writable", Writable); + TagConfigJson.Set(_bag, "addressString", AddressString); return TagConfigJson.Serialize(_bag); } @@ -124,7 +195,7 @@ public sealed class ModbusTagConfigModel ?? TagConfigJson.DescribeInvalidEnum(_bag, "stringByteOrder") ?? TagConfigParseGate.Describe( _source, - j => ModbusTagDefinitionFactory.FromTagConfig(j, TagConfigParseGate.AuthoringRawPath, out _), + j => ModbusTagDefinitionFactory.FromTagConfigAnyFamily(j, TagConfigParseGate.AuthoringRawPath, out _), ModbusTagDefinitionFactory.Inspect, "Modbus"); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/CommsRecoveryTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/CommsRecoveryTests.cs index 857ce3b7..7aeefb70 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/CommsRecoveryTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/CommsRecoveryTests.cs @@ -86,6 +86,37 @@ public sealed class CommsRecoveryTests(ModbusSimulatorFixture sim) driver.GetHealth().State.ShouldBe(DriverState.Healthy); } + /// + /// U1 (DisableFC23 wired): a multi-register write on the default (FC23 write + read-back) path + /// round-trips against the real server — whether it implements FC23 or answers Illegal Function and + /// is moved to FC16 — and so does the forced-FC16 path. Uses the standard profile's scratch HR[200..209]. + /// + [Theory] + [InlineData(false, 200)] + [InlineData(true, 202)] + public async Task Multi_register_write_round_trips_with_and_without_FC23(bool disableFc23, int address) + { + SkipUnlessStandardLive(); + var tag = new ModbusTagDefinition("F", ModbusRegion.HoldingRegisters, (ushort)address, ModbusDataType.Float32); + var opts = new ModbusDriverOptions + { + Host = sim.Host, Port = sim.Port, UnitId = 1, Timeout = TimeSpan.FromSeconds(2), DisableFC23 = disableFc23, + RawTags = ModbusRawTags.Entries([tag]), Probe = new ModbusProbeOptions { Enabled = false }, + }; + await using var driver = new ModbusDriver(opts, "modbus-u1"); + await driver.InitializeAsync("{}", TestContext.Current.CancellationToken); + + var value = disableFc23 ? 12.25f : 6.5f; + for (var i = 0; i < 2; i++) // second write exercises the latched path when the server lacks FC23 + { + var w = await driver.WriteAsync([new WriteRequest("F", value + i)], TestContext.Current.CancellationToken); + w[0].StatusCode.ShouldBe(StatusGood); + var r = await driver.ReadAsync(["F"], TestContext.Current.CancellationToken); + r[0].Value.ShouldBe(value + i); + } + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + } + /// /// H2: a deadbanded tag whose device returns holding the SAME value must publish the Good recovery. /// HR[5] is a constant 5 in the standard profile, so the recovery value is identical by construction. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDriverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDriverTests.cs index 7a686ace..457d0dfb 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDriverTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDriverTests.cs @@ -46,6 +46,7 @@ public sealed class ModbusDriverTests 0x06 => Task.FromResult(WriteSingleReg(pdu)), 0x0F => Task.FromResult(WriteMultipleCoils(pdu)), 0x10 => Task.FromResult(WriteMultipleRegs(pdu)), + 0x17 => Task.FromResult(ReadWriteMultipleRegs(pdu)), _ => Task.FromException(new ModbusException(fc, 0x01, $"fc={fc} not supported by fake")), }; } @@ -102,6 +103,26 @@ public sealed class ModbusDriverTests return new byte[] { 0x10, pdu[1], pdu[2], pdu[3], pdu[4] }; } + // FC23 Read/Write Multiple Registers: write first, then read back the read range. + private byte[] ReadWriteMultipleRegs(byte[] pdu) + { + var readAddr = (ushort)((pdu[1] << 8) | pdu[2]); + var readQty = (ushort)((pdu[3] << 8) | pdu[4]); + var writeAddr = (ushort)((pdu[5] << 8) | pdu[6]); + var writeQty = (ushort)((pdu[7] << 8) | pdu[8]); + for (var i = 0; i < writeQty; i++) + HoldingRegisters[writeAddr + i] = (ushort)((pdu[10 + i * 2] << 8) | pdu[11 + i * 2]); + var resp = new byte[2 + readQty * 2]; + resp[0] = 0x17; + resp[1] = (byte)(readQty * 2); + for (var i = 0; i < readQty; i++) + { + resp[2 + i * 2] = (byte)(HoldingRegisters[readAddr + i] >> 8); + resp[3 + i * 2] = (byte)(HoldingRegisters[readAddr + i] & 0xFF); + } + return resp; + } + private byte[] WriteMultipleCoils(byte[] pdu) { var addr = (ushort)((pdu[1] << 8) | pdu[2]); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusFamilyWiringTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusFamilyWiringTests.cs new file mode 100644 index 00000000..6ed8e509 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusFamilyWiringTests.cs @@ -0,0 +1,179 @@ +using System.Text.Json; +using System.Text.Json.Serialization; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests; + +/// +/// Review 07 U1: Family, MelsecSubFamily and DisableFC23 were authored in the +/// AdminUI and consumed by nothing. A tag's addressString now resolves through +/// with the driver's family; DisableFC23 gates the FC23 +/// write-and-read-back path for multi-register writes. Also pins that the AdminUI form's own +/// (TimeSpan-shaped) config keys reach the driver. +/// +[Trait("Category", "Unit")] +public sealed class ModbusFamilyWiringTests +{ + // ---------------- addressString through the family-aware parser ---------------- + + [Fact] + public void A_MELSEC_D_register_resolves_only_under_the_MELSEC_family() + { + const string json = """{"addressString":"D100"}"""; + ModbusTagDefinitionFactory.FromTagConfig(json, "T", ModbusFamily.MELSEC, MelsecFamily.Q_L_iQR, out var def).ShouldBeTrue(); + def.Region.ShouldBe(ModbusRegion.HoldingRegisters); + def.Address.ShouldBe((ushort)100); + // Positive control: the same text means nothing to a Generic (Modicon) driver. + ModbusTagDefinitionFactory.FromTagConfig(json, "T", ModbusFamily.Generic, MelsecFamily.Q_L_iQR, out _).ShouldBeFalse(); + } + + [Fact] + public void A_DL205_V_memory_address_resolves_octal_under_DL205() + { + ModbusTagDefinitionFactory.FromTagConfig("""{"addressString":"V2000"}""", "T", ModbusFamily.DL205, MelsecFamily.Q_L_iQR, out var def) + .ShouldBeTrue(); + def.Region.ShouldBe(ModbusRegion.HoldingRegisters); + def.Address.ShouldBe((ushort)0x400); // V2000 octal = 1024 + } + + [Fact] + public void The_address_string_carries_type_order_and_count_and_keeps_the_other_keys() + { + ModbusTagDefinitionFactory.FromTagConfig( + """{"addressString":"40011:F:CDAB:4","writable":false,"unitId":3,"region":"Coils","address":9}""", + "T", ModbusFamily.Generic, MelsecFamily.Q_L_iQR, out var def).ShouldBeTrue(); + def.Region.ShouldBe(ModbusRegion.HoldingRegisters, "addressString is authoritative over stale resolved keys"); + def.Address.ShouldBe((ushort)10); + def.DataType.ShouldBe(ModbusDataType.Float32); + def.ByteOrder.ShouldBe(ModbusByteOrder.WordSwap); + def.ArrayCount.ShouldBe(4); + def.Writable.ShouldBeFalse(); + def.UnitId.ShouldBe((byte)3); + } + + [Fact] + public void An_unparseable_address_string_rejects_the_tag_and_the_deploy_gate_says_why() + { + const string json = """{"addressString":"ZZ9"}"""; + ModbusTagDefinitionFactory.FromTagConfig(json, "T", out _).ShouldBeFalse(); + ModbusTagDefinitionFactory.Inspect(json).ShouldContain(w => w.Contains("addressString")); + // The family-unaware gate accepts what SOME family resolves (it must never be stricter than the driver). + ModbusTagDefinitionFactory.Inspect("""{"addressString":"D100"}""").ShouldBeEmpty(); + } + + [Fact] + public async Task The_driver_resolves_address_strings_with_its_configured_family() + { + var fake = new ScriptedTransport(); + fake.Bank.HoldingRegisters[100] = 1234; + var opts = new ModbusDriverOptions + { + Host = "f", Family = ModbusFamily.MELSEC, Probe = new ModbusProbeOptions { Enabled = false }, + RawTags = [new RawTagEntry("D100", """{"addressString":"D100"}""", false)], + }; + await using var drv = new ModbusDriver(opts, "m1", _ => fake); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + var r = await drv.ReadAsync(["D100"], TestContext.Current.CancellationToken); + r[0].StatusCode.ShouldBe(0u); + r[0].Value.ShouldBe((short)1234); + } + + [Fact] + public void Family_and_MelsecSubFamily_parse_from_the_driver_config() + { + var o = ModbusDriverFactoryExtensions.ParseOptions("m1", """{"host":"h","family":"MELSEC","melsecSubFamily":"F_iQF","disableFC23":true}"""); + o.Family.ShouldBe(ModbusFamily.MELSEC); + o.MelsecSubFamily.ShouldBe(MelsecFamily.F_iQF); + o.DisableFC23.ShouldBeTrue(); + } + + // ---------------- DisableFC23 gates the FC23 multi-register write ---------------- + + private static async Task<(ModbusDriver, ScriptedTransport)> StartWriteAsync(bool disableFc23) + { + var t = new ScriptedTransport(); + var opts = new ModbusDriverOptions + { + Host = "f", DisableFC23 = disableFc23, Probe = new ModbusProbeOptions { Enabled = false }, + RawTags = ModbusRawTags.Entries([new ModbusTagDefinition("F", ModbusRegion.HoldingRegisters, 20, ModbusDataType.Float32)]), + }; + var drv = new ModbusDriver(opts, "m1", _ => t); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + return (drv, t); + } + + [Fact] + public async Task Multi_register_writes_use_FC23_write_and_read_back_by_default() + { + var (drv, t) = await StartWriteAsync(disableFc23: false); + await using var _ = drv; + var w = await drv.WriteAsync([new WriteRequest("F", 1.5f)], TestContext.Current.CancellationToken); + w[0].StatusCode.ShouldBe(0u); + t.Sent.Select(s => s.Pdu[0]).ShouldBe([(byte)0x17]); + (await drv.ReadAsync(["F"], TestContext.Current.CancellationToken))[0].Value.ShouldBe(1.5f); + } + + [Fact] + public async Task DisableFC23_writes_with_FC16() + { + var (drv, t) = await StartWriteAsync(disableFc23: true); + await using var _ = drv; + (await drv.WriteAsync([new WriteRequest("F", 1.5f)], TestContext.Current.CancellationToken))[0].StatusCode.ShouldBe(0u); + t.Sent.Select(s => s.Pdu[0]).ShouldBe([(byte)0x10]); + } + + [Fact] + public async Task A_device_without_FC23_falls_back_to_FC16_once_and_stays_on_it() + { + var (drv, t) = await StartWriteAsync(disableFc23: false); + await using var _ = drv; + t.Handler = (_, pdu, _) => pdu[0] == 0x17 + ? Task.FromException(new ModbusException(0x17, 0x01, "illegal function")) + : null; + + (await drv.WriteAsync([new WriteRequest("F", 1.5f)], TestContext.Current.CancellationToken))[0].StatusCode.ShouldBe(0u); + (await drv.WriteAsync([new WriteRequest("F", 2.5f)], TestContext.Current.CancellationToken))[0].StatusCode.ShouldBe(0u); + + t.Sent.Select(s => s.Pdu[0]).ShouldBe([(byte)0x17, (byte)0x10, (byte)0x10]); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + // ---------------- the AdminUI form's own keys reach the driver ---------------- + + [Fact] + public void The_AdminUI_form_shape_of_the_timing_keys_is_honoured() + { + // The form serialises ModbusDriverOptions itself (camelCase, TimeSpan strings), not the *Ms DTO keys. + var formShape = JsonSerializer.Serialize(new ModbusDriverOptions + { + Host = "h", + Timeout = TimeSpan.FromSeconds(7), + IdleDisconnectTimeout = TimeSpan.FromSeconds(90), + AutoProhibitReprobeInterval = TimeSpan.FromSeconds(45), + Probe = new ModbusProbeOptions { Interval = TimeSpan.FromSeconds(11), Timeout = TimeSpan.FromSeconds(3) }, + KeepAlive = new ModbusKeepAliveOptions { Time = TimeSpan.FromSeconds(60), Interval = TimeSpan.FromSeconds(20) }, + Reconnect = new ModbusReconnectOptions { InitialDelay = TimeSpan.FromSeconds(2), MaxDelay = TimeSpan.FromSeconds(40) }, + }, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase, Converters = { new JsonStringEnumConverter() } }); + + var o = ModbusDriverFactoryExtensions.ParseOptions("m1", formShape); + + o.Timeout.ShouldBe(TimeSpan.FromSeconds(7)); + o.IdleDisconnectTimeout.ShouldBe(TimeSpan.FromSeconds(90)); + o.AutoProhibitReprobeInterval.ShouldBe(TimeSpan.FromSeconds(45)); + o.Probe.Interval.ShouldBe(TimeSpan.FromSeconds(11)); + o.Probe.Timeout.ShouldBe(TimeSpan.FromSeconds(3)); + o.KeepAlive.Time.ShouldBe(TimeSpan.FromSeconds(60)); + o.KeepAlive.Interval.ShouldBe(TimeSpan.FromSeconds(20)); + o.Reconnect.InitialDelay.ShouldBe(TimeSpan.FromSeconds(2)); + o.Reconnect.MaxDelay.ShouldBe(TimeSpan.FromSeconds(40)); + } + + [Fact] + public void The_millisecond_keys_still_win_when_both_are_present() + { + var o = ModbusDriverFactoryExtensions.ParseOptions("m1", """{"host":"h","timeoutMs":1500,"timeout":"00:00:07"}"""); + o.Timeout.ShouldBe(TimeSpan.FromMilliseconds(1500)); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusRtuFramingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusRtuFramingTests.cs index 3a26fa28..6c084904 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusRtuFramingTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusRtuFramingTests.cs @@ -25,6 +25,17 @@ public sealed class ModbusRtuFramingTests pdu.ShouldBe(new byte[] { 0x03, 0x02, 0x00, 0x0A }); // fc + byteCount + data, no addr/CRC } + /// FC23 (Read/Write Multiple) replies like a read: byte count + data. It used to be sized as a + /// fixed 4-byte write echo and desync. + [Fact] + public async Task ReadResponse_FC23_returns_bare_pdu() + { + var body = new byte[] { 0x01, 0x17, 0x04, 0x00, 0x0A, 0x00, 0x0B }; + await using var s = Canned(ModbusCrc.Append(body)); + var pdu = await ModbusRtuFraming.ReadResponsePduAsync(s, 0x01, TestContext.Current.CancellationToken); + pdu.ShouldBe(new byte[] { 0x17, 0x04, 0x00, 0x0A, 0x00, 0x0B }); + } + [Fact] public async Task ReadResponse_exception_frame_throws_ModbusException() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/ModbusTagConfigModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/ModbusTagConfigModelTests.cs index e1afd0c1..49abe9ab 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/ModbusTagConfigModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/ModbusTagConfigModelTests.cs @@ -141,4 +141,43 @@ public sealed class ModbusTagConfigModelTests [Fact] public void Validate_refuses_an_incompatible_region_and_type() => ModbusTagConfigModel.FromJson("""{"region":"Coils","address":1,"dataType":"Int16"}""").Validate().ShouldNotBeNull(); + + /// Review 07 U1: the editor resolves an address string with the DRIVER's family and persists + /// both the string and the resolved fields; the driver re-resolves the string with its family. + [Fact] + public void ResolveAddress_uses_the_driver_family_and_persists_string_and_resolved_fields() + { + var (family, sub) = ModbusTagConfigModel.ReadFamily("""{"Family":"MELSEC","melsecSubFamily":"Q_L_iQR"}"""); + family.ShouldBe(ModbusFamily.MELSEC); + + var m = ModbusTagConfigModel.FromJson("{}"); + m.ResolveAddress("D100", family, sub).ShouldBeNull(); + m.Region.ShouldBe(ModbusRegion.HoldingRegisters); + m.Address.ShouldBe(100); + + var json = m.ToJson(); + json.ShouldContain("\"addressString\":\"D100\""); + json.ShouldContain("\"address\":100"); + ModbusTagDefinitionFactory.FromTagConfig(json, "T", ModbusFamily.MELSEC, MelsecFamily.Q_L_iQR, out var def).ShouldBeTrue(); + def.Address.ShouldBe((ushort)100); + ModbusTagConfigModel.FromJson(json).Validate().ShouldBeNull("the family-unaware authoring gate accepts what the MELSEC driver serves"); + + // Positive control: under Generic the same text is an error, and nothing is persisted. + var g = ModbusTagConfigModel.FromJson("{}"); + g.ResolveAddress("D100", ModbusFamily.Generic, MelsecFamily.Q_L_iQR).ShouldNotBeNull(); + g.AddressString.ShouldBeNull(); + } + + [Fact] + public void A_hand_edit_after_resolving_drops_the_address_string() + { + var m = ModbusTagConfigModel.FromJson("{}"); + m.ResolveAddress("40011:F", ModbusFamily.Generic, MelsecFamily.Q_L_iQR).ShouldBeNull(); + m.ClearAddressString(); + m.Address = 50; + var json = m.ToJson(); + json.ShouldNotContain("addressString"); + ModbusTagDefinitionFactory.FromTagConfig(json, "T", out var def).ShouldBeTrue(); + def.Address.ShouldBe((ushort)50); + } }