diff --git a/Daqifi.Desktop.Test/Device/DisplayIdentifierTests.cs b/Daqifi.Desktop.Test/Device/DisplayIdentifierTests.cs index 9c614b22..8c140c85 100644 --- a/Daqifi.Desktop.Test/Device/DisplayIdentifierTests.cs +++ b/Daqifi.Desktop.Test/Device/DisplayIdentifierTests.cs @@ -73,12 +73,33 @@ public void SerialStreamingDevice_ConnectionType_ShouldBeUsb() { // Arrange var device = new SerialStreamingDevice("COM1"); - + // Act & Assert Assert.AreEqual(ConnectionType.Usb, device.ConnectionType, "SerialStreamingDevice should have USB connection type"); } - + + [TestMethod] + public void SerialStreamingDevice_Connect_WithMissingPort_ReturnsFalseWithoutThrowing() + { + // Regression for issue #524: SerialPort.Open() throws FileNotFoundException on Windows + // when a syntactically-valid but absent COM port (e.g. "COM250") is opened. Connect() + // must catch and return false rather than letting the exception escape — previously + // this surfaced as a Sentry "error" for what is a user/environmental condition. + // "COM250" is intentionally chosen so it parses as a real COM port name (so we hit + // the FileNotFoundException path on Windows) but is unlikely to be present on a CI + // runner. On non-Windows hosts SerialPort.Open() throws PlatformNotSupportedException + // or similar; the generic Exception catch still suffices to satisfy this assertion. + var device = new SerialStreamingDevice("COM250"); + + var result = device.Connect(); + + Assert.IsFalse(result, + "Connect() against a nonexistent port should report failure, not throw."); + Assert.IsFalse(device.IsConnected, + "Device should not be marked connected when the underlying port could not be opened."); + } + #endregion #region DaqifiStreamingDevice (WiFi) Tests diff --git a/Daqifi.Desktop.Test/ViewModels/ConnectionDialogViewModelCloseTests.cs b/Daqifi.Desktop.Test/ViewModels/ConnectionDialogViewModelCloseTests.cs index 2fec520e..cb7306f4 100644 --- a/Daqifi.Desktop.Test/ViewModels/ConnectionDialogViewModelCloseTests.cs +++ b/Daqifi.Desktop.Test/ViewModels/ConnectionDialogViewModelCloseTests.cs @@ -8,6 +8,10 @@ namespace Daqifi.Desktop.Test.ViewModels; [TestClass] public class ConnectionDialogViewModelCloseTests { + // A name guaranteed to be absent from SerialPort.GetPortNames() on every CI runner — + // Windows uses COM1..COMn, macOS/Linux use /dev/tty.* paths. + private const string NonexistentPortName = "COM_DOES_NOT_EXIST_524"; + private Func? _originalDuplicateDeviceHandler; [TestInitialize] @@ -66,6 +70,37 @@ public async Task ConnectManualSerialCommand_WithBlankPort_DoesNotRaiseCloseRequ await viewModel.ConnectManualSerialCommand.ExecuteAsync(null); Assert.IsFalse(closeRaised(), "CloseRequested should not fire when the manual port is blank."); + Assert.IsNull(viewModel.ManualPortError, + "Blank input should not surface the missing-port error message."); + } + + [TestMethod] + public async Task ConnectManualSerialCommand_WithNonexistentPort_SetsManualPortError() + { + var viewModel = CreateViewModel(); + viewModel.ManualPortName = NonexistentPortName; + var closeRaised = SubscribeToClose(viewModel); + + await viewModel.ConnectManualSerialCommand.ExecuteAsync(null); + + Assert.IsFalse(closeRaised(), + "CloseRequested should not fire when the entered port is not present on the system."); + Assert.IsNotNull(viewModel.ManualPortError, + "ManualPortError should be set when the entered port is not present on the system."); + StringAssert.Contains(viewModel.ManualPortError, NonexistentPortName, + "Error message should mention the offending port name."); + } + + [TestMethod] + public void ManualPortError_ClearsWhenManualPortNameChanges() + { + var viewModel = CreateViewModel(); + viewModel.ManualPortError = "stale validation message"; + + viewModel.ManualPortName = "COM1"; + + Assert.IsNull(viewModel.ManualPortError, + "Editing the manual port name should clear any prior validation error."); } [TestMethod] diff --git a/Daqifi.Desktop/App.xaml b/Daqifi.Desktop/App.xaml index 6d3543f4..d2c65fd4 100644 --- a/Daqifi.Desktop/App.xaml +++ b/Daqifi.Desktop/App.xaml @@ -47,6 +47,7 @@ + diff --git a/Daqifi.Desktop/Device/SerialDevice/SerialStreamingDevice.cs b/Daqifi.Desktop/Device/SerialDevice/SerialStreamingDevice.cs index 599d4ada..13bcd17d 100644 --- a/Daqifi.Desktop/Device/SerialDevice/SerialStreamingDevice.cs +++ b/Daqifi.Desktop/Device/SerialDevice/SerialStreamingDevice.cs @@ -1,4 +1,5 @@ -using System.IO.Ports; +using System.IO; +using System.IO.Ports; using Daqifi.Core.Device; using Daqifi.Core.Communication.Transport; using Daqifi.Core.Communication.Messages; @@ -112,6 +113,24 @@ public override bool Connect() WaitForInitialStatusMessage(); return true; } + catch (FileNotFoundException ex) + { + // .NET's SerialPort.Open throws FileNotFoundException when the COM port no + // longer exists (device unplugged, never present, or renamed). Treat as a + // user/environmental condition, not an app bug — log a warning instead of + // capturing to Sentry. + AppLogger.Warning($"Cannot connect on {PortName}: port is not available ({ex.Message})"); + CleanupConnection(); + return false; + } + catch (UnauthorizedAccessException ex) + { + // SerialPort.Open throws UnauthorizedAccessException when another process + // already holds the port open. Same classification as above. + AppLogger.Warning($"Cannot connect on {PortName}: port is in use by another process ({ex.Message})"); + CleanupConnection(); + return false; + } catch (Exception ex) { AppLogger.Error(ex, $"Failed to connect on {PortName}"); diff --git a/Daqifi.Desktop/View/ConnectionDialog.xaml b/Daqifi.Desktop/View/ConnectionDialog.xaml index 5d8248da..4e99802a 100644 --- a/Daqifi.Desktop/View/ConnectionDialog.xaml +++ b/Daqifi.Desktop/View/ConnectionDialog.xaml @@ -411,6 +411,10 @@ + AvailableSerialDevices { get; } = []; public ObservableCollection AvailableHidDevices { get; } = []; - public string ManualPortName { get; set; } + [ObservableProperty] + private string? _manualPortName; + + /// + /// User-facing validation message for the Manual USB tab. Non-null when the entered + /// COM port failed pre-flight validation (e.g. port not present on the system). + /// Cleared automatically when the user edits . + /// + [ObservableProperty] + private string? _manualPortError; public SerialStreamingDevice ManualSerialDevice { get; set; } public string ManualIpAddress { get; set; } #endregion + partial void OnManualPortNameChanged(string? value) + { + // Clear stale validation error as soon as the user starts editing the port name. + if (!string.IsNullOrEmpty(ManualPortError)) + { + ManualPortError = null; + } + } + #region Constructor public ConnectionDialogViewModel() : this(ServiceLocator.Resolve()) { } @@ -215,14 +234,59 @@ private async Task ConnectSerialAsync(object? selectedItems) private async Task ConnectManualSerialAsync() { + ManualPortError = null; + if (string.IsNullOrWhiteSpace(ManualPortName)) { return; } - ManualSerialDevice = new SerialStreamingDevice(ManualPortName); + var portName = ManualPortName.Trim(); + + if (!IsPortAvailable(portName)) + { + // Avoid the FileNotFoundException round-trip from SerialPort.Open by + // pre-checking against the system's enumerated ports. Surface a friendly + // message in the dialog instead of silently closing. + ManualPortError = + $"Port '{portName}' is not available. Plug in the device or check Device Manager for the correct port name."; + Common.Loggers.AppLogger.Instance.Warning( + $"Manual serial connect rejected: port '{portName}' is not present on the system."); + return; + } + + ManualSerialDevice = new SerialStreamingDevice(portName); await ConnectionManager.Instance.Connect(ManualSerialDevice); + // Post-connect status check covers failures the pre-flight enumeration cannot + // catch — most notably "port exists but is held by another process", which + // SerialStreamingDevice.Connect now classifies as a Warning (no Sentry capture). + // Without this, the dialog would close silently and the user would have no idea + // the connection failed. + if (ConnectionManager.Instance.ConnectionStatus == DAQiFiConnectionStatus.Error) + { + ManualPortError = + $"Could not connect to '{portName}'. The port may be in use by another application or the device is not responding."; + return; + } + RaiseCloseRequested(); } + private static bool IsPortAvailable(string portName) + { + try + { + return SerialPort.GetPortNames() + .Any(p => string.Equals(p, portName, StringComparison.OrdinalIgnoreCase)); + } + catch (Exception ex) + { + // If port enumeration itself fails, fall back to the connect attempt rather + // than blocking the user — Connect() will surface its own error path. + Common.Loggers.AppLogger.Instance.Warning( + $"Failed to enumerate serial ports during manual-connect validation: {ex.Message}"); + return true; + } + } + private async Task ConnectManualWifiAsync() { if (string.IsNullOrWhiteSpace(ManualIpAddress)) { return; }