From 2a23c2985b6d49a3da66fc123bddab34edfb553f Mon Sep 17 00:00:00 2001
From: Tyler Kron <tylerkron@gmail.com>
Date: Sat, 18 Jul 2026 23:30:12 -0600
Subject: [PATCH] feat(firmware): make FirmwareUpdateService logger optional
 (NullLogger default) (part of #340)

FirmwareUpdateService required a non-nullable ILogger, so DI-less consumers had to construct and
pass one just to use it. The logger parameter is now optional and defaults to
NullLogger<FirmwareUpdateService>.Instance, matching the optional-logger convention the rest of the
library uses. Non-breaking (callers passing a logger are unaffected; the parameter sits before the
existing optional parameters). No flash-path logic changed.

Addresses acceptance-criterion 4 of #340.

- New test: construction without a logger does not throw. Full suite 1636 pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 .../Firmware/FirmwareUpdateServiceTests.cs        | 12 ++++++++++++
 src/Daqifi.Core/Firmware/FirmwareUpdateService.cs | 15 +++++++++------
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs b/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs
index f1fd8346..bdac143f 100644
--- a/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs
+++ b/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs
@@ -13,6 +13,18 @@ namespace Daqifi.Core.Tests.Firmware;
 
 public class FirmwareUpdateServiceTests
 {
+    [Fact]
+    public void Constructor_WithoutLogger_UsesNullLogger_DoesNotThrow()
+    {
+        // #340: the service is usable without wiring a logger (falls back to NullLogger).
+        var ex = Record.Exception(() => new FirmwareUpdateService(
+            new FakeHidTransport(),
+            new FakeFirmwareDownloadService(),
+            new FakeExternalProcessRunner()));
+
+        Assert.Null(ex);
+    }
+
     [Fact]
     public async Task UpdateFirmwareAsync_HappyPath_TransitionsThroughExpectedStatesAndReportsProgress()
     {
diff --git a/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs b/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs
index 33048d81..d94fecdc 100644
--- a/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs
+++ b/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs
@@ -7,6 +7,7 @@
 using Daqifi.Core.Device;
 using Daqifi.Core.Device.Discovery;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
 
 namespace Daqifi.Core.Firmware;
 
@@ -143,16 +144,18 @@ private static readonly IReadOnlySet<FirmwareUpdateState> CleanupEligibleStates
     private bool _disposed;
 
     /// <summary>
-    /// Initializes a new firmware update service. <paramref name="usbLocationProvider"/> resolves
-    /// an enumerated bootloader's USB physical-location key when targeting by
-    /// <c>targetLocationKey</c>; when null, a platform-default provider is used (Windows → WMI,
-    /// others → no-op fallback, which makes location-key targeting a no-op).
+    /// Initializes a new firmware update service. The optional <paramref name="logger"/> defaults to
+    /// a no-op logger (<see cref="NullLogger{T}.Instance"/>) when omitted, so the service is usable
+    /// without wiring up logging. <paramref name="usbLocationProvider"/> resolves an enumerated
+    /// bootloader's USB physical-location key when targeting by <c>targetLocationKey</c>; when null,
+    /// a platform-default provider is used (Windows → WMI, others → no-op fallback, which makes
+    /// location-key targeting a no-op).
     /// </summary>
     public FirmwareUpdateService(
         IHidTransport hidTransport,
         IFirmwareDownloadService firmwareDownloadService,
         IExternalProcessRunner externalProcessRunner,
-        ILogger<FirmwareUpdateService> logger,
+        ILogger<FirmwareUpdateService>? logger = null,
         IBootloaderProtocol? bootloaderProtocol = null,
         IHidDeviceEnumerator? hidDeviceEnumerator = null,
         FirmwareUpdateServiceOptions? options = null,
@@ -161,7 +164,7 @@ public FirmwareUpdateService(
         _hidTransport = hidTransport ?? throw new ArgumentNullException(nameof(hidTransport));
         FirmwareDownloadService = firmwareDownloadService ?? throw new ArgumentNullException(nameof(firmwareDownloadService));
         _externalProcessRunner = externalProcessRunner ?? throw new ArgumentNullException(nameof(externalProcessRunner));
-        _logger = logger ?? throw new ArgumentNullException(nameof(logger));
+        _logger = logger ?? NullLogger<FirmwareUpdateService>.Instance;
         _bootloaderProtocol = bootloaderProtocol ?? new Pic32BootloaderProtocol();
         _hidDeviceEnumerator = hidDeviceEnumerator ?? new HidLibraryDeviceEnumerator();
         _options = options ?? new FirmwareUpdateServiceOptions();