feat(device): route DaqifiDevice diagnostics through an optional ILogger (part of #340)

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>(TState state) where TState : notnull => null;
+            public bool IsEnabled(LogLevel logLevel) => true;
+
+            public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception,
+                Func<TState, Exception?, string> formatter)
+                => Entries.Add((logLevel, formatter(state, exception)));
+        }
+
+        private sealed class ThrowingLogger : ILogger
+        {
+            public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;
+            public bool IsEnabled(LogLevel logLevel) => true;
+            public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception,
+                Func<TState, Exception?, string> formatter)
+                => throw new InvalidOperationException("boom");
+        }
+    }
+}

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<IChannel> GetChannelsSnapshot()
         public DeviceState State { get; private set; } = DeviceState.Disconnected;
 
         private ConnectionStatus _status;
+
+        /// <summary>
+        /// Sink for this device's diagnostics. Defaults to <see cref="NullLogger.Instance"/> 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
+        /// <c>Trace.WriteLine</c> path to consumers on Microsoft.Extensions.Logging. Never null.
+        /// </summary>
+        private readonly ILogger _logger;
+
         private IMessageProducer<string>? _messageProducer;
         private IMessageConsumer<DaqifiOutMessage>? _messageConsumer;
         private readonly IStreamTransport? _transport;
@@ -234,11 +245,13 @@ private set
         /// </summary>
         /// <param name="name">The name of the device.</param>
         /// <param name="ipAddress">The IP address of the device, if known.</param>
-        public DaqifiDevice(string name, IPAddress? ipAddress = null)
+        /// <param name="logger">Optional logger for device diagnostics; a no-op logger is used when null.</param>
+        public DaqifiDevice(string name, IPAddress? ipAddress = null, ILogger? logger = null)
         {
             Name = name;
             IpAddress = ipAddress;
             _status = ConnectionStatus.Disconnected;
+            _logger = logger ?? NullLogger.Instance;
         }
 
         /// <summary>
@@ -247,11 +260,13 @@ public DaqifiDevice(string name, IPAddress? ipAddress = null)
         /// <param name="name">The name of the device.</param>
         /// <param name="stream">The stream for device communication.</param>
         /// <param name="ipAddress">The IP address of the device, if known.</param>
-        public DaqifiDevice(string name, Stream stream, IPAddress? ipAddress = null)
+        /// <param name="logger">Optional logger for device diagnostics; a no-op logger is used when null.</param>
+        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<string>(stream);
             _directStream = stream;
         }
@@ -261,12 +276,14 @@ public DaqifiDevice(string name, Stream stream, IPAddress? ipAddress = null)
         /// </summary>
         /// <param name="name">The name of the device.</param>
         /// <param name="transport">The transport for device communication.</param>
-        public DaqifiDevice(string name, IStreamTransport transport)
+        /// <param name="logger">Optional logger for device diagnostics; a no-op logger is used when null.</param>
+        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()
         /// <param name="message">The message to send to the device.</param>
         /// <exception cref="InvalidOperationException">
         /// Thrown when the device is not connected, or when connected but has no transport or
-        /// stream to send on (e.g. the producer-less <see cref="DaqifiDevice(string, IPAddress)"/>
+        /// stream to send on (e.g. the producer-less <see cref="DaqifiDevice(string, IPAddress, ILogger)"/>
         /// constructor).
         /// </exception>
         public virtual void Send<T>(IOutboundMessage<T> message)
@@ -656,7 +673,7 @@ private async Task<IReadOnlyList<string>> 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<string>(
@@ -673,13 +690,13 @@ private async Task<IReadOnlyList<string>> 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<IReadOnlyList<string>> 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<IReadOnlyList<string>> 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<IReadOnlyList<string>> 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<IReadOnlyList<string>> 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<IReadOnlyList<string>> 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)
         /// <param name="handler">The event delegate to invoke, or <c>null</c> if unsubscribed.</param>
         /// <param name="message">The message to pass to subscribers.</param>
         /// <param name="eventName">The event name, for the trace log if a subscriber throws.</param>
-        private static void RaiseClassifiedEvent(Action<DaqifiOutMessage>? handler, DaqifiOutMessage message, string eventName)
+        private void RaiseClassifiedEvent(Action<DaqifiOutMessage>? handler, DaqifiOutMessage message, string eventName)
         {
             if (handler == null)
             {
@@ -1231,7 +1248,25 @@ private static void RaiseClassifiedEvent(Action<DaqifiOutMessage>? handler, Daqi
             }
             catch (Exception ex)
             {
-                Trace.WriteLine($"[{eventName}] Subscriber threw: {ex}");
+                SafeLog(() => _logger.LogWarning(ex, "[{EventName}] classified-event subscriber threw", eventName));
+            }
+        }
+
+        /// <summary>
+        /// Runs a logging call, swallowing any exception a misbehaving <see cref="ILogger"/> throws.
+        /// A consumer-supplied logger must never affect device operation — least of all in
+        /// <see cref="RaiseClassifiedEvent"/>, whose whole purpose is to isolate frame processing
+        /// from faults. Mirrors <c>MessageProducer.SafeLog</c>.
+        /// </summary>
+        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;
             }
 

src/Daqifi.Core/Device/DaqifiDeviceFactory.cs

@@ -319,7 +319,8 @@ private static async Task<DaqifiDevice> 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<DaqifiDevice> 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<DaqifiDevice> 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();

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
         /// </summary>
         /// <param name="name">The name of the device.</param>
         /// <param name="ipAddress">The IP address of the device, if known.</param>
-        public DaqifiStreamingDevice(string name, IPAddress? ipAddress = null) : base(name, ipAddress)
+        /// <param name="logger">Optional logger for device diagnostics; a no-op logger is used when null.</param>
+        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
         /// </summary>
         /// <param name="name">The name of the device.</param>
         /// <param name="transport">The transport for device communication.</param>
-        public DaqifiStreamingDevice(string name, IStreamTransport transport) : base(name, transport)
+        /// <param name="logger">Optional logger for device diagnostics; a no-op logger is used when null.</param>
+        public DaqifiStreamingDevice(string name, IStreamTransport transport, ILogger? logger = null)
+            : base(name, transport, logger)
         {
             StreamingFrequency = 100;
         }