dohertj2/lmxopcua
1
0
Fork 0
Code Issues 43 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(cli): AbCip subscribe serialises and guards console writes; Ctrl-C exits cleanly; arrays render

Browse Source 
R07 U7: Driver.Cli.Common gains RunCancellableAsync (moved up from AbLegacy) and
CreateChangeLineWriter (one lock-serialised line per change; a failing console
write is logged and swallowed so it cannot fault the poll loop). AbCip
probe/read/write print Cancelled. on Ctrl-C instead of a stack trace, and AbCip
subscribe uses the shared writer. SnapshotFormatter renders arrays/lists as
[a, b, c] instead of System.Int32[].

Refs #747
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
2026-10-22 16:34:48 -04:00
parent 2d8bb037f5
commit f9fbd4ee0d
9 changed files with 277 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/ProbeCommand.cs
+14 -10
View File
@@ -44,17 +44,21 @@ public sealed class ProbeCommand : AbCipCommandBase
await using var driver = new AbCipDriver(options, DriverInstanceId);
try
{
await driver.InitializeAsync("{}", ct);
var snapshot = await driver.ReadAsync(["__probe"], ct);
var health = driver.GetHealth();
// Ctrl-C prints "Cancelled." instead of an OperationCanceledException stack trace (review 07 U7).
await RunCancellableAsync(console, ct, async () =>
{
await driver.InitializeAsync("{}", ct);
var snapshot = await driver.ReadAsync(["__probe"], ct);
var health = driver.GetHealth();
await console.Output.WriteLineAsync($"Gateway: {Gateway}");
await console.Output.WriteLineAsync($"Family: {Family}");
await console.Output.WriteLineAsync($"Health: {health.State}");
if (health.LastError is { } err)
await console.Output.WriteLineAsync($"Last error: {err}");
await console.Output.WriteLineAsync();
await console.Output.WriteLineAsync(SnapshotFormatter.Format(TagPath, snapshot[0]));
await console.Output.WriteLineAsync($"Gateway: {Gateway}");
await console.Output.WriteLineAsync($"Family: {Family}");
await console.Output.WriteLineAsync($"Health: {health.State}");
if (health.LastError is { } err)
await console.Output.WriteLineAsync($"Last error: {err}");
await console.Output.WriteLineAsync();
await console.Output.WriteLineAsync(SnapshotFormatter.Format(TagPath, snapshot[0]));
});
}
finally
{
src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/ReadCommand.cs
+7 -3
View File
@@ -45,9 +45,13 @@ public sealed class ReadCommand : AbCipCommandBase
await using var driver = new AbCipDriver(options, DriverInstanceId);
try
{
await driver.InitializeAsync("{}", ct);
var snapshot = await driver.ReadAsync([tagName], ct);
await console.Output.WriteLineAsync(SnapshotFormatter.Format(TagPath, snapshot[0]));
// Ctrl-C prints "Cancelled." instead of an OperationCanceledException stack trace (review 07 U7).
await RunCancellableAsync(console, ct, async () =>
{
await driver.InitializeAsync("{}", ct);
var snapshot = await driver.ReadAsync([tagName], ct);
await console.Output.WriteLineAsync(SnapshotFormatter.Format(TagPath, snapshot[0]));
});
}
finally
{
src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/SubscribeCommand.cs
+13 -10
View File
@@ -50,23 +50,22 @@ public sealed class SubscribeCommand : AbCipCommandBase
await using var driver = new AbCipDriver(options, DriverInstanceId);
ISubscriptionHandle? handle = null;
// The poll engine raises OnDataChange on a background thread: serialise its lines against each other and
// against this thread's own, and never let a failed console write fault the poll loop (review 07 U7).
var writeLock = new object();
try
{
await driver.InitializeAsync("{}", ct);
await console.Output.WriteLineAsync(
$"Subscribed to {TagPath} @ {IntervalMs}ms. Ctrl+C to stop.");
driver.OnDataChange += (_, e) =>
{
var line = $"[{DateTime.UtcNow:HH:mm:ss.fff}] " +
$"{e.FullReference} = {SnapshotFormatter.FormatValue(e.Snapshot.Value)} " +
$"({SnapshotFormatter.FormatStatus(e.Snapshot.StatusCode)})";
console.Output.WriteLine(line);
};
driver.OnDataChange += CreateChangeLineWriter(console.Output, writeLock);
handle = await driver.SubscribeAsync([tagName], TimeSpan.FromMilliseconds(IntervalMs), ct);
lock (writeLock)
{
console.Output.WriteLine($"Subscribed to {TagPath} @ {IntervalMs}ms. Ctrl+C to stop.");
}
try
{
await Task.Delay(System.Threading.Timeout.InfiniteTimeSpan, ct);
@@ -76,6 +75,10 @@ public sealed class SubscribeCommand : AbCipCommandBase
// Expected on Ctrl+C.
}
}
catch (OperationCanceledException) when (ct.IsCancellationRequested)
{
// Ctrl+C while still connecting/subscribing: a clean exit, not a stack trace (review 07 U7).
}
finally
{
if (handle is not null)
src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/WriteCommand.cs
+7 -3
View File
@@ -57,9 +57,13 @@ public sealed class WriteCommand : AbCipCommandBase
await using var driver = new AbCipDriver(options, DriverInstanceId);
try
{
await driver.InitializeAsync("{}", ct);
var results = await driver.WriteAsync([new WriteRequest(tagName, parsed)], ct);
await console.Output.WriteLineAsync(SnapshotFormatter.FormatWrite(TagPath, results[0]));
// Ctrl-C prints "Cancelled." instead of an OperationCanceledException stack trace (review 07 U7).
await RunCancellableAsync(console, ct, async () =>
{
await driver.InitializeAsync("{}", ct);
var results = await driver.WriteAsync([new WriteRequest(tagName, parsed)], ct);
await console.Output.WriteLineAsync(SnapshotFormatter.FormatWrite(TagPath, results[0]));
});
}
finally
{
src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/AbLegacyCommandBase.cs
-21
View File
@@ -32,27 +32,6 @@ public abstract class AbLegacyCommandBase : DriverCommandBase
"LogixPccc unverified).")]
public AbLegacyCliTransport Transport { get; init; } = AbLegacyCliTransport.Default;
/// <summary>
/// Runs a one-shot command's driver work so a Ctrl-C (cancellation of <paramref name="ct"/>) prints
/// <c>Cancelled.</c> instead of an unhandled <see cref="OperationCanceledException"/> stack trace (review 07 U7).
/// Any other exception — and an OCE the operator did not ask for — still propagates.
/// </summary>
/// <param name="console">The CLI console.</param>
/// <param name="ct">The console's Ctrl-C token.</param>
/// <param name="body">The command body.</param>
/// <returns>A task that completes when the body has finished or was cancelled.</returns>
internal static async ValueTask RunCancellableAsync(CliFx.Infrastructure.IConsole console, CancellationToken ct, Func<Task> body)
{
try
{
await body();
}
catch (OperationCanceledException) when (ct.IsCancellationRequested)
{
await console.Error.WriteLineAsync("Cancelled.");
}
}
/// <summary>Gets the per-operation timeout in milliseconds.</summary>
[CommandOption("timeout-ms", Description = "Per-operation timeout in ms (default 5000).")]
public int TimeoutMs { get; init; } = 5000;
src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/DriverCommandBase.cs
+56
View File
@@ -79,5 +79,61 @@ public abstract class DriverCommandBase : ICommand
/// </summary>
protected static void FlushLogging() => Log.CloseAndFlush();
/// <summary>
/// Runs a one-shot command's driver work so a Ctrl-C (cancellation of <paramref name="ct"/>) prints
/// <c>Cancelled.</c> instead of an unhandled <see cref="OperationCanceledException"/> stack trace (review 07 U7).
/// Any other exception — and an OCE the operator did not ask for — still propagates.
/// </summary>
/// <param name="console">The CLI console.</param>
/// <param name="ct">The console's Ctrl-C token.</param>
/// <param name="body">The command body.</param>
/// <returns>A task that completes when the body has finished or was cancelled.</returns>
public static async ValueTask RunCancellableAsync(IConsole console, CancellationToken ct, Func<Task> body)
{
try
{
await body();
}
catch (OperationCanceledException) when (ct.IsCancellationRequested)
{
await console.Error.WriteLineAsync("Cancelled.");
}
}
/// <summary>
/// Builds the <c>OnDataChange</c> handler a <c>subscribe</c> command attaches: one
/// <c>[HH:mm:ss.fff] ref = value (status)</c> line per change. Writes are serialised on
/// <paramref name="writeLock"/> (the poll engine raises from a background thread, concurrently with the
/// command thread's own lines), and a failing console write (closed pipe, redirected stream error) is logged
/// and swallowed so it cannot fault the driver's poll loop (review 07 U7).
/// </summary>
/// <param name="output">The console output writer.</param>
/// <param name="writeLock">The lock every console write of the command takes.</param>
/// <returns>The event handler.</returns>
public static EventHandler<ZB.MOM.WW.OtOpcUa.Core.Abstractions.DataChangeEventArgs> CreateChangeLineWriter(
TextWriter output, object writeLock)
{
ArgumentNullException.ThrowIfNull(output);
ArgumentNullException.ThrowIfNull(writeLock);
return (_, e) =>
{
try
{
var line = $"[{DateTime.UtcNow:HH:mm:ss.fff}] " +
$"{e.FullReference} = {SnapshotFormatter.FormatValue(e.Snapshot.Value)} " +
$"({SnapshotFormatter.FormatStatus(e.Snapshot.StatusCode)})";
lock (writeLock)
{
output.WriteLine(line);
}
}
catch (Exception ex)
{
Log.Logger.Warning(ex, "subscribe: console write failed for {Tag}; continuing the poll loop.",
e.FullReference);
}
};
}
private bool _loggingConfigured;
}
src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs
+19 -1
View File
@@ -101,7 +101,11 @@ public static class SnapshotFormatter
return sb.ToString().TrimEnd();
}
/// <summary>Formats a value for console output, handling null, bool, string, and formattable types.</summary>
/// <summary>
/// Formats a value for console output, handling null, bool, string, formattable and array/list types. An
/// array renders its elements (<c>[1, 2, 3]</c>, each formatted as a scalar) rather than its CLR type name
/// (<c>System.Int32[]</c>).
/// </summary>
/// <param name="value">The value to format.</param>
/// <returns>A formatted string representation of the value.</returns>
public static string FormatValue(object? value) => value switch
@@ -110,9 +114,23 @@ public static class SnapshotFormatter
bool b => b ? "true" : "false",
string s => $"\"{s}\"",
IFormattable f => f.ToString(null, CultureInfo.InvariantCulture),
System.Collections.IEnumerable items => FormatSequence(items),
_ => value.ToString() ?? "<null>",
};
private static string FormatSequence(System.Collections.IEnumerable items)
{
var sb = new System.Text.StringBuilder("[");
var first = true;
foreach (var item in items)
{
if (!first) sb.Append(", ");
sb.Append(FormatValue(item));
first = false;
}
return sb.Append(']').ToString();
}
/// <summary>Formats an OPC UA status code as a hexadecimal string with named severity classification.</summary>
/// <param name="statusCode">The OPC UA status code to format.</param>
/// <returns>A formatted status string like "0x00000000 (Good)" or "0x80050000 (BadCommunicationError)".</returns>
tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/CancellationHandlingTests.cs
+50
View File
@@ -0,0 +1,50 @@
using CliFx.Infrastructure;
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Commands;
namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests;
/// <summary>
/// Review 07 U7: the AbCip probe/read/write commands had no <see cref="OperationCanceledException"/> arm, so a
/// Ctrl-C printed a stack trace. A command cancelled by the operator now exits cleanly with <c>Cancelled.</c>.
/// The gateway is TEST-NET (never answers), so nothing is sent to a real device.
/// </summary>
[Trait("Category", "Unit")]
public sealed class CancellationHandlingTests
{
private const string Blackhole = "ab://192.0.2.1/1,0";
private static async Task<string> RunCancelledAsync(AbCipCommandBase command)
{
using var console = new FakeInMemoryConsole();
console.RequestCancellation();
await Should.NotThrowAsync(async () => await command.ExecuteAsync(console));
return console.ReadErrorString();
}
/// <summary>A cancelled read prints Cancelled. and does not throw.</summary>
[Fact]
public async Task A_cancelled_read_prints_Cancelled()
{
(await RunCancelledAsync(new ReadCommand { Gateway = Blackhole, TagPath = "Motor01" }))
.ShouldContain("Cancelled.");
}
/// <summary>A cancelled probe prints Cancelled. and does not throw.</summary>
[Fact]
public async Task A_cancelled_probe_prints_Cancelled()
{
(await RunCancelledAsync(new ProbeCommand { Gateway = Blackhole, TagPath = "Motor01" }))
.ShouldContain("Cancelled.");
}
/// <summary>A cancelled write prints Cancelled. and does not throw.</summary>
[Fact]
public async Task A_cancelled_write_prints_Cancelled()
{
(await RunCancelledAsync(new WriteCommand { Gateway = Blackhole, TagPath = "Motor01", Value = "1" }))
.ShouldContain("Cancelled.");
}
}
tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common.Tests/CliHelpersU7Tests.cs
+111
View File
@@ -0,0 +1,111 @@
using System.Text;
using CliFx.Infrastructure;
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
using ZB.MOM.WW.OtOpcUa.Driver.Cli.Common;
namespace ZB.MOM.WW.OtOpcUa.Driver.Cli.Common.Tests;
/// <summary>
/// Review 07 U7, shared half: arrays render their elements (not <c>System.Int32[]</c>), Ctrl-C prints
/// <c>Cancelled.</c>, and the subscribe change-line writer is serialised and never throws into the poll loop.
/// </summary>
[Trait("Category", "Unit")]
public sealed class CliHelpersU7Tests
{
private sealed record Handle(string DiagnosticId) : ISubscriptionHandle;
private static DataChangeEventArgs Change(string reference, object? value) =>
new(new Handle("h"), reference, new DataValueSnapshot(value, 0u, DateTime.UtcNow, DateTime.UtcNow));
/// <summary>An array value renders as its elements, each formatted like a scalar.</summary>
[Fact]
public void FormatValue_renders_array_elements()
{
SnapshotFormatter.FormatValue(new[] { 1, 2, 3 }).ShouldBe("[1, 2, 3]");
SnapshotFormatter.FormatValue(new[] { 1.5f, -2f }).ShouldBe("[1.5, -2]");
SnapshotFormatter.FormatValue(new[] { true, false }).ShouldBe("[true, false]");
SnapshotFormatter.FormatValue(new[] { "a", "b" }).ShouldBe("[\"a\", \"b\"]");
SnapshotFormatter.FormatValue(new List<short> { 7 }).ShouldBe("[7]");
SnapshotFormatter.FormatValue(Array.Empty<int>()).ShouldBe("[]");
}
/// <summary>A string is still a scalar, not a sequence of chars (positive control for the array arm).</summary>
[Fact]
public void FormatValue_keeps_a_string_scalar()
{
SnapshotFormatter.FormatValue("abc").ShouldBe("\"abc\"");
}
/// <summary>Operator Ctrl-C prints Cancelled. instead of a stack trace.</summary>
[Fact]
public async Task RunCancellableAsync_operator_cancellation_prints_Cancelled()
{
using var console = new FakeInMemoryConsole();
using var cts = new CancellationTokenSource();
cts.Cancel();
await Should.NotThrowAsync(async () =>
await DriverCommandBase.RunCancellableAsync(console, cts.Token, () => Task.FromCanceled(cts.Token)));
console.ReadErrorString().ShouldContain("Cancelled.");
}
/// <summary>A cancellation the operator did not request still propagates.</summary>
[Fact]
public async Task RunCancellableAsync_unrequested_cancellation_propagates()
{
using var console = new FakeInMemoryConsole();
await Should.ThrowAsync<OperationCanceledException>(async () =>
await DriverCommandBase.RunCancellableAsync(console, CancellationToken.None,
() => throw new OperationCanceledException()));
}
/// <summary>A change line carries the reference, the formatted value and the status.</summary>
[Fact]
public void ChangeLineWriter_writes_one_formatted_line_per_change()
{
var output = new StringWriter();
var write = DriverCommandBase.CreateChangeLineWriter(output, new object());
write(this, Change("Tag:DInt", new[] { 4, 5 }));
var line = output.ToString().TrimEnd();
line.ShouldContain("Tag:DInt = [4, 5] (0x00000000 (Good))");
}
/// <summary>Concurrent poll-thread changes produce whole lines, never interleaved fragments.</summary>
[Fact]
public async Task ChangeLineWriter_serialises_concurrent_writes()
{
var output = new StringWriter(new StringBuilder());
var write = DriverCommandBase.CreateChangeLineWriter(output, new object());
await Task.WhenAll(Enumerable.Range(0, 8).Select(t => Task.Run(() =>
{
for (var i = 0; i < 200; i++) write(this, Change($"T{t}", i));
})));
var lines = output.ToString().Split(Environment.NewLine, StringSplitOptions.RemoveEmptyEntries);
lines.Length.ShouldBe(1600);
lines.ShouldAllBe(l => l.StartsWith('[') && l.EndsWith("(Good))"));
}
/// <summary>A failing console (closed pipe) is swallowed so it cannot fault the driver's poll loop.</summary>
[Fact]
public void ChangeLineWriter_swallows_a_console_write_failure()
{
var write = DriverCommandBase.CreateChangeLineWriter(new ThrowingWriter(), new object());
Should.NotThrow(() => write(this, Change("T", 1)));
}
private sealed class ThrowingWriter : TextWriter
{
public override Encoding Encoding => Encoding.UTF8;
public override void Write(char value) => throw new IOException("pipe closed");
public override void WriteLine(string? value) => throw new IOException("pipe closed");
}
}