diff --git a/src/Daqifi.Core.Tests/Device/Network/NetworkConfigurableTests.cs b/src/Daqifi.Core.Tests/Device/Network/NetworkConfigurableTests.cs index 50160dc1..a3139c4c 100644 --- a/src/Daqifi.Core.Tests/Device/Network/NetworkConfigurableTests.cs +++ b/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( + () => 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() { diff --git a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs index 36896f8c..b50ee5c4 100644 --- a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs @@ -1217,10 +1217,17 @@ private void EnsureChannelBelongs(IChannel channel) /// /// Updates the device network configuration with the specified settings. /// + /// + /// 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 over WiFi + /// rather than dispatch a sequence it cannot complete (#352). + /// /// The new network configuration to apply. /// A cancellation token to observe while waiting for the task to complete. /// A task that represents the asynchronous operation. /// Thrown when the device is not connected. + /// Thrown when the active control transport is not USB. /// Thrown when is null. /// Thrown when an unsupported WiFi mode or security type is specified. /// Thrown when the operation is canceled. @@ -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); diff --git a/src/Daqifi.Core/Device/Network/INetworkConfigurable.cs b/src/Daqifi.Core/Device/Network/INetworkConfigurable.cs index e18cf7a4..b03f5c5e 100644 --- a/src/Daqifi.Core/Device/Network/INetworkConfigurable.cs +++ b/src/Daqifi.Core/Device/Network/INetworkConfigurable.cs @@ -21,7 +21,15 @@ public interface INetworkConfigurable /// A task that represents the asynchronous operation. /// /// - /// 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 + /// 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. + /// + /// + /// Once the USB precondition is met, this method performs the following steps: /// /// /// Stops any active streaming @@ -40,6 +48,7 @@ public interface INetworkConfigurable /// and cannot be used simultaneously. This method handles the interface switching automatically. /// /// + /// Thrown when the active control transport is not USB. Task UpdateNetworkConfigurationAsync(NetworkConfiguration configuration, CancellationToken cancellationToken = default); /// diff --git a/src/Daqifi.Core/Device/Network/NetworkReconfigurationRequiresUsbException.cs b/src/Daqifi.Core/Device/Network/NetworkReconfigurationRequiresUsbException.cs new file mode 100644 index 00000000..917403a6 --- /dev/null +++ b/src/Daqifi.Core/Device/Network/NetworkReconfigurationRequiresUsbException.cs @@ -0,0 +1,57 @@ +using System; + +namespace Daqifi.Core.Device.Network +{ + /// + /// Thrown by when it is + /// invoked over a WiFi/TCP control transport instead of USB. + /// + /// + /// The reconfiguration sequence applies the new LAN settings with + /// SYSTem:COMMunicate:LAN:APPLY (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 before the trailing SYSTem:COMMunicate:LAN:SAVE 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. + /// + 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."; + + /// + /// Initializes a new instance of the + /// class with a default explanatory message. + /// + public NetworkReconfigurationRequiresUsbException() + : base(DefaultMessage) + { + } + + /// + /// Initializes a new instance of the + /// class with a custom message. + /// + /// The message that describes the error. + public NetworkReconfigurationRequiresUsbException(string message) + : base(message) + { + } + + /// + /// Initializes a new instance of the + /// class with a custom message and inner exception. + /// + /// The message that describes the error. + /// The exception that is the cause of this exception. + public NetworkReconfigurationRequiresUsbException(string message, Exception innerException) + : base(message, innerException) + { + } + } +}