From 13bd9820b0ec3a902cd17e98bd3222d825b556e8 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 24 Jul 2026 14:12:12 -0400 Subject: [PATCH] feat(modbus-rtu): route driver + probe transports through ModbusTransportFactory Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW --- .../ModbusDriver.cs | 9 ++--- .../ModbusDriverProbe.cs | 35 ++++++++++++++----- .../ModbusTransportWiringTests.cs | 33 +++++++++++++++++ 3 files changed, 63 insertions(+), 14 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusTransportWiringTests.cs 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 69646220..12854f7c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs @@ -94,7 +94,8 @@ public sealed class ModbusDriver /// Initializes a new Modbus TCP driver with the specified options and transport factory. /// Driver configuration options. /// Unique identifier for this driver instance. - /// Factory to create the Modbus transport; defaults to ModbusTcpTransport. + /// Factory to create the Modbus transport; defaults to + /// , which honours . /// Logger instance; defaults to null logger if not provided. public ModbusDriver(ModbusDriverOptions options, string driverInstanceId, Func? transportFactory = null, @@ -107,11 +108,7 @@ public sealed class ModbusDriver _resolver = new EquipmentTagRefResolver( r => _tagsByRawPath.TryGetValue(r, out var t) ? t : null); _transportFactory = transportFactory - ?? (o => new ModbusTcpTransport( - o.Host, o.Port, o.Timeout, o.AutoReconnect, - keepAlive: o.KeepAlive, - idleDisconnect: o.IdleDisconnectTimeout, - reconnect: o.Reconnect)); + ?? (o => ModbusTransportFactory.Create(o)); _poll = new PollGroupEngine( reader: ReadAsync, onChange: (handle, tagRef, snapshot) => diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverProbe.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverProbe.cs index afd6683c..77739782 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverProbe.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverProbe.cs @@ -29,15 +29,19 @@ public sealed class ModbusDriverProbe : IDriverProbe /// public string DriverType => "Modbus"; - /// The parsed probe target — host/port/unitId + the effective connection timeout, all read - /// from the SAME factory DTO shape (ModbusDriverConfigDto) the driver factory parses, so - /// timeoutMs binds identically to the factory (R2-11, 05/CONV-2; the OpcUaClient parity rule). - internal readonly record struct ProbeTarget(string Host, int Port, byte UnitId, TimeSpan Timeout); + /// The parsed probe target — host/port/unitId + the effective connection timeout + the wire + /// transport mode, all read from the SAME factory DTO shape (ModbusDriverConfigDto) the driver + /// factory parses, so timeoutMs binds identically to the factory (R2-11, 05/CONV-2; the + /// OpcUaClient parity rule) and an RtuOverTcp-authored config probes over RTU framing — + /// matching how the driver will actually run. + internal readonly record struct ProbeTarget(string Host, int Port, byte UnitId, TimeSpan Timeout, ModbusTransportMode Transport); /// Parse the driver config JSON into a using the factory DTO shape. /// Returns a null target + an error message when the JSON is invalid or has no host/port. The effective /// timeout mirrors the factory (TimeSpan.FromMilliseconds(dto.TimeoutMs ?? …)); when the config - /// omits timeoutMs the caller's is used. + /// omits timeoutMs the caller's is used. Transport + /// mirrors the factory's null-defaults-to-Tcp convention (); + /// an unrecognized value is reported as a probe-target error rather than silently falling back. /// The driver config JSON (factory DTO shape). /// The timeout used when the config omits timeoutMs. /// The parsed target, or a null target with an error string. @@ -52,8 +56,15 @@ public sealed class ModbusDriverProbe : IDriverProbe if (string.IsNullOrWhiteSpace(dto.Host) || port <= 0) return (null, "Config has no host/port to probe."); + var transportMode = ModbusTransportMode.Tcp; + if (dto.Transport is not null && !Enum.TryParse(dto.Transport, ignoreCase: true, out transportMode)) + { + return (null, $"Config has unknown Transport '{dto.Transport}'. " + + $"Expected one of {string.Join(", ", Enum.GetNames())}"); + } + var timeout = dto.TimeoutMs is { } ms && ms > 0 ? TimeSpan.FromMilliseconds(ms) : fallbackTimeout; - return (new ProbeTarget(dto.Host!, port, dto.UnitId ?? 1, timeout), null); + return (new ProbeTarget(dto.Host!, port, dto.UnitId ?? 1, timeout, transportMode), null); } /// @@ -72,9 +83,17 @@ public sealed class ModbusDriverProbe : IDriverProbe using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct); cts.CancelAfter(timeout); - // Phase 1 — TCP connect (using ModbusTcpTransport which handles IPv4 preference). + // Phase 1 — TCP connect, over whichever transport the config's Transport mode selects + // (ModbusTransportFactory is the single mapping point — same switch the live driver uses). // autoReconnect=false: this is a one-shot probe, no retry loops. - var transport = new ModbusTcpTransport(host, port, timeout, autoReconnect: false); + var transport = ModbusTransportFactory.Create(new ModbusDriverOptions + { + Host = host, + Port = port, + Timeout = timeout, + Transport = t.Transport, + AutoReconnect = false, + }); await using (transport.ConfigureAwait(false)) { try diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusTransportWiringTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusTransportWiringTests.cs new file mode 100644 index 00000000..edbff6bd --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusTransportWiringTests.cs @@ -0,0 +1,33 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests; + +/// +/// Confirms the driver's default transport-factory closure — and, by extension, the +/// probe's transport construction — routes through +/// instead of hardcoding , so an RtuOverTcp-authored +/// config actually gets RTU framing rather than silently running as TCP/MBAP. +/// +[Trait("Category", "Unit")] +public sealed class ModbusTransportWiringTests +{ + private static IModbusTransport BuildDefaultTransport(ModbusDriverOptions opts) + { + using var driver = new ModbusDriver(opts, "wire-test"); + var factory = (Func)typeof(ModbusDriver) + .GetField("_transportFactory", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)! + .GetValue(driver)!; + return factory(opts); + } + + [Fact] + public void Default_closure_builds_rtu_transport_for_RtuOverTcp() + => BuildDefaultTransport(new ModbusDriverOptions { Transport = ModbusTransportMode.RtuOverTcp }) + .ShouldBeOfType(); + + [Fact] + public void Default_closure_builds_tcp_transport_for_Tcp() + => BuildDefaultTransport(new ModbusDriverOptions()) + .ShouldBeOfType(); +}