fix(network): reject WiFi reconfiguration over a WiFi/TCP transport (closes #352)

src/Daqifi.Core.Tests/Device/Network/NetworkConfigurableTests.cs

@@ -43,6 +43,48 @@ public async Task UpdateNetworkConfigurationAsync_WhenDisconnected_ThrowsInvalid
             Assert.Equal("Device is not connected.", exception.Message);
         }
 
+        [Fact]
+        public async Task UpdateNetworkConfigurationAsync_OverWifi_ThrowsRequiresUsbAndSendsNothing()
+        {
+            // Over a WiFi/TCP control connection, ApplyNetworkLan restarts the WiFi module and would
+            // drop the connection before SaveNetworkLan persists the config. The operation must be
+            // rejected up front, before any command is dispatched (#352).
+            var device = new TestableNonUsbDaqifiStreamingDevice("TestDevice");
+            device.Connect();
+            device.SentMessages.Clear();
+            var config = new NetworkConfiguration(
+                WifiMode.ExistingNetwork,
+                WifiSecurityType.WpaPskPhrase,
+                "TestNetwork",
+                "TestPassword");
+
+            await Assert.ThrowsAsync<NetworkReconfigurationRequiresUsbException>(
+                () => device.UpdateNetworkConfigurationAsync(config));
+
+            // Nothing half-applied: no reconfiguration command left the device.
+            Assert.Empty(device.SentMessages);
+        }
+
+        [Fact]
+        public async Task UpdateNetworkConfigurationAsync_OverUsb_DoesNotThrowRequiresUsb()
+        {
+            // The USB path is unaffected by the transport guard — a WiFi module restart does not
+            // disrupt the USB control connection, so the full sequence runs.
+            var device = new TestableDaqifiStreamingDevice("TestDevice");
+            device.Connect();
+            var config = new NetworkConfiguration(
+                WifiMode.ExistingNetwork,
+                WifiSecurityType.WpaPskPhrase,
+                "TestNetwork",
+                "TestPassword");
+
+            await device.UpdateNetworkConfigurationAsync(config);
+
+            var sentCommands = device.SentMessages.Select(m => m.Data).ToList();
+            Assert.Contains("SYSTem:COMMunicate:LAN:APPLY", sentCommands);
+            Assert.Contains("SYSTem:COMMunicate:LAN:SAVE", sentCommands);
+        }
+
         [Fact]
         public async Task UpdateNetworkConfigurationAsync_WithNullConfiguration_ThrowsArgumentNullException()
         {
@@ -321,27 +363,6 @@ public async Task UpdateNetworkConfigurationAsync_PreparesLanInterface()
             Assert.Contains("SYSTem:COMMunicate:LAN:ENAbled 1", sentCommands);
         }
 
-        [Fact]
-        public async Task UpdateNetworkConfigurationAsync_OverWifi_StillReEnablesLan()
-        {
-            // Regression: network reconfiguration must bring the LAN back up after ApplyNetworkLan
-            // even over a non-USB (WiFi/TCP) control transport — it owns the LAN state and must NOT
-            // rely on the transport-aware PrepareLanInterface (which leaves LAN alone over WiFi).
-            var device = new TestableNonUsbDaqifiStreamingDevice("TestDevice");
-            device.Connect();
-            var config = new NetworkConfiguration(
-                WifiMode.SelfHosted,
-                WifiSecurityType.WpaPskPhrase,
-                "TestNetwork",
-                "TestPassword");
-
-            await device.UpdateNetworkConfigurationAsync(config);
-
-            var sentCommands = device.SentMessages.Select(m => m.Data).ToList();
-            Assert.Contains("SYSTem:COMMunicate:LAN:ENAbled 1", sentCommands); // EnableNetworkLan (unconditional)
-            Assert.Contains("SYSTem:STORage:SD:ENAble 0", sentCommands);       // DisableStorageSd
-        }
-
         [Fact]
         public void PrepareSdInterface_WhenDisconnected_ThrowsInvalidOperationException()
         {

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs

@@ -1217,10 +1217,17 @@ private void EnsureChannelBelongs(IChannel channel)
         /// <summary>
         /// Updates the device network configuration with the specified settings.
         /// </summary>
+        /// <remarks>
+        /// Requires a USB control connection. Applying the LAN settings restarts the WiFi module,
+        /// which would drop a WiFi/TCP control connection before the trailing save is delivered, so
+        /// this method throws <see cref="NetworkReconfigurationRequiresUsbException"/> over WiFi
+        /// rather than dispatch a sequence it cannot complete (#352).
+        /// </remarks>
         /// <param name="configuration">The new network configuration to apply.</param>
         /// <param name="cancellationToken">A cancellation token to observe while waiting for the task to complete.</param>
         /// <returns>A task that represents the asynchronous operation.</returns>
         /// <exception cref="InvalidOperationException">Thrown when the device is not connected.</exception>
+        /// <exception cref="NetworkReconfigurationRequiresUsbException">Thrown when the active control transport is not USB.</exception>
         /// <exception cref="ArgumentNullException">Thrown when <paramref name="configuration"/> is null.</exception>
         /// <exception cref="ArgumentOutOfRangeException">Thrown when an unsupported WiFi mode or security type is specified.</exception>
         /// <exception cref="OperationCanceledException">Thrown when the operation is canceled.</exception>
@@ -1238,6 +1245,17 @@ public async Task UpdateNetworkConfigurationAsync(NetworkConfiguration configura
                 throw new InvalidOperationException("Device is not connected.");
             }
 
+            // Reject reconfiguration over WiFi/TCP up front. ApplyNetworkLan (and the later
+            // EnableNetworkLan) restart the WiFi module, tearing down a WiFi/TCP control connection
+            // before the trailing SaveNetworkLan is delivered — leaving the new config
+            // applied-but-not-persisted (lost on power cycle) or the tail undelivered. Fail before
+            // sending any command so nothing is half-applied; the caller must reconfigure over USB
+            // (#352).
+            if (!IsUsbConnection)
+            {
+                throw new NetworkReconfigurationRequiresUsbException();
+            }
+
             // Stop streaming if active
             if (IsStreaming)
             {
@@ -1297,11 +1315,11 @@ public async Task UpdateNetworkConfigurationAsync(NetworkConfiguration configura
             await Task.Delay(WIFI_MODULE_RESTART_DELAY_MS, cancellationToken);
 
             // Re-enable the LAN interface after the reconfig. ApplyNetworkLan restarts the WiFi
-            // module, so this is a network-configuration step that OWNS the LAN state and must
-            // bring LAN back up regardless of the control transport. It deliberately does NOT call
-            // PrepareLanInterface() — that is the transport-aware SD-operation restore, which
-            // leaves the LAN alone over WiFi (where #598/#599 keep it up). Here the LAN enable is
-            // unconditional.
+            // module, so this is a network-configuration step that OWNS the LAN state and brings it
+            // back up. It deliberately does NOT call PrepareLanInterface() — that is the
+            // transport-aware SD-operation restore, which leaves the LAN alone over WiFi. Here the
+            // LAN enable is unconditional; the USB-only guard above guarantees this runs only over a
+            // control connection the WiFi-module restart cannot drop (#352).
             Send(ScpiMessageProducer.DisableStorageSd);
             Send(ScpiMessageProducer.EnableNetworkLan);
 

src/Daqifi.Core/Device/Network/INetworkConfigurable.cs

@@ -21,7 +21,15 @@ public interface INetworkConfigurable
         /// <returns>A task that represents the asynchronous operation.</returns>
         /// <remarks>
         /// <para>
-        /// This method performs the following steps:
+        /// Requires a USB control connection. Applying the LAN settings restarts the WiFi module,
+        /// which would drop a WiFi/TCP control connection before the save step is delivered (leaving
+        /// the new config applied-but-not-persisted), so this method throws
+        /// <see cref="NetworkReconfigurationRequiresUsbException"/> when invoked over WiFi/TCP. The
+        /// transport is checked first: over a non-USB transport the method throws before stopping
+        /// streaming and before dispatching any command, so nothing is left half-applied.
+        /// </para>
+        /// <para>
+        /// Once the USB precondition is met, this method performs the following steps:
         /// </para>
         /// <list type="number">
         /// <item><description>Stops any active streaming</description></item>
@@ -40,6 +48,7 @@ public interface INetworkConfigurable
         /// and cannot be used simultaneously. This method handles the interface switching automatically.
         /// </para>
         /// </remarks>
+        /// <exception cref="NetworkReconfigurationRequiresUsbException">Thrown when the active control transport is not USB.</exception>
         Task UpdateNetworkConfigurationAsync(NetworkConfiguration configuration, CancellationToken cancellationToken = default);
 
         /// <summary>

src/Daqifi.Core/Device/Network/NetworkReconfigurationRequiresUsbException.cs

@@ -0,0 +1,57 @@
+using System;
+
+namespace Daqifi.Core.Device.Network
+{
+    /// <summary>
+    /// Thrown by <see cref="INetworkConfigurable.UpdateNetworkConfigurationAsync"/> when it is
+    /// invoked over a WiFi/TCP control transport instead of USB.
+    /// </summary>
+    /// <remarks>
+    /// The reconfiguration sequence applies the new LAN settings with
+    /// <c>SYSTem:COMMunicate:LAN:APPLY</c> (and later re-enables the interface), which restarts the
+    /// WiFi module. Over a WiFi/TCP control connection that restart tears down the very channel
+    /// carrying the command stream <b>before</b> the trailing <c>SYSTem:COMMunicate:LAN:SAVE</c> can
+    /// be delivered, leaving the device with the new settings applied-but-not-persisted (lost on the
+    /// next power cycle) or the tail of the sequence undelivered entirely. Because the outcome is
+    /// unrecoverable from the caller's side once the connection drops, the operation is rejected up
+    /// front over WiFi rather than dispatched into that race. Reconnect the device over USB and
+    /// reissue the reconfiguration. See issue #352.
+    /// </remarks>
+    public sealed class NetworkReconfigurationRequiresUsbException : InvalidOperationException
+    {
+        private const string DefaultMessage =
+            "Network reconfiguration requires a USB connection. Applying LAN settings restarts the " +
+            "WiFi module, which would drop a WiFi/TCP control connection before the configuration is " +
+            "saved. Reconnect the device over USB and retry.";
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="NetworkReconfigurationRequiresUsbException"/>
+        /// class with a default explanatory message.
+        /// </summary>
+        public NetworkReconfigurationRequiresUsbException()
+            : base(DefaultMessage)
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="NetworkReconfigurationRequiresUsbException"/>
+        /// class with a custom message.
+        /// </summary>
+        /// <param name="message">The message that describes the error.</param>
+        public NetworkReconfigurationRequiresUsbException(string message)
+            : base(message)
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="NetworkReconfigurationRequiresUsbException"/>
+        /// class with a custom message and inner exception.
+        /// </summary>
+        /// <param name="message">The message that describes the error.</param>
+        /// <param name="innerException">The exception that is the cause of this exception.</param>
+        public NetworkReconfigurationRequiresUsbException(string message, Exception innerException)
+            : base(message, innerException)
+        {
+        }
+    }
+}