diff --git a/src/Daqifi.Core.Tests/Device/DaqifiDeviceLoggerTests.cs b/src/Daqifi.Core.Tests/Device/DaqifiDeviceLoggerTests.cs new file mode 100644 index 00000000..143237d8 --- /dev/null +++ b/src/Daqifi.Core.Tests/Device/DaqifiDeviceLoggerTests.cs @@ -0,0 +1,101 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Daqifi.Core.Communication.Messages; +using Daqifi.Core.Device; +using Microsoft.Extensions.Logging; +using Xunit; + +namespace Daqifi.Core.Tests.Device +{ + public class DaqifiDeviceLoggerTests + { + [Fact] + public void PopulateChannels_NoUsableResolution_LogsWarningThroughInjectedLogger() + { + var logger = new CapturingLogger(); + var device = new DaqifiStreamingDevice("Lab Nq1", ipAddress: null, logger: logger); + device.Connect(); + + device.PopulateChannelsFromStatus(StatusWithResolution(analogCount: 2, resolution: 0)); + + var warning = logger.Entries.SingleOrDefault(e => e.Level == LogLevel.Warning); + Assert.NotEqual(default, warning); + Assert.Contains("no usable ADC resolution", warning.Message); + Assert.Contains("Lab Nq1", warning.Message); // device name rendered from the template + } + + [Fact] + public void PopulateChannels_ValidStatus_LogsNoWarning() + { + var logger = new CapturingLogger(); + var device = new DaqifiStreamingDevice("Lab Nq1", ipAddress: null, logger: logger); + device.Connect(); + + device.PopulateChannelsFromStatus(StatusWithResolution(analogCount: 2, resolution: 65535)); + + Assert.DoesNotContain(logger.Entries, e => e.Level == LogLevel.Warning); + } + + [Fact] + public void PopulateChannels_NoLogger_UsesNullLogger_DoesNotThrow() + { + // No logger supplied — the device must fall back to a no-op logger, not NRE on a warning path. + var device = new DaqifiStreamingDevice("Lab Nq1"); // logger defaults to null -> NullLogger + device.Connect(); + + var ex = Record.Exception(() => device.PopulateChannelsFromStatus(StatusWithResolution(2, resolution: 0))); + Assert.Null(ex); + } + + [Fact] + public void DeviceConnectionOptions_Logger_DefaultsToNull() + { + Assert.Null(new DeviceConnectionOptions().Logger); + } + + [Fact] + public void PopulateChannels_ThrowingLogger_IsSwallowed_DoesNotPropagate() + { + // A misbehaving consumer logger must never take down device operation (SafeLog isolation). + var device = new DaqifiStreamingDevice("Lab Nq1", ipAddress: null, logger: new ThrowingLogger()); + device.Connect(); + + var ex = Record.Exception(() => device.PopulateChannelsFromStatus(StatusWithResolution(2, resolution: 0))); + Assert.Null(ex); + } + + private static DaqifiOutMessage StatusWithResolution(int analogCount, uint resolution) + { + var status = new DaqifiOutMessage + { + AnalogInPortNum = (uint)analogCount, + DigitalPortNum = 0, + AnalogInRes = resolution, + }; + for (var i = 0; i < analogCount; i++) status.AnalogInPortRange.Add(1.0f); + return status; + } + + private sealed class CapturingLogger : ILogger + { + public readonly List<(LogLevel Level, string Message)> Entries = new(); + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) + => Entries.Add((logLevel, formatter(state, exception))); + } + + private sealed class ThrowingLogger : ILogger + { + public IDisposable? BeginScope(TState state) where TState : notnull => null; + public bool IsEnabled(LogLevel logLevel) => true; + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) + => throw new InvalidOperationException("boom"); + } + } +} diff --git a/src/Daqifi.Core/Device/DaqifiDevice.cs b/src/Daqifi.Core/Device/DaqifiDevice.cs index 70ae49a2..413681ce 100644 --- a/src/Daqifi.Core/Device/DaqifiDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiDevice.cs @@ -5,6 +5,8 @@ using Daqifi.Core.Communication.Transport; using Daqifi.Core.Device.Protocol; using Daqifi.Core.Firmware; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using System; using System.Collections.Generic; using System.Diagnostics; @@ -110,6 +112,15 @@ public IReadOnlyList GetChannelsSnapshot() public DeviceState State { get; private set; } = DeviceState.Disconnected; private ConnectionStatus _status; + + /// + /// Sink for this device's diagnostics. Defaults to when the + /// caller opts out — the safety-relevant warnings (bad calibration/resolution → wrong scaled + /// samples) are then simply discarded, as they were effectively invisible via the previous + /// Trace.WriteLine path to consumers on Microsoft.Extensions.Logging. Never null. + /// + private readonly ILogger _logger; + private IMessageProducer? _messageProducer; private IMessageConsumer? _messageConsumer; private readonly IStreamTransport? _transport; @@ -234,11 +245,13 @@ private set /// /// The name of the device. /// The IP address of the device, if known. - public DaqifiDevice(string name, IPAddress? ipAddress = null) + /// Optional logger for device diagnostics; a no-op logger is used when null. + public DaqifiDevice(string name, IPAddress? ipAddress = null, ILogger? logger = null) { Name = name; IpAddress = ipAddress; _status = ConnectionStatus.Disconnected; + _logger = logger ?? NullLogger.Instance; } /// @@ -247,11 +260,13 @@ public DaqifiDevice(string name, IPAddress? ipAddress = null) /// The name of the device. /// The stream for device communication. /// The IP address of the device, if known. - public DaqifiDevice(string name, Stream stream, IPAddress? ipAddress = null) + /// Optional logger for device diagnostics; a no-op logger is used when null. + public DaqifiDevice(string name, Stream stream, IPAddress? ipAddress = null, ILogger? logger = null) { Name = name; IpAddress = ipAddress; _status = ConnectionStatus.Disconnected; + _logger = logger ?? NullLogger.Instance; _messageProducer = new MessageProducer(stream); _directStream = stream; } @@ -261,12 +276,14 @@ public DaqifiDevice(string name, Stream stream, IPAddress? ipAddress = null) /// /// The name of the device. /// The transport for device communication. - public DaqifiDevice(string name, IStreamTransport transport) + /// Optional logger for device diagnostics; a no-op logger is used when null. + public DaqifiDevice(string name, IStreamTransport transport, ILogger? logger = null) { Name = name; _status = ConnectionStatus.Disconnected; + _logger = logger ?? NullLogger.Instance; _transport = transport; - + // Subscribe to transport status changes _transport.StatusChanged += OnTransportStatusChanged; } @@ -407,7 +424,7 @@ public void Disconnect() /// The message to send to the device. /// /// Thrown when the device is not connected, or when connected but has no transport or - /// stream to send on (e.g. the producer-less + /// stream to send on (e.g. the producer-less /// constructor). /// public virtual void Send(IOutboundMessage message) @@ -656,7 +673,7 @@ private async Task> ExecuteTextCommandCoreAsync( } } - Trace.WriteLine($"[ExecuteTextCommandAsync] Protobuf consumer stopped at {sw.ElapsedMilliseconds}ms"); + SafeLog(() => _logger.LogDebug("[ExecuteTextCommandAsync] Protobuf consumer stopped at {ElapsedMs}ms", sw.ElapsedMilliseconds)); // Create a temporary text consumer on the same stream using var textConsumer = new StreamMessageConsumer( @@ -673,13 +690,13 @@ private async Task> ExecuteTextCommandCoreAsync( // sync context (e.g. UI thread) would deadlock if that thread calls Disconnect(). await Task.Delay(50, cancellationToken).ConfigureAwait(false); - Trace.WriteLine($"[ExecuteTextCommandAsync] Text consumer started at {sw.ElapsedMilliseconds}ms"); + SafeLog(() => _logger.LogDebug("[ExecuteTextCommandAsync] Text consumer started at {ElapsedMs}ms", sw.ElapsedMilliseconds)); // Execute the setup action (sends SCPI commands). ConfigureAwait(false) // matches the surrounding lock-protected awaits. await setupActionAsync(cancellationToken).ConfigureAwait(false); - Trace.WriteLine($"[ExecuteTextCommandAsync] Setup action completed at {sw.ElapsedMilliseconds}ms"); + SafeLog(() => _logger.LogDebug("[ExecuteTextCommandAsync] Setup action completed at {ElapsedMs}ms", sw.ElapsedMilliseconds)); // Wait for responses using a two-phase inactivity-based timeout: // Phase 1: Wait up to responseTimeoutMs for the first response. @@ -699,7 +716,7 @@ private async Task> ExecuteTextCommandCoreAsync( if (!hasReceivedAny) { hasReceivedAny = true; - Trace.WriteLine($"[ExecuteTextCommandAsync] First response at {sw.ElapsedMilliseconds}ms"); + SafeLog(() => _logger.LogDebug("[ExecuteTextCommandAsync] First response at {ElapsedMs}ms", sw.ElapsedMilliseconds)); } } @@ -723,7 +740,7 @@ private async Task> ExecuteTextCommandCoreAsync( } } - Trace.WriteLine($"[ExecuteTextCommandAsync] Collection complete at {sw.ElapsedMilliseconds}ms, {collectedLines.Count} lines"); + SafeLog(() => _logger.LogDebug("[ExecuteTextCommandAsync] Collection complete at {ElapsedMs}ms, {LineCount} lines", sw.ElapsedMilliseconds, collectedLines.Count)); // Stop the text consumer textConsumer.StopSafely(); @@ -749,7 +766,7 @@ private async Task> ExecuteTextCommandCoreAsync( _messageConsumer.MessageReceived += OnInboundMessageReceived; } - Trace.WriteLine($"[ExecuteTextCommandAsync] Total elapsed: {sw.ElapsedMilliseconds}ms"); + SafeLog(() => _logger.LogDebug("[ExecuteTextCommandAsync] Total elapsed: {ElapsedMs}ms", sw.ElapsedMilliseconds)); } return collectedLines; @@ -832,7 +849,7 @@ public virtual async Task> DrainErrorQueueAsync( { // Empty reply means timeout or unresponsive device, not a // queued error — terminate rather than spin to maxIterations. - Trace.WriteLine($"[DrainErrorQueueAsync] Empty reply on iteration {i}; terminating after {popped.Count} popped entries."); + SafeLog(() => _logger.LogDebug("[DrainErrorQueueAsync] Empty reply on iteration {Iteration}; terminating after {PoppedCount} popped entries.", i, popped.Count)); return popped; } @@ -851,7 +868,7 @@ public virtual async Task> DrainErrorQueueAsync( popped.Add(reply); } - Trace.WriteLine($"[DrainErrorQueueAsync] Did not converge after {maxIterations} iterations; queue may still contain entries."); + SafeLog(() => _logger.LogWarning("[DrainErrorQueueAsync] Did not converge after {MaxIterations} iterations; queue may still contain entries.", maxIterations)); return popped; } @@ -1218,7 +1235,7 @@ protected virtual void OnStatusMessageReceived(DaqifiOutMessage message) /// The event delegate to invoke, or null if unsubscribed. /// The message to pass to subscribers. /// The event name, for the trace log if a subscriber throws. - private static void RaiseClassifiedEvent(Action? handler, DaqifiOutMessage message, string eventName) + private void RaiseClassifiedEvent(Action? handler, DaqifiOutMessage message, string eventName) { if (handler == null) { @@ -1231,7 +1248,25 @@ private static void RaiseClassifiedEvent(Action? handler, Daqi } catch (Exception ex) { - Trace.WriteLine($"[{eventName}] Subscriber threw: {ex}"); + SafeLog(() => _logger.LogWarning(ex, "[{EventName}] classified-event subscriber threw", eventName)); + } + } + + /// + /// Runs a logging call, swallowing any exception a misbehaving throws. + /// A consumer-supplied logger must never affect device operation — least of all in + /// , whose whole purpose is to isolate frame processing + /// from faults. Mirrors MessageProducer.SafeLog. + /// + private static void SafeLog(Action logAction) + { + try + { + logAction(); + } + catch + { + // A logger that throws is not permitted to take down device operation. } } @@ -1340,7 +1375,7 @@ private int PopulateAnalogChannels(DaqifiOutMessage message, Dictionary<(Channel if (resolutionIsAssumed && count > 0) { - Trace.WriteLine($"[PopulateAnalogChannels] Device '{Name}' reported no usable ADC resolution (analog_in_res={analogInResolution}) for {count} analog channel(s); assuming {resolution}. Scaled samples on this device may be systematically wrong."); + SafeLog(() => _logger.LogWarning("[PopulateAnalogChannels] Device '{DeviceName}' reported no usable ADC resolution (analog_in_res={Resolution}) for {ChannelCount} analog channel(s); assuming {AssumedResolution}. Scaled samples on this device may be systematically wrong.", Name, analogInResolution, count, resolution)); } for (var i = 0; i < count; i++) @@ -1397,7 +1432,7 @@ private double SanitizeScalingValue(double value, double fallback, double maxMag if (invalid) { - Trace.WriteLine($"[PopulateAnalogChannels] Device '{Name}' reported invalid {fieldName}={value} for analog channel {channelIndex}; substituting {fallback}. Scaled samples on this channel may be affected."); + SafeLog(() => _logger.LogWarning("[PopulateAnalogChannels] Device '{DeviceName}' reported invalid {FieldName}={Value} for analog channel {ChannelIndex}; substituting {Fallback}. Scaled samples on this channel may be affected.", Name, fieldName, value, channelIndex, fallback)); return fallback; } @@ -1413,7 +1448,7 @@ private double SanitizePortRange(double value, int channelIndex) { if (!double.IsFinite(value) || value <= 0.0 || value > AnalogChannel.MaxPortRangeVolts) { - Trace.WriteLine($"[PopulateAnalogChannels] Device '{Name}' reported invalid portRange={value} for analog channel {channelIndex}; substituting 1.0. Scaled samples on this channel may be affected."); + SafeLog(() => _logger.LogWarning("[PopulateAnalogChannels] Device '{DeviceName}' reported invalid portRange={Value} for analog channel {ChannelIndex}; substituting 1.0. Scaled samples on this channel may be affected.", Name, value, channelIndex)); return 1.0; } diff --git a/src/Daqifi.Core/Device/DaqifiDeviceFactory.cs b/src/Daqifi.Core/Device/DaqifiDeviceFactory.cs index ee041d3a..b87a8bc9 100644 --- a/src/Daqifi.Core/Device/DaqifiDeviceFactory.cs +++ b/src/Daqifi.Core/Device/DaqifiDeviceFactory.cs @@ -319,7 +319,8 @@ private static async Task ConnectWiFiDeviceAsync( DeviceName = deviceName, ConnectionRetry = effectiveOptions.ConnectionRetry, InitializeDevice = effectiveOptions.InitializeDevice, - ChannelPopulationTimeout = effectiveOptions.ChannelPopulationTimeout + ChannelPopulationTimeout = effectiveOptions.ChannelPopulationTimeout, + Logger = effectiveOptions.Logger }; // Honor LocalInterfaceAddress so multi-homed hosts egress on the NIC that @@ -361,7 +362,8 @@ private static async Task ConnectSerialDeviceAsync( DeviceName = deviceName, ConnectionRetry = effectiveOptions.ConnectionRetry, InitializeDevice = effectiveOptions.InitializeDevice, - ChannelPopulationTimeout = effectiveOptions.ChannelPopulationTimeout + ChannelPopulationTimeout = effectiveOptions.ChannelPopulationTimeout, + Logger = effectiveOptions.Logger }; return await ConnectSerialAsync( @@ -404,7 +406,7 @@ private static async Task ConnectWithTransportAsync( // Step 2: Create the device with the transport // Note: Once created, the device owns the transport and will dispose it - device = new DaqifiStreamingDevice(options.DeviceName, transport); + device = new DaqifiStreamingDevice(options.DeviceName, transport, options.Logger); // Step 3: Connect the device (starts message producers/consumers) device.Connect(); diff --git a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs index 24187f20..0f0cdc50 100644 --- a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs @@ -4,6 +4,7 @@ using Daqifi.Core.Communication.Producers; using Daqifi.Core.Communication.Transport; using Daqifi.Core.Device.Diagnostics; +using Microsoft.Extensions.Logging; using Daqifi.Core.Device.Network; using Daqifi.Core.Device.SdCard; using Daqifi.Core.Firmware; @@ -154,7 +155,9 @@ public int StreamingFrequency /// /// The name of the device. /// The IP address of the device, if known. - public DaqifiStreamingDevice(string name, IPAddress? ipAddress = null) : base(name, ipAddress) + /// Optional logger for device diagnostics; a no-op logger is used when null. + public DaqifiStreamingDevice(string name, IPAddress? ipAddress = null, ILogger? logger = null) + : base(name, ipAddress, logger) { StreamingFrequency = 100; } @@ -164,7 +167,9 @@ public DaqifiStreamingDevice(string name, IPAddress? ipAddress = null) : base(na /// /// The name of the device. /// The transport for device communication. - public DaqifiStreamingDevice(string name, IStreamTransport transport) : base(name, transport) + /// Optional logger for device diagnostics; a no-op logger is used when null. + public DaqifiStreamingDevice(string name, IStreamTransport transport, ILogger? logger = null) + : base(name, transport, logger) { StreamingFrequency = 100; } diff --git a/src/Daqifi.Core/Device/DeviceConnectionOptions.cs b/src/Daqifi.Core/Device/DeviceConnectionOptions.cs index 0931959c..3e7fb131 100644 --- a/src/Daqifi.Core/Device/DeviceConnectionOptions.cs +++ b/src/Daqifi.Core/Device/DeviceConnectionOptions.cs @@ -1,4 +1,5 @@ using Daqifi.Core.Communication.Transport; +using Microsoft.Extensions.Logging; #nullable enable @@ -36,6 +37,12 @@ public class DeviceConnectionOptions /// public TimeSpan ChannelPopulationTimeout { get; set; } = TimeSpan.FromSeconds(8); + /// + /// Optional logger the constructed device routes its diagnostics through (bad calibration/ + /// resolution warnings, SCPI text-exchange timing). When null, the device uses a no-op logger. + /// + public ILogger? Logger { get; set; } + /// /// Creates a default configuration with default retry behavior and device initialization enabled. ///