add guard for cancel

.github/workflows/dotnet.yml

@@ -34,6 +34,9 @@ jobs:
 
     - name: Restore Packages
       run: nuget restore NINASonyCameraPlugin.sln
+
+    - name: Test capture coordination
+      run: dotnet test Tests/CaptureController.Tests.csproj --configuration Release
 
     - name: Build
       run: msbuild.exe NINASonyCameraPlugin.sln /nologo /nr:false /p:DeployOnBuild=true /p:DeployDefaultTarget=WebPublish /p:WebPublishMethod=FileSystem /p:DeleteExistingFiles=True /p:platform="Any CPU" /p:configuration="Release" /p:PublishUrl="../_build"
@@ -43,4 +46,4 @@ jobs:
       with:
         name: SonyCamera-DLL
         path: |
-          **/bin/Release/**/NINA.RetroKiwi.Plugin.SonyCamera.dll
+          **/bin/Release/**/NINA.RetroKiwi.Plugin.SonyCamera.dll

CHANGELOG.md

@@ -1,4 +1,12 @@
-# Sony Camera Plugin
+# Sony Camera Plugin
+
+## 1.0.0.5
+
+* Added a profile-specific native cancel option, defaulted to off to avoid abort-related crashes.
+* Serialized native capture starts and status/cancel calls; busy or unknown states reject new exposures without cancelling the previous capture.
+* Made aborts idempotent, kept their lock wait bounded, and prevented delayed aborts from cancelling a newer exposure.
+* Reported cancelled and failed captures correctly rather than downloading a stale image.
+* Added automated capture coordination regression tests.
 
 ## 1.0.0.4
 * Added the `UpdateSubSampleArea` implementation and bumped the minimum NINA version so the plugin loads in 3.2.

Drivers/CameraDriver.cs

@@ -29,15 +29,8 @@ public class CameraDriver : BaseINPC, ICamera {
         private const uint PROPID_ISO = 0xD21E; // Actual ISO currently set
         private const uint PROPID_ISOS = 0xFFFE; // Registry-backed list of learnt ISOs (may be empty until learnt)
 
-        // Capture Status
-        private const uint CAPTURE_CREATED    = 0x0000;
-        private const uint CAPTURE_CAPTURING  = 0x0001;
-        private const uint CAPTURE_FAILED     = 0x0002;
-        private const uint CAPTURE_CANCELLED  = 0x0003;
-        private const uint CAPTURE_COMPLETE   = 0x0004;
-        private const uint CAPTURE_STARTING   = 0x8001;
-        private const uint CAPTURE_READING    = 0x8002;
-        private const uint CAPTURE_PROCESSING = 0x8003;
+        private readonly PluginOptionsAccessor _pluginSettings;
+        private readonly CaptureController _capture;
 
         private SonyCameraInfo _camera = null;
         private SonyDevice _device = null;
@@ -48,10 +41,19 @@ public class CameraDriver : BaseINPC, ICamera {
         private short _readoutModeForNormalImages;
         private AsyncObservableCollection<BinningMode> _binningModes;
 
-        public CameraDriver(IProfileService profileService, IExposureDataFactory exposureDataFactory, SonyDevice device) {
+        public CameraDriver(IProfileService profileService, IExposureDataFactory exposureDataFactory, SonyDevice device, PluginOptionsAccessor pluginSettings) {
             _profileService = profileService;
             _exposureDataFactory = exposureDataFactory;
             _device = device;
+            _pluginSettings = pluginSettings;
+            _capture = new CaptureController(
+                () => {
+                    TryGetCaptureStatus(SonyDriver.GetInstance(), out var status, "capture status");
+                    return status;
+                },
+                exposureTime => SonyDriver.GetInstance().StartCapture(_camera.Handle, exposureTime),
+                TryNativeCancel,
+                () => NativeCancelEnabled);
         }
 
         #region Internal Helpers
@@ -91,6 +93,39 @@ private void NotifyGainPropertiesChanged() {
             RaisePropertyChanged(nameof(Gains));
         }
 
+        private bool NativeCancelEnabled {
+            get {
+                try {
+                    var raw = _pluginSettings.GetValueString(nameof(SonyCamera.EnableNativeCancel), bool.FalseString);
+                    return bool.TryParse(raw, out var enabled) && enabled;
+                } catch (Exception ex) {
+                    Logger.Warning($"EnableNativeCancel read failed; defaulting to false. {ex.Message}");
+                    return false;
+                }
+            }
+        }
+
+        private bool TryNativeCancel() {
+            try {
+                SonyDriver.GetInstance().CancelCapture(_camera.Handle);
+                return true;
+            } catch (Exception ex) {
+                Logger.Error("CancelCapture failed; letting capture finish.", ex);
+                return false;
+            }
+        }
+
+        private bool TryGetCaptureStatus(SonyDriver driver, out uint status, string reason) {
+            try {
+                status = driver.GetCaptureStatus(_camera.Handle);
+                return true;
+            } catch (Exception ex) {
+                Logger.Warning($"Unable to get capture status ({reason}); camera may not have responded: {ex.Message}.");
+                status = CaptureController.StatusUnknown;
+                return false;
+            }
+        }
+
         #endregion
 
         #region Supported Properties
@@ -516,19 +551,12 @@ public void SetupDialog() {
 
         public void StartExposure(CaptureSequence sequence) {
             if (_camera != null) {
-                SonyDriver driver = SonyDriver.GetInstance();
-                uint captureStatus = driver.GetCaptureStatus(_camera.Handle);
-
-                if (captureStatus == CAPTURE_CAPTURING || captureStatus == CAPTURE_PROCESSING || captureStatus == CAPTURE_STARTING ||
-                    captureStatus == CAPTURE_READING || captureStatus == CAPTURE_PROCESSING) {
-                    Notification.ShowWarning("Another exposure still in progress. Cancelling it to start another.");
+                try {
+                    _capture.Start((float)sequence.ExposureTime);
+                } catch (InvalidOperationException ex) {
+                    Logger.Warning(ex.Message);
+                    throw new SonyException(ex.Message);
                 }
-
-                // Tell the camera to cancel capture, we do this every time regardless - this will reset the status to be non-complete
-                driver.CancelCapture(_camera.Handle);
-
-                double exposureTime = sequence.ExposureTime;
-                driver.StartCapture(_camera.Handle, (float)exposureTime); //);
             }
         }
 
@@ -537,30 +565,25 @@ public void StopExposure() {
         }
 
         public void AbortExposure() {
-            if (_camera != null) {
-                SonyDriver.GetInstance().CancelCapture(_camera.Handle);
+            if (_camera == null) {
+                return;
+            }
+
+            if (_capture.Abort() == CaptureController.AbortResult.SoftCancelRequested) {
+                Notification.ShowWarning("Abort requested. The camera will finish the current exposure before another can start. Native cancellation is disabled or unavailable.");
+                Logger.Info("AbortExposure requested; native cancel unavailable; letting capture finish.");
             }
         }
 
         public async Task WaitUntilExposureIsReady(CancellationToken token) {
             using (token.Register(AbortExposure)) {
-                uint[] completionStates = { CAPTURE_CANCELLED, CAPTURE_COMPLETE, CAPTURE_FAILED };
-
-                SonyDriver driver = SonyDriver.GetInstance();
-
                 try {
-                    uint captureStatus = driver.GetCaptureStatus(_camera.Handle);
-                    Logger.Info(
-                        $"Waiting for image to be ready, current state is {captureStatus}, completion states are {String.Join(", ", completionStates)}");
-
-                    while (!completionStates.Contains(captureStatus)) {
-                        await CoreUtil.Wait(TimeSpan.FromMilliseconds(100), token);
-                        captureStatus = driver.GetCaptureStatus(_camera.Handle);
-                    }
-
-                    Logger.Info($"Wait for image ready complete, completion state is {captureStatus}");
+                    await _capture.WaitUntilReady(token);
+                } catch (OperationCanceledException) {
+                    Logger.Info("WaitUntilExposureIsReady cancelled.");
+                    throw;
                 } catch (Exception ex) {
-                    Logger.Error("WaitUntilExposureIsReady got exception", ex);
+                    Logger.Error("WaitUntilExposureIsReady got exception.", ex);
                     throw new SonyException("Problem while waiting for image to be ready (see log)");
                 }
             }

Drivers/CameraProvider.cs

@@ -2,6 +2,7 @@
 using NINA.Equipment.Interfaces.ViewModel;
 using NINA.Equipment.Interfaces;
 using NINA.Profile.Interfaces;
+using NINA.Profile;
 using System;
 using System.Collections.Generic;
 using System.Linq;
@@ -24,11 +25,13 @@ public class CameraProvider : IEquipmentProvider<ICamera> {
         private IProfileService profileService;
         private IExposureDataFactory exposureDataFactory;
         SonyDriver driver;
+        private readonly PluginOptionsAccessor pluginSettings;
 
         [ImportingConstructor]
         public CameraProvider(IProfileService profileService, IExposureDataFactory exposureDataFactory) {
             this.profileService = profileService;
             this.exposureDataFactory = exposureDataFactory;
+            this.pluginSettings = new PluginOptionsAccessor(profileService, SonyCamera.PluginGuid);
 
             if (!DllLoader.IsX86()) {
                 try {
@@ -43,14 +46,13 @@ public CameraProvider(IProfileService profileService, IExposureDataFactory expos
 
         public IList<ICamera> GetEquipment() {
             var devices = new List<ICamera>();
-
             if (this.driver != null) {
                 try {
                     int count = 0;
 
                     foreach (var sonyDevice in driver.Cameras()) {
                         count++;
-                        devices.Add(new CameraDriver(profileService, exposureDataFactory, sonyDevice));
+                        devices.Add(new CameraDriver(profileService, exposureDataFactory, sonyDevice, pluginSettings));
                     }
 
                     Logger.Info($"Found {count} Sony Cameras");

Drivers/CaptureController.cs

@@ -0,0 +1,131 @@
+using System;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace NINA.RetroKiwi.Plugin.SonyCamera.Drivers {
+    // Keep capture coordination independent of NINA and the native DLL so races can be tested without a camera.
+    internal sealed class CaptureController {
+        internal const uint StatusUnknown = 0xFFFFFFFF;
+        private const uint Created = 0x0000;
+        private const uint Capturing = 0x0001;
+        private const uint Failed = 0x0002;
+        private const uint Cancelled = 0x0003;
+        private const uint Complete = 0x0004;
+        private const uint Starting = 0x8001;
+        private const uint Reading = 0x8002;
+        private const uint Processing = 0x8003;
+
+        internal enum AbortResult { AlreadyRequested, NativeCancelRequested, SoftCancelRequested }
+
+        private readonly object captureLock = new object();
+        private readonly Func<uint> getStatus;
+        private readonly Action<float> startCapture;
+        private readonly Func<bool> cancelCapture;
+        private readonly Func<bool> nativeCancelEnabled;
+        private readonly int cancelLockTimeoutMs;
+        private sealed class CaptureRequest {
+            internal int CancelRequested;
+        }
+        private CaptureRequest currentRequest = new CaptureRequest();
+
+        internal CaptureController(Func<uint> getStatus, Action<float> startCapture,
+            Func<bool> cancelCapture, Func<bool> nativeCancelEnabled, int cancelLockTimeoutMs = 1000) {
+            this.getStatus = getStatus;
+            this.startCapture = startCapture;
+            this.cancelCapture = cancelCapture;
+            this.nativeCancelEnabled = nativeCancelEnabled;
+            this.cancelLockTimeoutMs = cancelLockTimeoutMs;
+        }
+
+        internal void Start(float exposureTime) {
+            lock (captureLock) {
+                uint status = getStatus();
+                if (IsBusy(status)) {
+                    throw new InvalidOperationException("Cannot start exposure: camera is still busy with a previous exposure.");
+                }
+                if (status != Created && status != Cancelled && status != Complete && status != Failed) {
+                    throw new InvalidOperationException($"Cannot start exposure: capture status unavailable or unexpected ({status}).");
+                }
+
+                // A rejected start must not clear cancellation of the preceding exposure.
+                Volatile.Write(ref currentRequest, new CaptureRequest());
+                // Serialize the native start itself: it can replace the native capture object.
+                startCapture(exposureTime);
+            }
+        }
+
+        internal AbortResult Abort() {
+            // Record cancellation before waiting for the lock, including while StartCapture is blocked.
+            var request = Volatile.Read(ref currentRequest);
+            if (Interlocked.Exchange(ref request.CancelRequested, 1) != 0) {
+                return AbortResult.AlreadyRequested;
+            }
+            if (!nativeCancelEnabled() || !Monitor.TryEnter(captureLock, cancelLockTimeoutMs)) {
+                return AbortResult.SoftCancelRequested;
+            }
+
+            try {
+                // An abort queued for the previous exposure must never cancel a new one.
+                if (!ReferenceEquals(request, currentRequest)) {
+                    return AbortResult.AlreadyRequested;
+                }
+                uint status = getStatus();
+                if (status == StatusUnknown || (!IsBusy(status) && status != Created &&
+                    status != Cancelled && status != Complete && status != Failed)) {
+                    return AbortResult.SoftCancelRequested;
+                }
+                if (!IsBusy(status)) {
+                    return AbortResult.AlreadyRequested;
+                }
+                return cancelCapture() ? AbortResult.NativeCancelRequested : AbortResult.SoftCancelRequested;
+            } finally {
+                Monitor.Exit(captureLock);
+            }
+        }
+
+        internal async Task WaitUntilReady(CancellationToken token) {
+            var request = Volatile.Read(ref currentRequest);
+            while (true) {
+                token.ThrowIfCancellationRequested();
+                ThrowIfCancelled(request);
+                uint status;
+                lock (captureLock) {
+                    if (!ReferenceEquals(request, currentRequest)) {
+                        throw new TaskCanceledException("Exposure superseded by a new capture.");
+                    }
+                    status = getStatus();
+                }
+                token.ThrowIfCancellationRequested();
+                ThrowIfCancelled(request);
+
+                switch (status) {
+                    case Complete:
+                        return;
+                    case Cancelled:
+                        throw new TaskCanceledException("Exposure cancelled by camera.");
+                    case Failed:
+                        throw new InvalidOperationException("Camera capture failed.");
+                    case Created:
+                    case Capturing:
+                    case Starting:
+                    case Reading:
+                    case Processing:
+                        await Task.Delay(100, token);
+                        break;
+                    default:
+                        throw new InvalidOperationException($"Capture status unavailable or unexpected ({status}).");
+                }
+            }
+        }
+
+        private static void ThrowIfCancelled(CaptureRequest request) {
+            if (Volatile.Read(ref request.CancelRequested) != 0) {
+                throw new TaskCanceledException("Exposure cancelled by user; the camera may still be finishing it.");
+            }
+        }
+
+        private static bool IsBusy(uint status) {
+            return status == Capturing || status == Starting || status == Reading || status == Processing;
+        }
+    }
+}

NINASonyCameraPlugin.csproj

@@ -40,6 +40,7 @@
     <PackageReference Include="Zlib.Portable.Signed" Version="1.11.0" />
   </ItemGroup>
   <ItemGroup>
+    <Compile Remove="Tests\**\*.cs" />
     <Compile Remove="SonyCameraPluginNative\obj\Debug\.NETFramework,Version=v4.8.AssemblyAttributes.cs" />
     <Compile Remove="SonyCameraPluginNative\obj\Release\.NETFramework,Version=v4.8.AssemblyAttributes.cs" />
   </ItemGroup>
@@ -68,7 +69,7 @@
     </None>
   </ItemGroup>
   <PropertyGroup />
-  <Target Name="PostBuild" AfterTargets="PostBuildEvent">
+  <Target Name="PostBuild" AfterTargets="PostBuildEvent" Condition="'$(OS)' == 'Windows_NT'">
     <Exec Command="if not exist &quot;%25localappdata%25\NINA\Plugins\3.0.0&quot; (&#xA;  echo &quot;Creating $(PlatformName) Plugins folder&quot;&#xA;  mkdir &quot;%25localappdata%25\NINA\Plugins\3.0.0&quot;&#xA;)&#xA;if not exist &quot;%25localappdata%25\NINA\Plugins\3.0.0\$(TargetName)&quot; (&#xA;  echo &quot;Creating $(PlatformName) Plugins $(TargetName) folder&quot;&#xA;  mkdir &quot;%25localappdata%25\NINA\Plugins\3.0.0\$(TargetName)&quot;&#xA;)&#xA;&#xA;echo &quot;Copying $(PlatformName) $(TargetFileName)&quot;&#xA;xcopy &quot;$(TargetPath)&quot; &quot;%25localappdata%25\NINA\Plugins\3.0.0\$(TargetName)&quot; /h/i/c/k/e/r/y" />
   </Target>
 </Project>

Options.xaml

@@ -6,15 +6,12 @@
     <!--  In order for this datatemplate to be picked correctly, the key has to follow the naming convention of <IPlugin.Name>_Options -->
     <!--  Furthermore the Resource Dictionary has to be exported via code behind export attributes  -->
     <DataTemplate x:Key="Sony Camera Plugin_Options">
-        <!--        <StackPanel Orientation="Vertical"> -->
-    <!--            <StackPanel Orientation="Horizontal"> -->
-    <!--                <TextBlock Text="Default Notification Message" /> -->
-    <!--                <TextBox MinWidth="50" Text="{Binding DefaultNotificationMessage}" /> -->
-    <!--            </StackPanel> -->
-    <!--            <StackPanel Orientation="Horizontal"> -->
-    <!--                <TextBlock Text="Profile Specific Notification Message" /> -->
-    <!--                <TextBox MinWidth="50" Text="{Binding ProfileSpecificNotificationMessage}" /> -->
-    <!--            </StackPanel> -->
-    <!--        </StackPanel> -->
+        <StackPanel Orientation="Vertical" Margin="0,4,0,0">
+            <TextBlock Text="Camera native capture cancellation" FontSize="14" FontWeight="SemiBold" Margin="0,0,0,4" />
+            <TextBlock Text="Allow abort requests to stop an exposure immediately. Some camera models and Windows drivers may crash NINA when this is enabled. Disabled by default; cancelled exposures finish on the camera before a new exposure can start." TextWrapping="Wrap" Opacity="0.7" Margin="0,0,0,8"/>
+            <CheckBox Content="Enable native cancel"
+                      HorizontalAlignment="Left"
+                      IsChecked="{Binding EnableNativeCancel, Mode=TwoWay}" />
+        </StackPanel>
     </DataTemplate>
-</ResourceDictionary>
+</ResourceDictionary>