From e5eb2c925d6d0d2a1088f39b6e0559a57727989e Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Sat, 18 Jul 2026 22:36:46 -0600 Subject: [PATCH 1/2] feat(device): route DaqifiDevice diagnostics through an optional ILogger (part of #340) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DaqifiDevice emitted all its diagnostics — including the bad-calibration/missing-resolution warnings that mean systematically wrong scaled samples — via Trace.WriteLine, invisible to consumers on Microsoft.Extensions.Logging. It now accepts an optional ILogger (default NullLogger), threaded via DeviceConnectionOptions.Logger through the factory, and all 12 Trace.WriteLine sites route through it with message templates: Warning for calibration/resolution anomalies, drain-not-converged, and classified-event subscriber exceptions; Debug for SCPI text-exchange timing. RaiseClassifiedEvent is now an instance method so it can log. Non-breaking (logger params are optional). Scope: this covers the central DaqifiDevice + factory reachability (acceptance criteria 1-2 of #340); finders/transports and the FirmwareUpdateService NullLogger default remain follow-ups, so this does NOT close #340. - 4 unit tests (bad resolution -> Warning through the injected logger; valid status -> no warning; no logger -> NullLogger, no throw; options.Logger defaults null). Core 1639 pass, MCP 23 pass. Co-Authored-By: Claude Opus 4.8 --- .../Device/DaqifiDeviceLoggerTests.cs | 81 +++++++++++++++++++ src/Daqifi.Core/Device/DaqifiDevice.cs | 53 +++++++----- src/Daqifi.Core/Device/DaqifiDeviceFactory.cs | 2 +- .../Device/DaqifiStreamingDevice.cs | 9 ++- .../Device/DeviceConnectionOptions.cs | 7 ++ 5 files changed, 131 insertions(+), 21 deletions(-) create mode 100644 src/Daqifi.Core.Tests/Device/DaqifiDeviceLoggerTests.cs diff --git a/src/Daqifi.Core.Tests/Device/DaqifiDeviceLoggerTests.cs b/src/Daqifi.Core.Tests/Device/DaqifiDeviceLoggerTests.cs new file mode 100644 index 00000000..a971be81 --- /dev/null +++ b/src/Daqifi.Core.Tests/Device/DaqifiDeviceLoggerTests.cs @@ -0,0 +1,81 @@ +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); + } + + 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))); + } + } +} diff --git a/src/Daqifi.Core/Device/DaqifiDevice.cs b/src/Daqifi.Core/Device/DaqifiDevice.cs index 70ae49a2..57d4a257 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"); + _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"); + _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"); + _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"); + _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"); + _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"); + _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."); + _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."); + _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,7 @@ private static void RaiseClassifiedEvent(Action? handler, Daqi } catch (Exception ex) { - Trace.WriteLine($"[{eventName}] Subscriber threw: {ex}"); + _logger.LogWarning(ex, "[{EventName}] classified-event subscriber threw", eventName); } } @@ -1340,7 +1357,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."); + _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 +1414,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."); + _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 +1430,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."); + _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..18ff5876 100644 --- a/src/Daqifi.Core/Device/DaqifiDeviceFactory.cs +++ b/src/Daqifi.Core/Device/DaqifiDeviceFactory.cs @@ -404,7 +404,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. /// From 52abfc1bb5e03716df89fbff20de620acdab379d Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Sat, 18 Jul 2026 22:50:28 -0600 Subject: [PATCH 2/2] fix(device): thread Logger through device-info factory path + isolate throwing loggers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses two Qodo findings on #360: - Factory: ConnectFromDeviceInfoAsync's WiFi/serial paths rebuilt DeviceConnectionOptions without copying Logger, so the device silently fell back to NullLogger on the discovery path. Now the rebuilt options carry Logger = effectiveOptions.Logger. - Reliability: all 12 DaqifiDevice logger calls are now wrapped in SafeLog (mirrors MessageProducer.SafeLog) so a throwing consumer ILogger can't escape — most importantly in RaiseClassifiedEvent's catch, whose whole purpose is to isolate frame processing from faults. - New test: a throwing ILogger on the calibration-warning path is swallowed and does not propagate. Full suite 1640 pass. Co-Authored-By: Claude Opus 4.8 --- .../Device/DaqifiDeviceLoggerTests.cs | 20 +++++++++ src/Daqifi.Core/Device/DaqifiDevice.cs | 42 +++++++++++++------ src/Daqifi.Core/Device/DaqifiDeviceFactory.cs | 6 ++- 3 files changed, 54 insertions(+), 14 deletions(-) diff --git a/src/Daqifi.Core.Tests/Device/DaqifiDeviceLoggerTests.cs b/src/Daqifi.Core.Tests/Device/DaqifiDeviceLoggerTests.cs index a971be81..143237d8 100644 --- a/src/Daqifi.Core.Tests/Device/DaqifiDeviceLoggerTests.cs +++ b/src/Daqifi.Core.Tests/Device/DaqifiDeviceLoggerTests.cs @@ -54,6 +54,17 @@ 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 @@ -77,5 +88,14 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except 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 57d4a257..413681ce 100644 --- a/src/Daqifi.Core/Device/DaqifiDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiDevice.cs @@ -673,7 +673,7 @@ private async Task> ExecuteTextCommandCoreAsync( } } - _logger.LogDebug("[ExecuteTextCommandAsync] Protobuf consumer stopped at {ElapsedMs}ms", sw.ElapsedMilliseconds); + 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( @@ -690,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); - _logger.LogDebug("[ExecuteTextCommandAsync] Text consumer started at {ElapsedMs}ms", sw.ElapsedMilliseconds); + 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); - _logger.LogDebug("[ExecuteTextCommandAsync] Setup action completed at {ElapsedMs}ms", sw.ElapsedMilliseconds); + 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. @@ -716,7 +716,7 @@ private async Task> ExecuteTextCommandCoreAsync( if (!hasReceivedAny) { hasReceivedAny = true; - _logger.LogDebug("[ExecuteTextCommandAsync] First response at {ElapsedMs}ms", sw.ElapsedMilliseconds); + SafeLog(() => _logger.LogDebug("[ExecuteTextCommandAsync] First response at {ElapsedMs}ms", sw.ElapsedMilliseconds)); } } @@ -740,7 +740,7 @@ private async Task> ExecuteTextCommandCoreAsync( } } - _logger.LogDebug("[ExecuteTextCommandAsync] Collection complete at {ElapsedMs}ms, {LineCount} lines", sw.ElapsedMilliseconds, collectedLines.Count); + SafeLog(() => _logger.LogDebug("[ExecuteTextCommandAsync] Collection complete at {ElapsedMs}ms, {LineCount} lines", sw.ElapsedMilliseconds, collectedLines.Count)); // Stop the text consumer textConsumer.StopSafely(); @@ -766,7 +766,7 @@ private async Task> ExecuteTextCommandCoreAsync( _messageConsumer.MessageReceived += OnInboundMessageReceived; } - _logger.LogDebug("[ExecuteTextCommandAsync] Total elapsed: {ElapsedMs}ms", sw.ElapsedMilliseconds); + SafeLog(() => _logger.LogDebug("[ExecuteTextCommandAsync] Total elapsed: {ElapsedMs}ms", sw.ElapsedMilliseconds)); } return collectedLines; @@ -849,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. - _logger.LogDebug("[DrainErrorQueueAsync] Empty reply on iteration {Iteration}; terminating after {PoppedCount} popped entries.", i, popped.Count); + SafeLog(() => _logger.LogDebug("[DrainErrorQueueAsync] Empty reply on iteration {Iteration}; terminating after {PoppedCount} popped entries.", i, popped.Count)); return popped; } @@ -868,7 +868,7 @@ public virtual async Task> DrainErrorQueueAsync( popped.Add(reply); } - _logger.LogWarning("[DrainErrorQueueAsync] Did not converge after {MaxIterations} iterations; queue may still contain entries.", maxIterations); + SafeLog(() => _logger.LogWarning("[DrainErrorQueueAsync] Did not converge after {MaxIterations} iterations; queue may still contain entries.", maxIterations)); return popped; } @@ -1248,7 +1248,25 @@ private void RaiseClassifiedEvent(Action? handler, DaqifiOutMe } catch (Exception ex) { - _logger.LogWarning(ex, "[{EventName}] classified-event subscriber threw", eventName); + 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. } } @@ -1357,7 +1375,7 @@ private int PopulateAnalogChannels(DaqifiOutMessage message, Dictionary<(Channel if (resolutionIsAssumed && count > 0) { - _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); + 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++) @@ -1414,7 +1432,7 @@ private double SanitizeScalingValue(double value, double fallback, double maxMag if (invalid) { - _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); + 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; } @@ -1430,7 +1448,7 @@ private double SanitizePortRange(double value, int channelIndex) { if (!double.IsFinite(value) || value <= 0.0 || value > AnalogChannel.MaxPortRangeVolts) { - _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); + 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 18ff5876..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(