From e39513af43a6a8d9f989b42698169cc75df91de8 Mon Sep 17 00:00:00 2001 From: riversn <101815602+riversn@users.noreply.github.com> Date: Thu, 22 Oct 2026 08:49:37 -0400 Subject: [PATCH] Reconnect automatically after the camera drops off USB An unattended sequence could not survive a USB drop: three native errors disabled the driver until N.I.N.A. restarted, the lost handle stayed registered in the native driver (so a later open reused a dead camera), and NINA dropped the camera from the sequence. Together with fixes in the native driver (dougforpres/SonyCamera), the plugin now recovers on its own. SonyDriver (Sony.cs) - Repeated native errors drop the handle instead of stopping the executor; only a hung P/Invoke (executor timeout) still requires a restart. - A lost handle is closed in the native driver exactly once. - ReopenCamera closes the stale handle, opens the camera again and re-attaches the lens. CanReopen / IsAwaitingReopen expose the state. - Close timeout raised from 5 s to 30 s: the native close can wait 20 s for an operation in progress, which was being treated as a hang. CameraDriver - After a failed exposure, a failed status read or a lost handle, the next exposure reopens the camera first, waiting up to 2 minutes for it to come back (retry every second) instead of failing straight away, so a short drop costs one frame rather than the rest of the sequence. Abort or Disconnect ends the wait. Connected stays true while the camera can be reopened. - After a reopen the requested ISO is restored once the camera reports its settings (a reopened camera reads 0 at first, and stepping from that left the wrong ISO), then verified and retried. - An exposure that never finishes times out (2x its length + 2 minutes). - Restarting while the camera still finishes a stopped exposure waits for it instead of failing every remaining frame in milliseconds. - AbortExposure no longer warns "Abort requested" when NINA is only cleaning up after a failed exposure. - Gains returns the same list instance while the ISO options are unchanged, so the Equipment > Camera "Default gain" dropdown no longer goes blank on reconnect. FocuserDriver stays connected while the camera is being reopened. Tests: simulator support for unplugging, reopens and unread ISO after a reopen, plus tests for each behaviour (133 passing). Tested with an ILCE-7M3 and N.I.N.A. 3.2.0.9001: manual and natural USB drops recover without user action; a 7.7-hour run (7,790 exposures) was clean. --- Drivers/CameraDriver.cs | 286 +++++++++++++++++- Drivers/CaptureController.cs | 2 +- Drivers/FocuserDriver.cs | 4 +- Drivers/ISonyCameraBackend.cs | 4 + Drivers/NativeSonyCameraBackend.cs | 2 + Drivers/SimulatedSonyCameraBackend.cs | 42 ++- SonyCameraPluginNative/Sony.cs | 81 ++++- Tests/NativeSonyDriverTests.cs | 63 +++- .../CameraDriverSimulatorTests.cs | 191 +++++++++++- 9 files changed, 640 insertions(+), 35 deletions(-) diff --git a/Drivers/CameraDriver.cs b/Drivers/CameraDriver.cs index fc0eac7..1b55b72 100644 --- a/Drivers/CameraDriver.cs +++ b/Drivers/CameraDriver.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; using System.Text; @@ -38,7 +39,39 @@ public class CameraDriver : BaseINPC, ICamera { private int _lastKnownGain = -1; private int _lastKnownBatteryLevel; - private SonyCameraInfo _camera = null; + private volatile SonyCameraInfo _camera = null; + // Set when an exposure fails or the camera stops answering (e.g. USB unplugged); + // the next exposure reopens the camera first so an unattended sequence can carry on. + private volatile bool _reopenNeeded; + private volatile bool _reopening; + // How long an exposure waits for an unplugged camera to come back before failing. + // Failing at once would burn through the rest of a sequence in seconds. + private readonly TimeSpan _reopenWait; + private static readonly TimeSpan DefaultReopenWait = TimeSpan.FromMinutes(2); + // Each failed attempt (camera not back yet) already takes about a second. + private const int ReopenRetryIntervalMs = 1000; + private const int ReopenWaitPollMs = 250; + private volatile bool _stopReopenWait; + private int _requestedGain = -1; + private const int GainRestoreAttempts = 3; + private const int IsoReadingWaitMs = 5000; + private const int IsoReadingPollMs = 250; + // NINA calls AbortExposure to clean up after a failed exposure too; that is not a + // user abort, and there is no capture left to wait for, so it must not warn. + private volatile bool _exposureFailed; + private volatile bool _starting; + // An exposure that has not finished after twice its length (Sony long-exposure noise + // reduction doubles it) plus this margin is treated as lost, so a camera stuck after a + // USB drop cannot hang the sequence. + private readonly TimeSpan _exposureTimeoutMargin; + private static readonly TimeSpan DefaultExposureTimeoutMargin = TimeSpan.FromMinutes(2); + private double _currentExposureSeconds; + // When the camera is still finishing an earlier exposure (e.g. the sequence was stopped + // and restarted), wait for it instead of failing: failing at once lets a "continue on + // error" sequence run through all of its exposures in a fraction of a second. + // Null: as long as that exposure could still take; zero: do not wait. + private readonly TimeSpan? _busyWaitLimit; + private const int BusyPollMs = 250; private double _lastCameraTemperature = double.NaN; private SonyDevice _device = null; private IProfileService _profileService; @@ -48,7 +81,10 @@ public class CameraDriver : BaseINPC, ICamera { private short _readoutModeForNormalImages; private AsyncObservableCollection _binningModes; - public CameraDriver(IProfileService profileService, IExposureDataFactory exposureDataFactory, SonyDevice device, IPluginOptionsAccessor pluginSettings, ISonyCameraBackend driver = null, Action showWarning = null) { + public CameraDriver(IProfileService profileService, IExposureDataFactory exposureDataFactory, SonyDevice device, IPluginOptionsAccessor pluginSettings, ISonyCameraBackend driver = null, Action showWarning = null, TimeSpan? reopenWait = null, TimeSpan? exposureTimeoutMargin = null, TimeSpan? busyWaitLimit = null) { + _reopenWait = reopenWait ?? DefaultReopenWait; + _exposureTimeoutMargin = exposureTimeoutMargin ?? DefaultExposureTimeoutMargin; + _busyWaitLimit = busyWaitLimit; _profileService = profileService; _exposureDataFactory = exposureDataFactory; _device = device; @@ -71,18 +107,36 @@ private PropertyValue GetPropertyValue(uint id) { return _driver.GetProperty(_camera.Handle, id); } + // NINA's Equipment > Camera "Default gain" dropdown binds straight to Gains. Handing it a + // new list on every read (and on every Gains change notification) made WPF rebuild the + // dropdown and blank the selection, so keep one list per opened camera, and the same list + // instance across a reopen whose ISO options are unchanged. + private SonyCameraInfo _isoOptionsCamera; + private IReadOnlyList _isoOptions = Array.Empty(); + private IList _gains = new List(); + private IReadOnlyList GetAvailableIsoOptions() { - if (_camera == null) { + SonyCameraInfo camera = _camera; + if (camera == null) { return Array.Empty(); } + if (ReferenceEquals(camera, _isoOptionsCamera)) { + return _isoOptions; + } uint[] propertyCandidates = { PROPID_ISOS, PROPID_ISO }; foreach (var propertyId in propertyCandidates) { try { - var options = _camera.GetPropertyInfo(propertyId)?.Options()?.Where(o => o.Value > 0 && o.Value < 0x00FFFFFF) + var options = camera.GetPropertyInfo(propertyId)?.Options()?.Where(o => o.Value > 0 && o.Value < 0x00FFFFFF) .GroupBy(o => o.Value).Select(group => group.First()).OrderBy(o => o.Value).ToList(); if (options != null && options.Count > 0) { + var gains = options.Select(option => (int)option.Value).ToList(); + if (!gains.SequenceEqual(_gains)) { + _gains = gains; + } + _isoOptions = options; + _isoOptionsCamera = camera; return options; } } catch (Exception ex) { @@ -132,10 +186,152 @@ private bool TryGetCaptureStatus(ISonyCameraBackend driver, out uint status, str } catch (Exception ex) { Logger.Warning($"Unable to get capture status ({reason}); camera may not have responded: {ex.Message}."); status = CaptureController.StatusUnknown; + _reopenNeeded = true; return false; } } + // Close the unresponsive handle and open the camera again. + private bool TryReopenCamera(bool firstAttempt) { + bool reopened = false; + lock (connectionLock) { + SonyCameraInfo previous = _camera; + if (previous == null) { + return false; + } + + _reopening = true; + try { + if (firstAttempt) Logger.Warning("The Sony camera stopped responding; reopening it before the next exposure."); + _camera = _driver.ReopenCamera(previous.Handle, _device.Id) + ?? throw new SonyException("Reopening the Sony camera returned no camera."); + _reopenNeeded = false; + reopened = true; + } catch (Exception ex) { + // Expected every few seconds while the camera is unplugged; only log the first. + if (firstAttempt) Logger.Warning($"Unable to reopen the Sony camera yet; waiting for it to come back. {ex.Message}"); + else Logger.Debug($"Sony camera still not reopened: {ex.Message}"); + } finally { + _reopening = false; + } + } + + if (reopened) { + Logger.Info("Sony camera reopened."); + RestoreRequestedGain(); + _showWarning("The Sony camera stopped responding and was reconnected automatically."); + NotifyGainPropertiesChanged(); + } + return reopened; + } + + // Retry the reopen until the camera is back, the wait runs out, or the exposure is aborted. + // A brief USB drop then just delays this exposure instead of failing it. + private bool WaitForReopen() { + _stopReopenWait = false; + var waited = Stopwatch.StartNew(); + for (int attempt = 0; ; attempt++) { + if (TryReopenCamera(firstAttempt: attempt == 0)) { + return true; + } + + var nextAttempt = waited.Elapsed + TimeSpan.FromMilliseconds(ReopenRetryIntervalMs); + while (waited.Elapsed < nextAttempt) { + if (_stopReopenWait || _camera == null || waited.Elapsed >= _reopenWait) { + return false; + } + Thread.Sleep(ReopenWaitPollMs); + } + } + } + + // Returns false if the wait was ended by an abort or disconnect. + private bool WaitWhileCameraBusy() { + // The previous exposure can take at most as long as its own timeout. + TimeSpan limit = _busyWaitLimit ?? + TimeSpan.FromSeconds(Math.Max(0, _currentExposureSeconds) * 2) + _exposureTimeoutMargin; + if (limit <= TimeSpan.Zero) { + return true; + } + + _stopReopenWait = false; + var waited = Stopwatch.StartNew(); + bool logged = false; + while (waited.Elapsed < limit && _camera != null) { + if (_stopReopenWait) { + return false; + } + if (!TryGetCaptureStatus(_driver, out uint status, "waiting for previous exposure") || !CaptureController.IsBusy(status)) { + return true; + } + if (!logged) { + Logger.Info("The Sony camera is still finishing the previous exposure; waiting for it before starting the next one."); + logged = true; + } + Thread.Sleep(BusyPollMs); + } + return true; + } + + // NINA sets the ISO before the exposure, which happens while the camera is still away; + // apply it to the reopened camera. Sony ISO changes are relative steps from the value the + // driver last read, and a just-reopened camera reports 0 until its settings arrive - + // stepping from that left the wrong ISO. So wait for a real reading, then set and verify. + private void RestoreRequestedGain() { + int gain = _requestedGain; + SonyCameraInfo camera = _camera; + if (gain <= 0 || camera == null) { + return; + } + + uint current = 0; + for (int attempt = 0; attempt < GainRestoreAttempts; attempt++) { + current = WaitForIsoReading(camera.Handle); + if (current == (uint)gain) { + Logger.Info($"Restored ISO {gain} on the reopened Sony camera."); + _lastKnownGain = gain; + return; + } + if (current == 0) { + break; + } + try { + _driver.SetProperty(camera.Handle, PROPID_ISO, (uint)gain); + } catch (Exception ex) { + Logger.Warning($"Setting ISO {gain} on the reopened Sony camera failed: {ex.Message}"); + } + } + + current = WaitForIsoReading(camera.Handle); + if (current == (uint)gain) { + Logger.Info($"Restored ISO {gain} on the reopened Sony camera."); + _lastKnownGain = gain; + } else { + Logger.Warning($"Could not restore ISO {gain} on the reopened Sony camera; it reports {(current == 0 ? "no ISO yet" : current.ToString())}."); + } + } + + // Returns the camera's ISO once it reports one (0 = not read yet), or 0 if it does not in time. + private uint WaitForIsoReading(uint handle) { + var waited = Stopwatch.StartNew(); + while (true) { + try { + uint value = GetIsoValue(handle); + if (value != 0) { + return value; + } + } catch (Exception ex) { + Logger.Debug($"Waiting for the reopened Sony camera to report its ISO: {ex.Message}"); + } + if (waited.ElapsedMilliseconds >= IsoReadingWaitMs) { + return 0; + } + Thread.Sleep(IsoReadingPollMs); + } + } + + private uint GetIsoValue(uint handle) => _driver.GetProperty(handle, PROPID_ISO).Value; + #endregion #region Supported Properties @@ -256,13 +452,16 @@ public bool LiveViewEnabled { public int BatteryLevel { get { - if (_camera != null) { + if (_camera != null && !_reopenNeeded) { try { _lastKnownBatteryLevel = (int)GetPropertyValue(PROPID_BATTERY).Value; } catch (Exception ex) { Logger.Warning($"Unable to read Sony battery level: {ex.Message}"); } return _lastKnownBatteryLevel; + } else if (_camera != null) { + // Waiting to reopen the camera; report the last reading rather than an error. + return _lastKnownBatteryLevel; } else { return 0; } @@ -313,6 +512,10 @@ public int GainMin { public int Gain { get { + if (_camera != null && _reopenNeeded) { + // Waiting to reopen the camera; report the last reading rather than an error. + return _lastKnownGain; + } if (_camera != null) { try { PropertyValue value = GetPropertyValue(PROPID_ISO); @@ -332,6 +535,11 @@ public int Gain { if (_camera != null) { try { if (!Gains.Contains(value)) throw new ArgumentOutOfRangeException(nameof(value), "Select a supported manual ISO value."); + _requestedGain = value; + if (_reopenNeeded) { + // Applied once the camera has been reopened (see RestoreRequestedGain). + return; + } _driver.SetProperty(_camera.Handle, PROPID_ISO, (uint)value); RaisePropertyChanged(nameof(Gain)); } catch (Exception ex) { @@ -341,7 +549,7 @@ public int Gain { } } - public IList Gains => GetAvailableIsoOptions().Select(option => (int)option.Value).ToList(); + public IList Gains => GetAvailableIsoOptions().Count > 0 ? _gains : new List(); public string Id => _device.Id == SimulatedSonyCameraBackend.DeviceId ? SimulatedSonyCameraBackend.DeviceId : "Sony"; @@ -360,7 +568,20 @@ public string DisplayName { public bool Connected { get { SonyCameraInfo camera = _camera; - return camera != null && _driver.IsOpen(camera.Handle); + if (camera == null) { + return false; + } + if (_reopening || _driver.IsOpen(camera.Handle)) { + return true; + } + // The handle was lost (e.g. USB unplugged). Stay connected so NINA keeps + // the camera in the sequence; the next exposure reopens it. Only report a + // disconnect once nothing can be reopened (a native call hung). + if (_driver.CanReopen) { + _reopenNeeded = true; + return true; + } + return false; } } @@ -490,6 +711,7 @@ public Task Connect(CancellationToken token) { _lastKnownBatteryLevel = 0; try { _camera = _driver.OpenCamera(_device.Id); + _reopenNeeded = false; } catch (Exception ex) { Logger.Error(ex); _camera = null; @@ -503,6 +725,7 @@ public Task Connect(CancellationToken token) { } public void Disconnect() { + _stopReopenWait = true; lock (connectionLock) { if (_camera == null) return; try { @@ -512,6 +735,7 @@ public void Disconnect() { } _camera = null; + _reopenNeeded = false; Volatile.Write(ref _lastCameraTemperature, double.NaN); } NotifyGainPropertiesChanged(); @@ -553,12 +777,39 @@ public void SetupDialog() { public void StartExposure(CaptureSequence sequence) { if (!Connected) throw new SonyException("Connect a Sony camera before starting an exposure."); + _exposureFailed = true; // until the capture has actually started + _starting = true; + try { + StartExposureCore(sequence); + } finally { + _starting = false; + } + } + + private void StartExposureCore(CaptureSequence sequence) { + if (_reopenNeeded && !WaitForReopen()) { + if (_stopReopenWait) { + throw new CameraExposureFailedException("Exposure cancelled while waiting for the Sony camera to come back."); + } + string message = $"The Sony camera is not responding (it may have been unplugged) and did not come back within {_reopenWait.TotalSeconds:0} s. The next exposure will wait for it again."; + Logger.Warning(message); + throw new CameraExposureFailedException(message); + } if (_camera != null) { + if (!WaitWhileCameraBusy()) { + throw new CameraExposureFailedException("Exposure cancelled while waiting for the camera to finish the previous one."); + } try { + _currentExposureSeconds = sequence.ExposureTime; _capture.Start((float)sequence.ExposureTime); + _exposureFailed = false; } catch (InvalidOperationException ex) { Logger.Warning(ex.Message); throw new CameraExposureFailedException(ex.Message, ex); + } catch (SonyException ex) { + _reopenNeeded = true; + Logger.Warning(ex.Message); + throw new CameraExposureFailedException(ex.Message, ex); } } } @@ -568,9 +819,15 @@ public void StopExposure() { } public void AbortExposure() { + // Also ends a wait for an unplugged camera to come back. + _stopReopenWait = true; if (_camera == null) { return; } + if (!_starting && (_exposureFailed || _reopenNeeded)) { + Logger.Debug("AbortExposure after the exposure had already failed; nothing to cancel."); + return; + } if (_capture.Abort() == CaptureController.AbortResult.SoftCancelRequested) { _showWarning("Abort requested. The camera will finish the current exposure before another can start. Native cancellation is disabled or unavailable."); @@ -579,13 +836,24 @@ public void AbortExposure() { } public async Task WaitUntilExposureIsReady(CancellationToken token) { - using (token.Register(AbortExposure)) { + var limit = TimeSpan.FromSeconds(Math.Max(0, _currentExposureSeconds) * 2) + _exposureTimeoutMargin; + using (token.Register(AbortExposure)) + using (var timeout = CancellationTokenSource.CreateLinkedTokenSource(token)) { + timeout.CancelAfter(limit); try { - await _capture.WaitUntilReady(token); + await _capture.WaitUntilReady(timeout.Token); + } catch (OperationCanceledException) when (timeout.IsCancellationRequested && !token.IsCancellationRequested) { + _exposureFailed = true; + _reopenNeeded = true; + Logger.Error($"The Sony camera did not finish the exposure within {limit.TotalSeconds:0} s; reopening it before the next exposure."); + throw new SonyException("The camera did not finish the exposure in time (see log)"); } catch (OperationCanceledException) { Logger.Info("WaitUntilExposureIsReady cancelled."); throw; } catch (Exception ex) { + // A failed capture is how a pulled cable usually shows up; reopen before the next one. + _exposureFailed = true; + _reopenNeeded = true; Logger.Error("WaitUntilExposureIsReady got exception.", ex); throw new SonyException("Problem while waiting for image to be ready (see log)"); } diff --git a/Drivers/CaptureController.cs b/Drivers/CaptureController.cs index 2ab0f04..33d4980 100644 --- a/Drivers/CaptureController.cs +++ b/Drivers/CaptureController.cs @@ -147,7 +147,7 @@ private static void ThrowIfCancelled(CaptureRequest request) { } } - private static bool IsBusy(uint status) { + internal static bool IsBusy(uint status) { return status == Capturing || status == Starting || status == Reading || status == Processing; } } diff --git a/Drivers/FocuserDriver.cs b/Drivers/FocuserDriver.cs index 558db2b..17d7b09 100644 --- a/Drivers/FocuserDriver.cs +++ b/Drivers/FocuserDriver.cs @@ -178,7 +178,9 @@ public bool Connected { if (_connected) { UInt32 hCamera = GetCameraHandle(); - if (hCamera == 0) { + // While the camera is being reopened after losing USB there is briefly no + // handle; stay connected so focusing works again once it is back. + if (hCamera == 0 && !SonyDriver.GetInstance().IsAwaitingReopen) { Logger.Debug("Sony camera was disconnected without closing focuser first, cleaning up"); IsConnected = false; Moving = false; diff --git a/Drivers/ISonyCameraBackend.cs b/Drivers/ISonyCameraBackend.cs index 2cba108..1552be8 100644 --- a/Drivers/ISonyCameraBackend.cs +++ b/Drivers/ISonyCameraBackend.cs @@ -5,8 +5,12 @@ namespace NINA.RetroKiwi.Plugin.SonyCamera.Drivers { public interface ISonyCameraBackend { IEnumerable Cameras(); SonyCameraInfo OpenCamera(string id); + // Close the handle if still held and open the camera again (e.g. after USB was unplugged). + SonyCameraInfo ReopenCamera(uint handle, string id); void CloseCamera(uint handle); bool IsOpen(uint handle); + // False once the backend can no longer open anything (a native call hung). + bool CanReopen { get; } PropertyValue GetProperty(uint handle, uint propertyId); void SetProperty(uint handle, uint propertyId, uint value); byte[] GetLiveView(uint handle); diff --git a/Drivers/NativeSonyCameraBackend.cs b/Drivers/NativeSonyCameraBackend.cs index c0a7858..69964db 100644 --- a/Drivers/NativeSonyCameraBackend.cs +++ b/Drivers/NativeSonyCameraBackend.cs @@ -7,8 +7,10 @@ internal sealed class NativeSonyCameraBackend : ISonyCameraBackend { public IEnumerable Cameras() => driver.Cameras(); public SonyCameraInfo OpenCamera(string id) => driver.OpenCamera(id); + public SonyCameraInfo ReopenCamera(uint handle, string id) => driver.ReopenCamera(handle, id); public void CloseCamera(uint handle) => driver.CloseCamera(handle); public bool IsOpen(uint handle) => driver.IsOpen(handle); + public bool CanReopen => driver.CanReopen; public PropertyValue GetProperty(uint handle, uint propertyId) => driver.GetProperty(handle, propertyId); public void SetProperty(uint handle, uint propertyId, uint value) => driver.SetProperty(handle, propertyId, value); public byte[] GetLiveView(uint handle) => driver.GetLiveView(handle); diff --git a/Drivers/SimulatedSonyCameraBackend.cs b/Drivers/SimulatedSonyCameraBackend.cs index a104ae5..c74d91a 100644 --- a/Drivers/SimulatedSonyCameraBackend.cs +++ b/Drivers/SimulatedSonyCameraBackend.cs @@ -29,6 +29,14 @@ public sealed class SimulatedSonyCameraBackend : ISonyCameraBackend { public bool FailCapture { get; set; } public bool IgnoreNativeCancel { get; set; } public int StatusReadFailures { get; set; } + // Simulates the USB cable being pulled: the handle still looks open, but + // every camera call fails and the camera cannot be opened until plugged back in. + public bool Unplugged { get; set; } + public int Reopens { get; private set; } + // After a reopen the camera reports ISO 0 for this many reads; setting the ISO in that + // window lands on the wrong value, as the real camera's relative ISO steps do. + public int IsoUnreadReadsAfterReopen { get; set; } + private int isoUnreadReads; public int NativeCancelCalls { get; private set; } public int CaptureStarts { get; private set; } @@ -58,6 +66,9 @@ public SonyCameraInfo OpenCamera(string id) { if (id != DeviceId) { throw new InvalidOperationException("Unknown simulated camera."); } + if (Unplugged) { + throw new InvalidOperationException("Simulated camera is unplugged."); + } if (connected) { throw new InvalidOperationException("Simulated camera is already connected."); } @@ -87,9 +98,28 @@ public bool IsOpen(uint cameraHandle) { lock (cameraLock) return connected && cameraHandle == handle; } + public bool CanReopen => true; + + public SonyCameraInfo ReopenCamera(uint cameraHandle, string id) { + lock (cameraLock) { + Reopens++; + if (connected && cameraHandle == handle) { + connected = false; + captureActive = false; + lastImage = null; + } + SonyCameraInfo camera = OpenCamera(id); + isoUnreadReads = IsoUnreadReadsAfterReopen; + return camera; + } + } + public void CloseCamera(uint cameraHandle) { lock (cameraLock) { - RequireConnection(cameraHandle); + // Like the native driver, closing works even while the camera is unplugged. + if (!connected || cameraHandle != handle) { + throw new InvalidOperationException("Simulated camera is disconnected or the handle is invalid."); + } connected = false; captureActive = false; lastImage = null; @@ -99,6 +129,10 @@ public void CloseCamera(uint cameraHandle) { public PropertyValue GetProperty(uint cameraHandle, uint propertyId) { lock (cameraLock) { RequireConnection(cameraHandle); + if (propertyId == IsoProperty && isoUnreadReads > 0) { + isoUnreadReads--; + return new PropertyValue { Id = propertyId, Value = 0 }; + } return propertyId switch { IsoProperty => new PropertyValue { Id = propertyId, Value = iso }, BatteryProperty => new PropertyValue { Id = propertyId, Value = 100 }, @@ -114,7 +148,8 @@ public void SetProperty(uint cameraHandle, uint propertyId, uint value) { value != 800 && value != 1600 && value != 3200)) { throw new InvalidOperationException("Unsupported simulated ISO value."); } - iso = value; + // Stepping from an unread ISO lands one step off + iso = isoUnreadReads > 0 ? (value == 100 ? 200u : 100u) : value; } } @@ -213,6 +248,9 @@ private void RequireConnection(uint cameraHandle) { if (!connected || cameraHandle != handle) { throw new InvalidOperationException("Simulated camera is disconnected or the handle is invalid."); } + if (Unplugged) { + throw new InvalidOperationException("Simulated camera is unplugged."); + } } } } diff --git a/SonyCameraPluginNative/Sony.cs b/SonyCameraPluginNative/Sony.cs index c095ca2..d717be6 100644 --- a/SonyCameraPluginNative/Sony.cs +++ b/SonyCameraPluginNative/Sony.cs @@ -9,16 +9,25 @@ namespace Sony { // Adapted from @totoantibes's PR #13: camera and focuser share one native // entry point. A timeout permanently blocks further native calls, including // close/open/enumeration, because the old P/Invoke cannot be interrupted. + // Repeated native errors (e.g. USB unplugged) only drop the handle: the + // driver still answers, so the camera can be reopened without a restart. public class SonyDriver { private static readonly Lazy _instance = new Lazy(() => new SonyDriver()); private static readonly NativeCallExecutor sharedCalls = new NativeCallExecutor(); private readonly NativeCallExecutor calls; private readonly SonyMTPCameraDll _sonydll; private volatile SonyCameraInfo _camera; + // Handle dropped after repeated native failures but not yet closed in the + // native driver; it must be closed once so a reopen does not reuse it. + private uint lostHandle; + private string attachedLensPath; private int timeoutNotified; private int consecutiveFailures; private readonly HashSet knownGoodProperties = new HashSet(); private const int LostCameraFailureThreshold = 3; + // The native close waits up to 20 s for an in-progress camera operation. + private const int CloseTimeoutMs = 30000; + private const int OpenTimeoutMs = 60000; public static SonyDriver GetInstance() => _instance.Value; @@ -61,10 +70,12 @@ private T InvokeCamera(uint handle, string operation, Func action, int tim // Count only returned native errors. An unsupported property, // verification mismatch or lock wait is not a lost device. if (++consecutiveFailures >= LostCameraFailureThreshold) { + // The driver answered, so this is not a hang: stop using the + // handle, but leave it to be closed and reopened. + lostHandle = handle; _camera = null; _sonydll.DiscardLastImage(); - calls.Stop(); - Logger.Error($"Sony camera failed {consecutiveFailures} consecutive native operations while trying to {operation}. Treating it as disconnected; restart NINA and power-cycle the camera before reconnecting.", ex); + Logger.Error($"Sony camera failed {consecutiveFailures} consecutive native operations while trying to {operation}. Treating it as disconnected until it is reopened.", ex); } throw; } @@ -84,7 +95,62 @@ public bool IsOpen(uint handle) { return !calls.IsUnavailable && camera != null && camera.Handle == handle; } + // False only once a native call has hung; then nothing can be reopened until NINA restarts. + public bool CanReopen => !calls.IsUnavailable; + public SonyCameraInfo OpenCamera(string path) => Invoke("open camera", () => { + if (lostHandle != 0) ReleaseHandle(lostHandle); + return OpenHandle(path); + }, timeoutMs: OpenTimeoutMs); + + // Close the given handle (if still held, open or lost) and open the camera + // again, restoring the attached lens so the focuser keeps working. + public SonyCameraInfo ReopenCamera(uint handle, string path) => Invoke("reopen camera", () => { + ReleaseHandle(handle); + if (lostHandle != 0) ReleaseHandle(lostHandle); + SonyCameraInfo camera = OpenHandle(path); + if (attachedLensPath != null) { + try { + _sonydll.SetAttachedLens(camera.Handle, attachedLensPath); + } catch (Exception ex) { + Logger.Warning($"Reopened the Sony camera but could not re-attach lens '{attachedLensPath}': {ex.Message}"); + } + } + return camera; + }, timeoutMs: CloseTimeoutMs + OpenTimeoutMs); + + public void CloseCamera(uint handle) { + Invoke("close camera", () => { + // A deliberate close ends any pending reopen, even if the handle was already lost. + attachedLensPath = null; + if (!ReleaseHandle(handle)) { + throw new SonyException("Cannot close camera: the camera handle is no longer open."); + } + return true; + }, timeoutMs: CloseTimeoutMs, lockTimeoutMs: 5000); + } + + // The camera was lost (not closed) with a lens attached and can still be reopened, + // so the focuser should stay connected and pick up the new handle afterwards. + public bool IsAwaitingReopen => CanReopen && _camera == null && attachedLensPath != null; + + // Runs inside Invoke. Closes a handle this driver still holds exactly once; + // returns false if it was never ours or has already been closed. + private bool ReleaseHandle(uint handle) { + if (_camera != null && _camera.Handle == handle) { + _camera = null; + } else if (lostHandle != 0 && lostHandle == handle) { + lostHandle = 0; + } else { + return false; + } + _sonydll.DiscardLastImage(); + _sonydll.CloseDevice(handle); + return true; + } + + // Runs inside Invoke. + private SonyCameraInfo OpenHandle(string path) { if (_camera != null) throw new SonyException("A Sony camera is already open. Disconnect it before opening another camera; restart NINA if the earlier disconnect timed out."); uint handle = _sonydll.OpenDevice(path); if (handle == 0 || handle == SonyMTPCameraDll.INVALID_HANDLE_VALUE) throw new SonyException("Unable to open the Sony camera."); @@ -109,15 +175,6 @@ public SonyCameraInfo OpenCamera(string path) => Invoke("open camera", () => { _sonydll.CloseDevice(handle); throw; } - }, timeoutMs: 60000); - - public void CloseCamera(uint handle) { - InvokeCamera(handle, "close camera", () => { - _camera = null; - _sonydll.DiscardLastImage(); - _sonydll.CloseDevice(handle); - return true; - }, timeoutMs: 5000, lockTimeoutMs: 5000); } public PropertyValue GetProperty(uint handle, uint propertyId) => @@ -163,7 +220,7 @@ public bool TryGetFocusLimit(uint handle, int timeoutMs, out uint limit) { } public void SetAttachedLens(uint handle, string path) => - InvokeCamera(handle, "attach lens", () => { _sonydll.SetAttachedLens(handle, path); return true; }); + InvokeCamera(handle, "attach lens", () => { _sonydll.SetAttachedLens(handle, path); attachedLensPath = path; return true; }); public void SetFocusPosition(uint handle, uint position) => InvokeCamera(handle, "move focus", () => { _sonydll.SetFocusPosition(handle, position); return true; }); public uint GetFocusPosition(uint handle) => diff --git a/Tests/NativeSonyDriverTests.cs b/Tests/NativeSonyDriverTests.cs index 1797b3f..3ac6677 100644 --- a/Tests/NativeSonyDriverTests.cs +++ b/Tests/NativeSonyDriverTests.cs @@ -20,11 +20,14 @@ private sealed class Camera { internal int ExposureCalls; internal int PropertyCalls; internal int CloseCalls; + internal bool OpenFails; + internal readonly List<(uint, string)> AttachedLenses = new List<(uint, string)>(); + private readonly SonyMTPCameraDll dll; internal Camera() { // Skip only the Windows DLL loader. Replace the P/Invoke delegates, // then exercise the actual SonyDriver and SonyMTPCameraDll methods. - var dll = (SonyMTPCameraDll)RuntimeHelpers.GetUninitializedObject(typeof(SonyMTPCameraDll)); + dll = (SonyMTPCameraDll)RuntimeHelpers.GetUninitializedObject(typeof(SonyMTPCameraDll)); SetField(dll, "fGetCaptureStatus", new SonyMTPCameraDll.dGetCaptureStatus((uint h, ref ImageInfo image) => { StatusCalls++; image.Status = 2; @@ -45,6 +48,10 @@ internal Camera() { return PropertyError; })); SetField(dll, "fCloseDevice", new SonyMTPCameraDll.dCloseDevice(h => { CloseCalls++; return 0; })); + SetField(dll, "fSetAttachedLens", new SonyMTPCameraDll.dSetAttachedLens((uint h, string lens) => { + AttachedLenses.Add((h, lens)); + return 0; + })); Driver = new SonyDriver(dll, new NativeCallExecutor()); uint[] shutters = { 0, 0x001E0001, 0x00010001, 0x00040005, 0x00010002, 0x00010050, 0x00010064 }; var properties = new List { @@ -54,6 +61,17 @@ internal Camera() { SetField(Driver, "_camera", new SonyCameraInfo(Handle, new DeviceInfo(), new CameraInfo(), properties)); } + // Stub the native open path: an empty property list and default device/camera info. + internal void UseOpenDevice(uint newHandle) { + SetField(dll, "fOpenDevice", new SonyMTPCameraDll.dOpenDevice(path => OpenFails ? SonyMTPCameraDll.INVALID_HANDLE_VALUE : newHandle)); + SetField(dll, "fGetPropertyList", new SonyMTPCameraDll.dGetPropertyList((uint h, IntPtr list, ref uint size) => { + size = 0; + return list == IntPtr.Zero ? SonyMTPCameraDll.ERROR_RETRY : 0; + })); + SetField(dll, "fGetDeviceInfo", new SonyMTPCameraDll.dGetDeviceInfo((uint h, ref DeviceInfo info) => 0)); + SetField(dll, "fGetCameraInfo", new SonyMTPCameraDll.dGetCameraInfo((uint h, ref CameraInfo info, uint flags) => 0)); + } + private static void SetField(object target, string name, object value) => target.GetType().GetField(name, BindingFlags.Instance | BindingFlags.NonPublic).SetValue(target, value); } @@ -61,7 +79,7 @@ private static void SetField(object target, string name, object value) => [Theory] [InlineData(false)] [InlineData(true)] - public void ThreeNativeCaptureFailuresDisconnectAndBlockEveryLaterNativeEntry(bool starting) { + public void ThreeNativeCaptureFailuresDropTheHandleWhichIsThenClosedExactlyOnce(bool starting) { var camera = new Camera { StatusError = 31, StartError = 1223 }; Action operation = () => { if (starting) camera.Driver.StartCapture(Camera.Handle, 1); @@ -72,14 +90,43 @@ public void ThreeNativeCaptureFailuresDisconnectAndBlockEveryLaterNativeEntry(bo Assert.Equal(i < 2, camera.Driver.IsOpen(Camera.Handle)); } Assert.Null(camera.Driver.CameraInfo); - Assert.Throws(operation); - Assert.Throws(() => camera.Driver.GetProperty(Camera.Handle, 0xD20D)); - Assert.Throws(() => camera.Driver.CloseCamera(Camera.Handle)); - Assert.Throws(() => camera.Driver.OpenCamera("camera")); - Assert.Throws(() => camera.Driver.Cameras()); - Assert.Throws(() => camera.Driver.Lenses()); + // The lost handle no longer reaches the camera, but the driver is still usable. + Assert.True(camera.Driver.CanReopen); + Assert.Throws(operation); + Assert.Throws(() => camera.Driver.GetProperty(Camera.Handle, 0xD20D)); Assert.Equal(3, starting ? camera.StartCalls : camera.StatusCalls); Assert.Equal(0, camera.CloseCalls); + // It is still released in the native driver, once, so a reconnect starts clean. + camera.Driver.CloseCamera(Camera.Handle); + Assert.Equal(1, camera.CloseCalls); + Assert.Throws(() => camera.Driver.CloseCamera(Camera.Handle)); + Assert.Equal(1, camera.CloseCalls); + } + + [Fact] + public void ReopenReleasesTheOldHandleOnceOpensANewOneAndReattachesTheLens() { + var camera = new Camera { StatusError = 31 }; + camera.UseOpenDevice(newHandle: 8); + camera.Driver.SetAttachedLens(Camera.Handle, "lens-1"); + for (int i = 0; i < 3; i++) Assert.Throws(() => camera.Driver.GetCaptureStatus(Camera.Handle)); + Assert.True(camera.Driver.IsAwaitingReopen); + + camera.OpenFails = true; + Assert.Throws(() => camera.Driver.ReopenCamera(Camera.Handle, "camera")); + Assert.Equal(1, camera.CloseCalls); + Assert.True(camera.Driver.IsAwaitingReopen); + + camera.OpenFails = false; + var reopened = camera.Driver.ReopenCamera(Camera.Handle, "camera"); + Assert.Equal(8u, reopened.Handle); + Assert.True(camera.Driver.IsOpen(8)); + Assert.False(camera.Driver.IsAwaitingReopen); + Assert.Equal(1, camera.CloseCalls); + Assert.Equal(new[] { (Camera.Handle, "lens-1"), (8u, "lens-1") }, camera.AttachedLenses); + + camera.Driver.CloseCamera(8); + Assert.Equal(2, camera.CloseCalls); + Assert.False(camera.Driver.IsAwaitingReopen); } [Fact] diff --git a/Tests/PluginIntegration/CameraDriverSimulatorTests.cs b/Tests/PluginIntegration/CameraDriverSimulatorTests.cs index 82a0e08..7afd921 100644 --- a/Tests/PluginIntegration/CameraDriverSimulatorTests.cs +++ b/Tests/PluginIntegration/CameraDriverSimulatorTests.cs @@ -27,7 +27,9 @@ private sealed class Fixture : IDisposable { internal ushort[] DownloadedPixels; internal int Warnings; - internal Fixture() { + // Most tests use no reopen wait: one reopen attempt per exposure, as without the wait. + // Likewise no busy wait by default, so a busy camera is rejected at once. + internal Fixture(TimeSpan? reopenWait = null, TimeSpan? exposureTimeoutMargin = null, TimeSpan? busyWaitLimit = null) { Backend = new SimulatedSonyCameraBackend(Clock); Profile.SetupGet(x => x.ActiveProfile.PluginSettings).Returns(() => ActiveSettings); Options = new PluginOptionsAccessor(Profile.Object, SonyCamera.PluginGuid); @@ -39,7 +41,7 @@ internal Fixture() { new Mock().Object); }); Driver = new CameraDriver(Profile.Object, ExposureFactory.Object, Backend.Cameras().Single(), Options, - Backend, message => Warnings++); + Backend, message => Warnings++, reopenWait ?? TimeSpan.Zero, exposureTimeoutMargin, busyWaitLimit ?? TimeSpan.Zero); } internal void Start(double duration = 1) => Driver.StartExposure(new CaptureSequence { ExposureTime = duration }); @@ -207,6 +209,191 @@ public async Task CaptureAndStatusFailuresAreErrorsRatherThanSuccessfulImages() Assert.Equal(2, test.DownloadedPixels[0]); } + [Fact] + public async Task UnpluggedCameraStaysConnectedAndIsReopenedBeforeTheNextExposure() { + using var test = new Fixture(); + Assert.True(await test.Driver.Connect(CancellationToken.None)); + test.Start(); + test.Backend.Unplugged = true; + test.Finish(); + var lost = await Assert.ThrowsAnyAsync(() => test.Driver.WaitUntilExposureIsReady(CancellationToken.None)); + Assert.False(lost is OperationCanceledException); + Assert.True(test.Driver.Connected); + // NINA cleans up a failed exposure with AbortExposure; that is not a user abort, so no warning. + test.Driver.AbortExposure(); + Assert.Equal(0, test.Warnings); + + // Still unplugged: the exposure fails quickly, NINA keeps the camera, and it is retried next time. + Assert.Throws(() => test.Start()); + Assert.True(test.Driver.Connected); + Assert.Equal(1, test.Backend.Reopens); + Assert.Equal(0, test.Warnings); + + test.Backend.Unplugged = false; + test.Start(); + Assert.Equal(2, test.Backend.Reopens); + Assert.Equal(1, test.Warnings); + test.Finish(); + await test.Driver.WaitUntilExposureIsReady(CancellationToken.None); + await test.Driver.DownloadExposure(CancellationToken.None); + Assert.Equal(2, test.DownloadedPixels[0]); + + // Healthy again: no further reopens. + test.Start(); + test.Finish(); + await test.Driver.WaitUntilExposureIsReady(CancellationToken.None); + Assert.Equal(2, test.Backend.Reopens); + } + + [Fact] + public async Task ExposureWaitsForAnUnpluggedCameraToComeBackInsteadOfFailing() { + using var test = new Fixture(reopenWait: TimeSpan.FromSeconds(30)); + Assert.True(await test.Driver.Connect(CancellationToken.None)); + Assert.Equal(400, test.Driver.Gain); // NINA polls this, so a last-known value exists + test.Backend.Unplugged = true; + // The exposure that discovers the loss fails... + Assert.Throws(() => test.Start()); + Assert.Equal(400, test.Driver.Gain); + test.Driver.Gain = 1600; + + // ...but the next one waits for the camera rather than failing straight away. + var replug = Task.Run(async () => { + await Task.Delay(500); + test.Backend.Unplugged = false; + }); + test.Start(); + await replug; + Assert.True(test.Backend.Reopens >= 2); + Assert.Equal(1, test.Warnings); + Assert.Equal(1600, test.Driver.Gain); + test.Finish(); + await test.Driver.WaitUntilExposureIsReady(CancellationToken.None); + await test.Driver.DownloadExposure(CancellationToken.None); + Assert.Equal(1, test.DownloadedPixels[0]); + } + + [Fact] + public async Task GainListIsTheSameInstanceAcrossReadsReconnectsAndReopens() { + using var test = new Fixture(); + Assert.True(await test.Driver.Connect(CancellationToken.None)); + var gains = test.Driver.Gains; + Assert.Same(gains, test.Driver.Gains); + + // A reopen after losing the camera keeps the list NINA's dropdown is bound to + test.Backend.Unplugged = true; + Assert.Throws(() => test.Start()); + test.Backend.Unplugged = false; + test.Start(); + Assert.Equal(1, test.Backend.Reopens); + Assert.Same(gains, test.Driver.Gains); + + // So does a manual disconnect and connect + test.Driver.Disconnect(); + Assert.Empty(test.Driver.Gains); + Assert.True(await test.Driver.Connect(CancellationToken.None)); + Assert.Same(gains, test.Driver.Gains); + } + + [Fact] + public async Task IsoIsRestoredOnlyOnceTheReopenedCameraReportsItsSettings() { + using var test = new Fixture(); + Assert.True(await test.Driver.Connect(CancellationToken.None)); + Assert.Equal(400, test.Driver.Gain); + test.Backend.Unplugged = true; + Assert.Throws(() => test.Start()); + test.Driver.Gain = 1600; // NINA sets the ISO before the next exposure, while it is away + + test.Backend.Unplugged = false; + test.Backend.IsoUnreadReadsAfterReopen = 3; + test.Start(); + Assert.Equal(1, test.Backend.Reopens); + Assert.Equal(1600, test.Driver.Gain); + } + + [Fact] + public async Task AnExposureThatNeverFinishesTimesOutAndTheCameraIsReopenedBeforeTheNext() { + using var test = new Fixture(exposureTimeoutMargin: TimeSpan.FromMilliseconds(300)); + Assert.True(await test.Driver.Connect(CancellationToken.None)); + // The simulated clock is never advanced, so this exposure never completes. + test.Start(0.01); + var error = await Assert.ThrowsAnyAsync(() => test.Driver.WaitUntilExposureIsReady(CancellationToken.None)); + Assert.False(error is OperationCanceledException); + Assert.Contains("in time", error.Message); + Assert.Equal(0, test.Backend.Reopens); + test.Start(); + Assert.Equal(1, test.Backend.Reopens); + test.Finish(); + await test.Driver.WaitUntilExposureIsReady(CancellationToken.None); + } + + [Fact] + public async Task RestartWhileTheCameraFinishesAStoppedExposureWaitsInsteadOfFailing() { + using var test = new Fixture(busyWaitLimit: TimeSpan.FromSeconds(30)); + Assert.True(await test.Driver.Connect(CancellationToken.None)); + // Sequence stopped mid-exposure: soft abort, the camera keeps exposing. + test.Start(5); + test.Driver.AbortExposure(); + await Assert.ThrowsAsync(() => test.Driver.WaitUntilExposureIsReady(CancellationToken.None)); + + // Restarted straight away: the next exposure waits for the camera instead of failing. + var finish = Task.Run(async () => { + await Task.Delay(500); + test.Finish(6); + }); + test.Start(1); + await finish; + Assert.Equal(2, test.Backend.CaptureStarts); + test.Finish(); + await test.Driver.WaitUntilExposureIsReady(CancellationToken.None); + await test.Driver.DownloadExposure(CancellationToken.None); + Assert.Equal(2, test.DownloadedPixels[0]); + } + + [Fact] + public async Task StoppingWhileWaitingForABusyCameraDoesNotStartAnotherExposure() { + using var test = new Fixture(busyWaitLimit: TimeSpan.FromSeconds(30)); + Assert.True(await test.Driver.Connect(CancellationToken.None)); + test.Start(5); + test.Driver.AbortExposure(); + await Assert.ThrowsAsync(() => test.Driver.WaitUntilExposureIsReady(CancellationToken.None)); + + var waiting = Task.Run(() => Assert.Throws(() => test.Start(1))); + await Task.Delay(300); + test.Driver.AbortExposure(); + await waiting.WaitAsync(TimeSpan.FromSeconds(5)); + Assert.Equal(1, test.Backend.CaptureStarts); + } + + [Fact] + public async Task AbortEndsTheWaitForAnUnpluggedCamera() { + using var test = new Fixture(reopenWait: TimeSpan.FromSeconds(30)); + Assert.True(await test.Driver.Connect(CancellationToken.None)); + test.Backend.Unplugged = true; + Assert.Throws(() => test.Start()); + + var waiting = Task.Run(() => Assert.Throws(() => test.Start())); + await Task.Delay(300); + test.Driver.AbortExposure(); + await waiting.WaitAsync(TimeSpan.FromSeconds(5)); + Assert.True(test.Driver.Connected); + } + + [Fact] + public async Task DisconnectWhileUnpluggedCleansUpAndAManualConnectWorksOnceBack() { + using var test = new Fixture(); + Assert.True(await test.Driver.Connect(CancellationToken.None)); + test.Backend.Unplugged = true; + Assert.Throws(() => test.Start()); + test.Driver.Disconnect(); + Assert.False(test.Driver.Connected); + Assert.False(await test.Driver.Connect(CancellationToken.None)); + test.Backend.Unplugged = false; + Assert.True(await test.Driver.Connect(CancellationToken.None)); + Assert.Equal(0, test.Backend.Reopens); + test.Start(); + Assert.Equal(0, test.Backend.Reopens); + } + [Fact] public async Task ReconnectDiscardsPreviousImageAndSubsamplingUsesReportedSensorSize() { using var test = new Fixture();