diff --git a/src/Daqifi.Core.Tests/Device/Internal/DeviceAdministrationOperationsTests.cs b/src/Daqifi.Core.Tests/Device/Internal/DeviceAdministrationOperationsTests.cs new file mode 100644 index 0000000..8233585 --- /dev/null +++ b/src/Daqifi.Core.Tests/Device/Internal/DeviceAdministrationOperationsTests.cs @@ -0,0 +1,247 @@ +using Daqifi.Core.Channel; +using Daqifi.Core.Communication.Messages; +using Daqifi.Core.Device; +using Daqifi.Core.Device.Internal; +using Daqifi.Core.Device.SdCard; +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Xunit; + +namespace Daqifi.Core.Tests.Device.Internal; + +/// +/// Unit tests for , the reboot / ADC-calibration / +/// voltage-precision / friendly-name block extracted from (#344). +/// +/// +/// +/// What each command puts on the wire, and that each one refuses a disconnected device, is already +/// pinned through the device by DaqifiStreamingDeviceTests, +/// DaqifiStreamingDeviceFriendlyNameTests and DeviceNotConnectedExceptionTests. Those +/// are deliberately untouched — they are the evidence that the extraction changed nothing, so they +/// are not repeated here. +/// +/// +/// These add the part only a direct test can see: the ordering and the total set of calls back +/// into the host. The fake below throws on every member outside this block's remit, so a future +/// change that reaches for the channels lock, stops a stream, or performs device I/O beyond the one +/// command fails loudly rather than passing quietly. +/// +/// +public class DeviceAdministrationOperationsTests +{ + [Fact] + public void Constructor_NullHost_Throws() + { + Assert.Throws(() => new DeviceAdministrationOperations(null!)); + } + + #region Reboot + + [Fact] + public void Reboot_SendsTheRebootCommandBeforeTearingTheConnectionDown() + { + var host = new FakeHost { IsConnected = true }; + + new DeviceAdministrationOperations(host).Reboot(); + + // Order is the whole point: disconnecting first would close the transport the reboot + // command still has to travel over, so the device would never be told to restart. + Assert.Equal(new[] { "send:SYSTem:REboot", "disconnect" }, host.Calls); + } + + [Fact] + public void Reboot_WhenNotConnected_ThrowsAndLeavesTheConnectionAlone() + { + var host = new FakeHost { IsConnected = false }; + + Assert.Throws(() => new DeviceAdministrationOperations(host).Reboot()); + + // The guard runs before anything else, so a refused reboot neither sends nor disconnects. + Assert.Empty(host.Calls); + } + + #endregion + + #region One command, and nothing else + + public static IEnumerable SingleCommandOperations() + { + yield return new object[] { "SaveAdcCalibration", "CONFigure:ADC:SAVEcal" }; + yield return new object[] { "LoadAdcCalibration", "CONFigure:ADC:LOADcal" }; + yield return new object[] { "SaveFactoryAdcCalibration", "CONFigure:ADC:SAVEFcal" }; + yield return new object[] { "LoadFactoryAdcCalibration", "CONFigure:ADC:LOADFcal" }; + yield return new object[] { "SaveVoltagePrecision", "CONFigure:VOLTage:SAVE" }; + yield return new object[] { "LoadVoltagePrecision", "CONFigure:VOLTage:LOAD" }; + yield return new object[] { "UseAdcCalibration(0)", "CONFigure:ADC:USECal 0" }; + yield return new object[] { "UseAdcCalibration(1)", "CONFigure:ADC:USECal 1" }; + } + + /// + /// Each of these is a single fire-and-forget command. The assertion is not only that the right + /// text goes out but that the whole interaction with the device is that one send — no stream + /// stop, no channels lock, no metadata write, no disconnect. Every one of those would throw + /// from , and a second send would fail the equality below. + /// + [Theory] + [MemberData(nameof(SingleCommandOperations))] + public void SingleCommandOperation_SendsExactlyThatCommandAndTouchesNothingElse( + string operation, + string expectedCommand) + { + var host = new FakeHost { IsConnected = true }; + var administration = new DeviceAdministrationOperations(host); + + // Dispatched by name rather than by a delegate parameter: the collaborator is internal, so + // an Action cannot appear on a public test method. + switch (operation) + { + case "SaveAdcCalibration": administration.SaveAdcCalibration(); break; + case "LoadAdcCalibration": administration.LoadAdcCalibration(); break; + case "SaveFactoryAdcCalibration": administration.SaveFactoryAdcCalibration(); break; + case "LoadFactoryAdcCalibration": administration.LoadFactoryAdcCalibration(); break; + case "SaveVoltagePrecision": administration.SaveVoltagePrecision(); break; + case "LoadVoltagePrecision": administration.LoadVoltagePrecision(); break; + case "UseAdcCalibration(0)": administration.UseAdcCalibration(0); break; + case "UseAdcCalibration(1)": administration.UseAdcCalibration(1); break; + default: throw new ArgumentOutOfRangeException(nameof(operation), operation, "Unmapped operation."); + } + + Assert.Equal(new[] { "send:" + expectedCommand }, host.Calls); + } + + [Fact] + public void SetAdcCalibrationSlope_SendsExactlyOneCommand() + { + var host = new FakeHost { IsConnected = true }; + + new DeviceAdministrationOperations(host).SetAdcCalibrationSlope(2, 1.0025); + + Assert.Single(host.Calls); + Assert.StartsWith("send:CONFigure:ADC:chanCALM ", host.Calls[0], StringComparison.Ordinal); + } + + [Fact] + public void SetAdcCalibrationOffset_SendsExactlyOneCommand() + { + var host = new FakeHost { IsConnected = true }; + + new DeviceAdministrationOperations(host).SetAdcCalibrationOffset(3, -0.0031); + + Assert.Single(host.Calls); + Assert.StartsWith("send:CONFigure:ADC:chanCALB ", host.Calls[0], StringComparison.Ordinal); + } + + #endregion + + #region Friendly name + + [Fact] + public async Task SetFriendlyNameAsync_SendsSetThenSaveAndThenWritesMetadata() + { + var host = new FakeHost { IsConnected = true }; + + await new DeviceAdministrationOperations(host).SetFriendlyNameAsync("Bench01"); + + Assert.Equal(2, host.Calls.Count); + Assert.StartsWith("send:SYSTem:DEVice:NAME ", host.Calls[0], StringComparison.Ordinal); + Assert.Equal("send:SYSTem:DEVice:NAME:SAVE", host.Calls[1]); + Assert.Equal("Bench01", host.Metadata.FriendlyName); + } + + /// + /// The metadata write is optimistic because the firmware never echoes the name back — but only + /// once both commands have actually gone out. A send that throws means the device was never + /// told, so the local name must not claim otherwise. + /// + [Fact] + public async Task SetFriendlyNameAsync_WhenTheSaveSendFails_LeavesMetadataUnchanged() + { + var host = new FakeHost { IsConnected = true, FailSendAt = 2 }; + host.Metadata.FriendlyName = "Original"; + + await Assert.ThrowsAsync( + () => new DeviceAdministrationOperations(host).SetFriendlyNameAsync("Bench01")); + + Assert.Equal("Original", host.Metadata.FriendlyName); + } + + [Fact] + public async Task SetFriendlyNameAsync_AlreadyCancelled_SendsNothingAndLeavesMetadataUnchanged() + { + var host = new FakeHost { IsConnected = true }; + host.Metadata.FriendlyName = "Original"; + using var cts = new CancellationTokenSource(); + cts.Cancel(); + + await Assert.ThrowsAnyAsync( + () => new DeviceAdministrationOperations(host).SetFriendlyNameAsync("Bench01", cts.Token)); + + Assert.Empty(host.Calls); + Assert.Equal("Original", host.Metadata.FriendlyName); + } + + #endregion + + /// + /// An that records, in order, the only two things this block + /// is allowed to do to a device: send a command, and (for reboot) disconnect. Everything else + /// throws. + /// + private sealed class FakeHost : IDeviceOperationHost + { + private int _sendCount; + + public List Calls { get; } = new(); + + public bool IsConnected { get; set; } + + public DeviceMetadata Metadata { get; } = new(); + + /// 1-based index of the send that should throw, or 0 for none. + public int FailSendAt { get; set; } + + public void Send(IOutboundMessage message) + { + if (++_sendCount == FailSendAt) + { + throw new InvalidOperationException("transport refused the command"); + } + + Calls.Add("send:" + message.Data); + } + + public void Disconnect() => Calls.Add("disconnect"); + + // Outside this block's remit — reaching for any of these is a regression, not a refinement. + public bool IsUsbConnection => throw new NotSupportedException(); + public bool IsStreaming { get => throw new NotSupportedException(); set => throw new NotSupportedException(); } + public int StreamingFrequency => throw new NotSupportedException(); + public TimeSpan SdCardDownloadTimeout => throw new NotSupportedException(); + public TimeSpan SdCardTransferIdleTimeout => throw new NotSupportedException(); + public void StopStreaming() => throw new NotSupportedException(); + public IReadOnlyList SnapshotChannels() => throw new NotSupportedException(); + public void WithChannelsLock(Action action) => throw new NotSupportedException(); + public Task> ExecuteTextCommandAsync( + Action setupAction, + int responseTimeoutMs = 1000, + int completionTimeoutMs = 250, + CancellationToken cancellationToken = default, + Func? prepareAsync = null, + Func? finalizeAsync = null) => throw new NotSupportedException(); + public Task ExecuteRawCaptureAsync( + Func rawAction, + CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public void EnsureSupported(DeviceFeature feature) => throw new NotSupportedException(); + public FeatureNotSupportedException CreateFeatureNotSupportedException(DeviceFeature feature) + => throw new NotSupportedException(); + public void RaiseLowSdSpaceWarning(LowSdSpaceWarningEventArgs e) => throw new NotSupportedException(); + public void RaiseStreamFrameDiscarded(StreamFrameDiscardedEventArgs e) => throw new NotSupportedException(); + public void RaiseGapDetected(TimestampGapEventArgs e) => throw new NotSupportedException(); + public void RaiseRawStreamFrame(DaqifiOutMessage message) => throw new NotSupportedException(); + public void RaiseStreamDecodeFailure(Exception error) => throw new NotSupportedException(); + } +} diff --git a/src/Daqifi.Core.Tests/Device/Internal/StreamFrameDecoderTests.cs b/src/Daqifi.Core.Tests/Device/Internal/StreamFrameDecoderTests.cs index d643ea7..6e1625a 100644 --- a/src/Daqifi.Core.Tests/Device/Internal/StreamFrameDecoderTests.cs +++ b/src/Daqifi.Core.Tests/Device/Internal/StreamFrameDecoderTests.cs @@ -435,9 +435,11 @@ public void RaiseStreamDecodeFailure(Exception error) public bool IsConnected => throw new NotSupportedException(); public bool IsUsbConnection => throw new NotSupportedException(); public int StreamingFrequency => throw new NotSupportedException(); + public DeviceMetadata Metadata => throw new NotSupportedException(); public TimeSpan SdCardDownloadTimeout => throw new NotSupportedException(); public TimeSpan SdCardTransferIdleTimeout => throw new NotSupportedException(); public void StopStreaming() => throw new NotSupportedException(); + public void Disconnect() => throw new NotSupportedException(); public void Send(IOutboundMessage message) => throw new NotSupportedException(); public void WithChannelsLock(Action action) => throw new NotSupportedException(); public Task> ExecuteTextCommandAsync( diff --git a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs index 5766b3d..4d77a4b 100644 --- a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs @@ -200,6 +200,7 @@ private void InitializeStreamingDevice() // method, so they are always in place before the device is handed to a caller. _frameDecoder = new StreamFrameDecoder(this); _channelControl = new ChannelControlOperations(this); + _administration = new DeviceAdministrationOperations(this); _networkOperations = new NetworkConfigurationOperations(this); _sdCardOperations = new SdCardOperations(this); _lanChipInfoOperations = new LanChipInfoOperations(this); @@ -963,165 +964,43 @@ public void SetPwmDutyCycle(IChannel channel, int dutyCyclePercent) /// Thrown when the device is not connected. /// Thrown when the operation is cancelled. public Task SetFriendlyNameAsync(string name, CancellationToken cancellationToken = default) - { - if (name is null) - { - throw new ArgumentNullException(nameof(name)); - } - - if (!ScpiMessageProducer.IsFriendlyNameValid(name)) - { - throw new ArgumentException( - $"Device name must be 1-{ScpiMessageProducer.MaxFriendlyNameLength} printable ASCII characters and cannot contain '\"' or '\\'.", - nameof(name)); - } - - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - cancellationToken.ThrowIfCancellationRequested(); - - Send(ScpiMessageProducer.SetDeviceName(name)); - Send(ScpiMessageProducer.SaveDeviceName); - Metadata.FriendlyName = name; - - return Task.CompletedTask; - } + => _administration.SetFriendlyNameAsync(name, cancellationToken); /// public void SetAnalogOutput(int channelNumber, double voltage) => _channelControl.SetAnalogOutput(channelNumber, voltage); /// - public void Reboot() - { - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.RebootDevice); - - // The device drops its link while restarting, so tear down the local - // connection rather than leaving a stale one that reports Connected. - Disconnect(); - } + public void Reboot() => _administration.Reboot(); /// - public void SaveAdcCalibration() - { - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.SaveAdcCalibration); - } + public void SaveAdcCalibration() => _administration.SaveAdcCalibration(); /// - public void LoadAdcCalibration() - { - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.LoadAdcCalibration); - } + public void LoadAdcCalibration() => _administration.LoadAdcCalibration(); /// public void SetAdcCalibrationSlope(int channelNumber, double calM) - { - if (channelNumber < 0) - { - throw new ArgumentOutOfRangeException(nameof(channelNumber), channelNumber, "Channel number cannot be negative."); - } - - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.SetAdcCalibrationSlope(channelNumber, calM)); - } + => _administration.SetAdcCalibrationSlope(channelNumber, calM); /// public void SetAdcCalibrationOffset(int channelNumber, double calB) - { - if (channelNumber < 0) - { - throw new ArgumentOutOfRangeException(nameof(channelNumber), channelNumber, "Channel number cannot be negative."); - } - - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.SetAdcCalibrationOffset(channelNumber, calB)); - } + => _administration.SetAdcCalibrationOffset(channelNumber, calB); /// - public void SaveFactoryAdcCalibration() - { - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.SaveFactoryAdcCalibration); - } + public void SaveFactoryAdcCalibration() => _administration.SaveFactoryAdcCalibration(); /// - public void LoadFactoryAdcCalibration() - { - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.LoadFactoryAdcCalibration); - } + public void LoadFactoryAdcCalibration() => _administration.LoadFactoryAdcCalibration(); /// - public void UseAdcCalibration(int bank) - { - if (bank is < 0 or > 1) - { - throw new ArgumentOutOfRangeException(nameof(bank), bank, "Calibration bank must be 0 (factory) or 1 (user)."); - } - - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.UseAdcCalibration(bank)); - } + public void UseAdcCalibration(int bank) => _administration.UseAdcCalibration(bank); /// - public void SaveVoltagePrecision() - { - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.SaveVoltagePrecision); - } + public void SaveVoltagePrecision() => _administration.SaveVoltagePrecision(); /// - public void LoadVoltagePrecision() - { - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.LoadVoltagePrecision); - } + public void LoadVoltagePrecision() => _administration.LoadVoltagePrecision(); // ----------------------------------------------------------------- @@ -1141,6 +1020,9 @@ public void LoadVoltagePrecision() /// Channel enable/disable, DIO, PWM and analog output (). private ChannelControlOperations _channelControl = null!; + /// Reboot, ADC calibration banks, voltage precision and the friendly-name write. + private DeviceAdministrationOperations _administration = null!; + /// WiFi/LAN configuration (). private NetworkConfigurationOperations _networkOperations = null!; @@ -1334,6 +1216,10 @@ bool IDeviceOperationHost.IsStreaming void IDeviceOperationHost.Send(IOutboundMessage message) => Send(message); + DeviceMetadata IDeviceOperationHost.Metadata => Metadata; + + void IDeviceOperationHost.Disconnect() => Disconnect(); + IReadOnlyList IDeviceOperationHost.SnapshotChannels() => SnapshotChannels(); void IDeviceOperationHost.WithChannelsLock(Action action) => WithChannelsLock(action); diff --git a/src/Daqifi.Core/Device/Internal/DeviceAdministrationOperations.cs b/src/Daqifi.Core/Device/Internal/DeviceAdministrationOperations.cs new file mode 100644 index 0000000..bc1eb03 --- /dev/null +++ b/src/Daqifi.Core/Device/Internal/DeviceAdministrationOperations.cs @@ -0,0 +1,196 @@ +using Daqifi.Core.Communication.Producers; +using System; +using System.Threading; +using System.Threading.Tasks; + +#nullable enable + +namespace Daqifi.Core.Device.Internal +{ + /// + /// The device-administration half of — reboot, the ADC + /// calibration banks, voltage-precision persistence, and the friendly-name write — extracted + /// from (#344) so the device delegates rather than hosts it. + /// + /// + /// + /// These are fire-and-forget SCPI commands with no reply to parse: each validates its arguments, + /// checks the connection, and sends. They are grouped here because they share that shape and + /// because none of them touches the channel collection, the streaming session, or any device + /// state — the two exceptions being 's local teardown and + /// 's optimistic metadata write, both of which go back through + /// the host rather than being done here. + /// + /// + /// Everything reaches the device through , so each command + /// still passes through the device's own virtual Send and any subclass override of it. + /// + /// + internal sealed class DeviceAdministrationOperations + { + private readonly IDeviceOperationHost _host; + + internal DeviceAdministrationOperations(IDeviceOperationHost host) + { + _host = host ?? throw new ArgumentNullException(nameof(host)); + } + + /// + internal Task SetFriendlyNameAsync(string name, CancellationToken cancellationToken = default) + { + if (name is null) + { + throw new ArgumentNullException(nameof(name)); + } + + if (!ScpiMessageProducer.IsFriendlyNameValid(name)) + { + throw new ArgumentException( + $"Device name must be 1-{ScpiMessageProducer.MaxFriendlyNameLength} printable ASCII characters and cannot contain '\"' or '\\'.", + nameof(name)); + } + + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + cancellationToken.ThrowIfCancellationRequested(); + + _host.Send(ScpiMessageProducer.SetDeviceName(name)); + _host.Send(ScpiMessageProducer.SaveDeviceName); + _host.Metadata.FriendlyName = name; + + return Task.CompletedTask; + } + + /// + internal void Reboot() + { + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.RebootDevice); + + // The device drops its link while restarting, so tear down the local + // connection rather than leaving a stale one that reports Connected. + _host.Disconnect(); + } + + /// + internal void SaveAdcCalibration() + { + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.SaveAdcCalibration); + } + + /// + internal void LoadAdcCalibration() + { + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.LoadAdcCalibration); + } + + /// + internal void SetAdcCalibrationSlope(int channelNumber, double calM) + { + if (channelNumber < 0) + { + throw new ArgumentOutOfRangeException(nameof(channelNumber), channelNumber, "Channel number cannot be negative."); + } + + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.SetAdcCalibrationSlope(channelNumber, calM)); + } + + /// + internal void SetAdcCalibrationOffset(int channelNumber, double calB) + { + if (channelNumber < 0) + { + throw new ArgumentOutOfRangeException(nameof(channelNumber), channelNumber, "Channel number cannot be negative."); + } + + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.SetAdcCalibrationOffset(channelNumber, calB)); + } + + /// + internal void SaveFactoryAdcCalibration() + { + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.SaveFactoryAdcCalibration); + } + + /// + internal void LoadFactoryAdcCalibration() + { + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.LoadFactoryAdcCalibration); + } + + /// + internal void UseAdcCalibration(int bank) + { + if (bank is < 0 or > 1) + { + throw new ArgumentOutOfRangeException(nameof(bank), bank, "Calibration bank must be 0 (factory) or 1 (user)."); + } + + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.UseAdcCalibration(bank)); + } + + /// + internal void SaveVoltagePrecision() + { + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.SaveVoltagePrecision); + } + + /// + internal void LoadVoltagePrecision() + { + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.LoadVoltagePrecision); + } + } +} diff --git a/src/Daqifi.Core/Device/Internal/IDeviceOperationHost.cs b/src/Daqifi.Core/Device/Internal/IDeviceOperationHost.cs index 114891f..01ce732 100644 --- a/src/Daqifi.Core/Device/Internal/IDeviceOperationHost.cs +++ b/src/Daqifi.Core/Device/Internal/IDeviceOperationHost.cs @@ -56,6 +56,23 @@ internal interface IDeviceOperationHost /// void Send(IOutboundMessage message); + /// + /// + /// The device's own metadata object, not a copy: the friendly-name write updates + /// optimistically on it, because the firmware does + /// not echo the new name back and may not stream another status frame for a while. + /// + DeviceMetadata Metadata { get; } + + /// + /// + /// Needed by the reboot command, which has to tear the local connection down after the + /// device drops its link. Routed through the device so the whole disconnect path — lifecycle + /// lock, message pumps, status event — runs exactly as it does for a caller-issued + /// . + /// + void Disconnect(); + /// IReadOnlyList SnapshotChannels();