From b2a56ebf3bcd5051e0ab866e791e16e1bf0711c0 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 11:07:44 -0400 Subject: [PATCH] =?UTF-8?q?fix(driver-modbus):=20probe=20parses=20the=20fa?= =?UTF-8?q?ctory=20DTO=20shape=20=E2=80=94=20timeoutMs=20no=20longer=20sil?= =?UTF-8?q?ently=20dropped=20(R2-11,=2005/CONV-2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...tagconfig-consolidation-plan.md.tasks.json | 2 +- .../ModbusDriverProbe.cs | 46 +++++++++++---- .../ModbusDriverProbeParityTests.cs | 56 +++++++++++++++++++ 3 files changed, 91 insertions(+), 13 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDriverProbeParityTests.cs diff --git a/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json b/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json index 9baa9f94..e014d4c4 100644 --- a/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json +++ b/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json @@ -20,7 +20,7 @@ { "id": "T16", "subject": "TwinCAT equipment-tag parser: freeze test, shared readers, honour writable key, Inspect warnings", "status": "completed", "blockedBy": ["T11"] }, { "id": "T17", "subject": "FOCAS equipment-tag parser: force Writable:false (05/UNDER-1 correction), shared readers, Inspect warnings (+ amend Contracts banner)", "status": "completed", "blockedBy": ["T11"] }, { "id": "T18", "subject": "FOCAS capability-matrix pre-flight on the equipment-tag _parseRef resolve path (BadNodeIdUnknown on rejection)", "status": "completed", "blockedBy": ["T17"] }, - { "id": "T19", "subject": "Modbus probe parses the factory DTO shape (timeoutMs round-trip; OpcUaClient parity rule)", "status": "pending", "blockedBy": [] }, + { "id": "T19", "subject": "Modbus probe parses the factory DTO shape (timeoutMs round-trip; OpcUaClient parity rule)", "status": "completed", "blockedBy": [] }, { "id": "T20", "subject": "EquipmentTagConfigInspector DriverType-dispatch map in ControlPlane (+6 Contracts references)", "status": "pending", "blockedBy": ["T12", "T13", "T14", "T15", "T16", "T17"] }, { "id": "T21", "subject": "AdminOperationsActor deploy-gate wiring + Deployment:TagConfigValidationMode (Warn default | Error opt-in); actor-level consuming test", "status": "pending", "blockedBy": ["T20"] }, { "id": "T22", "subject": "AdminUI writable checkbox in the six driver-typed tag editors (models + razor; live /run verify at execution)", "status": "pending", "blockedBy": [] }, 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 38e7c5da..afd6683c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverProbe.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverProbe.cs @@ -29,19 +29,44 @@ 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); + + /// 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. + /// 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. + internal static (ProbeTarget? Target, string? Error) TryParseProbeTarget(string configJson, TimeSpan fallbackTimeout) + { + ModbusDriverFactoryExtensions.ModbusDriverConfigDto? dto; + try { dto = JsonSerializer.Deserialize(configJson, _opts); } + catch (Exception ex) { return (null, $"Config JSON is invalid: {ex.Message}"); } + if (dto is null) return (null, "Config JSON deserialized to null."); + + var port = dto.Port ?? 502; + if (string.IsNullOrWhiteSpace(dto.Host) || port <= 0) + return (null, "Config has no host/port to probe."); + + var timeout = dto.TimeoutMs is { } ms && ms > 0 ? TimeSpan.FromMilliseconds(ms) : fallbackTimeout; + return (new ProbeTarget(dto.Host!, port, dto.UnitId ?? 1, timeout), null); + } + /// public async Task ProbeAsync(string configJson, TimeSpan timeout, CancellationToken ct) { - ModbusDriverOptions? opts; - try { opts = JsonSerializer.Deserialize(configJson, _opts); } - catch (Exception ex) { return new(false, $"Config JSON is invalid: {ex.Message}", null); } - if (opts is null) return new(false, "Config JSON deserialized to null.", null); + var (target, error) = TryParseProbeTarget(configJson, timeout); + if (target is not { } t) return new(false, error!, null); - var (host, port) = ExtractTarget(opts); - if (string.IsNullOrWhiteSpace(host) || port <= 0) - return new(false, "Config has no host/port to probe.", null); - - var unitId = opts.UnitId; + var host = t.Host; + var port = t.Port; + var unitId = t.UnitId; + // Honour the config's timeoutMs (factory parity) — the caller's timeout is the fallback. + timeout = t.Timeout; var sw = Stopwatch.StartNew(); using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct); @@ -93,7 +118,4 @@ public sealed class ModbusDriverProbe : IDriverProbe } } } - - private static (string host, int port) ExtractTarget(ModbusDriverOptions opts) - => (opts.Host, opts.Port); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDriverProbeParityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDriverProbeParityTests.cs new file mode 100644 index 00000000..552795ba --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDriverProbeParityTests.cs @@ -0,0 +1,56 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.Modbus; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests; + +/// +/// R2-11 (05/CONV-2): the Modbus probe parses the SAME factory DTO shape the driver factory parses, so +/// timeoutMs binds identically instead of being silently dropped (it never bound to the runtime +/// ModbusDriverOptions.Timeout TimeSpan). The OpcUaClient probe/factory parse-parity rule. +/// +public sealed class ModbusDriverProbeParityTests +{ + [Fact] + public void Config_timeoutMs_binds_as_the_effective_probe_timeout() + { + var (target, error) = ModbusDriverProbe.TryParseProbeTarget( + "{\"host\":\"10.0.0.5\",\"port\":502,\"unitId\":3,\"timeoutMs\":250}", + fallbackTimeout: TimeSpan.FromSeconds(5)); + + error.ShouldBeNull(); + target.ShouldNotBeNull(); + target!.Value.Host.ShouldBe("10.0.0.5"); + target.Value.Port.ShouldBe(502); + target.Value.UnitId.ShouldBe((byte)3); + target.Value.Timeout.ShouldBe(TimeSpan.FromMilliseconds(250)); + } + + [Fact] + public void Absent_timeoutMs_falls_back_to_the_caller_timeout() + { + var (target, _) = ModbusDriverProbe.TryParseProbeTarget( + "{\"host\":\"10.0.0.5\",\"port\":502}", fallbackTimeout: TimeSpan.FromSeconds(5)); + + target!.Value.Timeout.ShouldBe(TimeSpan.FromSeconds(5)); + target.Value.UnitId.ShouldBe((byte)1); // DTO default + } + + [Fact] + public void Missing_host_is_an_error() + { + var (target, error) = ModbusDriverProbe.TryParseProbeTarget( + "{\"port\":0}", fallbackTimeout: TimeSpan.FromSeconds(5)); + target.ShouldBeNull(); + error.ShouldNotBeNull(); + } + + [Fact] + public void Invalid_json_is_an_error() + { + var (target, error) = ModbusDriverProbe.TryParseProbeTarget( + "not valid json {{", fallbackTimeout: TimeSpan.FromSeconds(5)); + target.ShouldBeNull(); + error.ShouldContain("invalid"); + } +}