From 75bd9d315e85e9f1bb9bbb30dbd99fa5aea1b6f8 Mon Sep 17 00:00:00 2001 From: Lucas Lepski Date: Sat, 17 Oct 2026 14:57:20 +0200 Subject: [PATCH 1/2] Fix capture abort and shutter verification for community preview 1.0.0.6 Preserve aborts through blocked start preflight, verify the nearest learned shutter step, retry unchanged Bulb mode, and stop native access after three qualifying consecutive device errors. Exercise the production native wrappers in portable regression tests and update manual installation and review notes. Restore Robert Hasson's repeated-failure protection from upstream PR #13 with restart-only recovery and exclusions for unsupported properties. Co-authored-by: Robert Hasson --- CHANGELOG.md | 12 ++ Drivers/CaptureController.cs | 46 +++-- INSTALLATION.md | 9 +- Properties/AssemblyInfo.cs | 4 +- README.md | 6 +- .../ExposureTimeController.cs | 66 +++---- SonyCameraPluginNative/NativeCallExecutor.cs | 4 +- SonyCameraPluginNative/Sony.cs | 45 +++-- SonyCameraPluginNative/SonyMTPCameraDll.cs | 33 ++-- Tests/CaptureController.Tests.csproj | 3 + Tests/CaptureControllerTests.cs | 36 ++++ Tests/ExposureTimeControllerTests.cs | 79 ++++++--- Tests/NativeSonyDriverTests.cs | 161 ++++++++++++++++++ Tests/NativeTestLogging.cs | 15 ++ docs/RELEASE-REVIEW-1.0.0.6.md | 24 +++ docs/UPSTREAM-REVIEW-1.0.0.5.md | 2 + 16 files changed, 445 insertions(+), 100 deletions(-) create mode 100644 Tests/NativeSonyDriverTests.cs create mode 100644 Tests/NativeTestLogging.cs create mode 100644 docs/RELEASE-REVIEW-1.0.0.6.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 666334c..16fb43a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Sony Camera Plugin +## 1.0.0.6 — corrective community preview + +Follow-up to the 1.0.0.5 audit, retaining all previously integrated contributions and Doug Henderson's original authorship. Install manually using [INSTALLATION.md](INSTALLATION.md). + +- Preserve aborts received during capture-start preflight. An aborted pending start cannot launch an exposure after its status check returns; a rejected busy start preserves cancellation of the preceding capture. +- Verify the specific nearest learnt shutter step using the native driver's float rounding and absolute-distance selection, replacing the broad ratio that accepted a stalled adjacent speed. +- Retry a valid previous Bulb readback when switching to a short numeric exposure. Malformed readbacks still fail, and a persistent mismatch cannot start capture. +- Restore the repeated-failure camera-loss protection adapted from Robert Hasson's upstream #13: three consecutive qualifying native errors mark the camera disconnected and block all further native access until NINA restarts. Never-working properties, explicit unsupported results, lock contention and shutter-verification mismatches do not count. A successful camera operation resets the count. No native close, retry or automatic reconnection is attempted after the guard trips. +- Add regression coverage using the production capture controller and managed native wrappers, replacing only native delegates and NINA logging/notifications for portable tests. + +See [the corrective review](docs/RELEASE-REVIEW-1.0.0.6.md). Physical camera/USB operation remains unverified; this is a prerelease for manual testing and is not distributed through NINA's plugin manager. + ## 1.0.0.5 — community preview First maintained release from [Lucas Lepski’s fork](https://github.com/shurkanTwo/NINASonyCameraPlugin), preserving Doug Henderson’s original authorship and MPL-2.0. Install manually using [INSTALLATION.md](INSTALLATION.md). This preview is not distributed through NINA’s plugin manager. diff --git a/Drivers/CaptureController.cs b/Drivers/CaptureController.cs index 2e6830a..2ab0f04 100644 --- a/Drivers/CaptureController.cs +++ b/Drivers/CaptureController.cs @@ -18,6 +18,7 @@ internal sealed class CaptureController { internal enum AbortResult { AlreadyRequested, NativeCancelRequested, SoftCancelRequested } private readonly object captureLock = new object(); + private readonly object requestLock = new object(); private readonly Func getStatus; private readonly Action startCapture; private readonly Func cancelCapture; @@ -27,6 +28,7 @@ private sealed class CaptureRequest { internal int CancelRequested; } private CaptureRequest currentRequest = new CaptureRequest(); + private CaptureRequest pendingRequest; internal CaptureController(Func getStatus, Action startCapture, Func cancelCapture, Func nativeCancelEnabled, int cancelLockTimeoutMs = 1000) { @@ -39,26 +41,44 @@ internal CaptureController(Func getStatus, Action startCapture, 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})."); + var request = new CaptureRequest(); + lock (requestLock) { + pendingRequest = request; } + try { + 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); + lock (requestLock) { + Volatile.Write(ref currentRequest, request); + pendingRequest = null; + } + ThrowIfCancelled(request); + // Serialize the native start itself: it can replace the native capture object. + startCapture(exposureTime); + } finally { + lock (requestLock) { + pendingRequest = null; + } + } } } 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; + CaptureRequest request; + lock (requestLock) { + request = pendingRequest ?? currentRequest; + int alreadyRequested = Interlocked.Exchange(ref request.CancelRequested, 1); + // If preflight rejects the new request, the previous capture must + // still retain this abort. No native call runs under requestLock. + Interlocked.Exchange(ref currentRequest.CancelRequested, 1); + if (alreadyRequested != 0) return AbortResult.AlreadyRequested; } if (!nativeCancelEnabled() || !Monitor.TryEnter(captureLock, cancelLockTimeoutMs)) { return AbortResult.SoftCancelRequested; diff --git a/INSTALLATION.md b/INSTALLATION.md index e9bf987..4d2a368 100644 --- a/INSTALLATION.md +++ b/INSTALLATION.md @@ -5,7 +5,7 @@ These instructions install the community-maintained build from The original plugin was developed by **Doug Henderson**. This fork is maintained by **Lucas Lepski and contributors** while shared upstream maintenance is being discussed. -The first community release, **1.0.0.5**, is a preview for manual testing. Windows +The current community release, **1.0.0.6**, is a preview for manual testing. Windows builds and automated tests are checked; physical Sony camera/USB operation has not been verified for this combined release. See the release notes for scope. @@ -17,7 +17,7 @@ not been verified for this combined release. See the release notes for scope. ## Install or replace an existing version -1. Download **`SonyCameraPlugin-1.0.0.5.zip`** from the [1.0.0.5 release](https://github.com/shurkanTwo/NINASonyCameraPlugin/releases/tag/1.0.0.5). Choose the release asset, rather than GitHub's automatic “Source code” archives. Extract it to a temporary folder. The plugin is `NINA.RetroKiwi.Plugin.SonyCamera.dll`. +1. Download **`SonyCameraPlugin-1.0.0.6.zip`** from the [1.0.0.6 release](https://github.com/shurkanTwo/NINASonyCameraPlugin/releases/tag/1.0.0.6). Choose the release asset, rather than GitHub's automatic “Source code” archives. Extract it to a temporary folder. The plugin is `NINA.RetroKiwi.Plugin.SonyCamera.dll`. 2. Disconnect the camera and focuser in NINA, then **close every NINA window** before changing plugin files. 3. Press **Win+R**, enter `%LOCALAPPDATA%\NINA\Plugins`, and open the plugin-version folder used by your NINA installation. NINA 3.x installations commonly use **`3.0.0`**: this folder tracks plugin compatibility, so it need not match the NINA version in About. If several version folders exist, use the one in NINA's startup log (`%LOCALAPPDATA%\NINA\Logs`); look for the path from which the Sony plugin was loaded. 4. If Sony Camera Plugin is already installed, locate its existing `NINA.RetroKiwi.Plugin.SonyCamera.dll` and copy the entire containing plugin folder **outside `%LOCALAPPDATA%\NINA\Plugins`** as a backup. Replace the DLL **in that existing folder**, even if the folder name differs from the example. Do not create another plugin folder containing a second copy. Keep the rest of the existing folder's files. @@ -29,7 +29,7 @@ not been verified for this combined release. See the release notes for scope. 6. Copy the extracted DLL into the existing or newly created folder. The ZIP also contains the license, source reference, and these instructions; you can keep those files alongside it. It contains no replacement NINA runtime libraries or native Sony driver. 7. If Windows shows an **Unblock** checkbox in the downloaded ZIP or DLL's Properties, unblock the file. Alternatively run `Unblock-File` in PowerShell on that downloaded ZIP/DLL before copying/extracting it. -8. Restart NINA. In its plugin manager, confirm **Sony Camera Plugin 1.0.0.5** is loaded; the description identifies the community-maintained build. Select the Sony camera in Equipment and connect it. +8. Restart NINA. In its plugin manager, confirm **Sony Camera Plugin 1.0.0.6** is loaded; the description identifies the community-maintained build. Select the Sony camera in Equipment and connect it. 9. Leave **Enable native cancel** disabled initially. Try one short exposure and check the downloaded image, ISO and exposure before using the build for a sequence. Temperature appears only after downloading an ARW that contains a supported Sony temperature reading. This is a replacement for the upstream plugin, with the same identifier so existing @@ -45,7 +45,7 @@ The release includes **`SHA256SUMS.txt`** for its ZIP and standalone DLL. In PowerShell, calculate a downloaded file's hash and compare it to the matching entry: ```powershell -Get-FileHash "$env:USERPROFILE\Downloads\SonyCameraPlugin-1.0.0.5.zip" -Algorithm SHA256 +Get-FileHash "$env:USERPROFILE\Downloads\SonyCameraPlugin-1.0.0.6.zip" -Algorithm SHA256 ``` ## Roll back @@ -62,6 +62,7 @@ version. The separate Sony driver installation and your images do not need chang - **Missing ISO choices:** use the native driver's `SonyCameraInfo64.exe /s` learning/scanning procedure. This release offers manual ISO choices; AUTO and Multi Frame NR modes are excluded. - **Cannot set the requested exposure:** the shutter change is retried before capture. If it cannot be verified, the exposure fails rather than being silently captured at the previous shutter speed. A camera without learnt shutter settings still uses its manually selected speed; learn its settings before using automatic flats. - **Abort appears to leave the camera busy:** soft cancellation stops NINA's wait while the camera finishes its current exposure. Wait for it to finish before starting another. Reading/processing is always soft-cancelled. +- **Camera marked disconnected after repeated failures:** three consecutive qualifying native errors stop further native access. A property that has never worked and explicit unsupported-property results are excluded. Restart NINA and power-cycle the camera before reconnecting; the plugin does not attempt recovery against the failed handle. - **Driver stopped responding:** close NINA, power-cycle the camera, and restart NINA before reconnecting. The plugin deliberately blocks further native calls after an execution timeout. USB unplugging and power-off while connected can still cause native crashes; disconnect in NINA first. Report problems with this build in [this fork's issue tracker](https://github.com/shurkanTwo/NINASonyCameraPlugin/issues). diff --git a/Properties/AssemblyInfo.cs b/Properties/AssemblyInfo.cs index 280982b..8cde284 100644 --- a/Properties/AssemblyInfo.cs +++ b/Properties/AssemblyInfo.cs @@ -8,8 +8,8 @@ // [MANDATORY] The assembly versioning //Should be incremented for each new release build of a plugin -[assembly: AssemblyVersion("1.0.0.5")] -[assembly: AssemblyFileVersion("1.0.0.5")] +[assembly: AssemblyVersion("1.0.0.6")] +[assembly: AssemblyFileVersion("1.0.0.6")] // [MANDATORY] The name of your plugin [assembly: AssemblyTitle("Sony Camera Plugin")] diff --git a/README.md b/README.md index 9de6965..fab0f11 100644 --- a/README.md +++ b/README.md @@ -25,7 +25,7 @@ Native NINA plugin for Sony mirrorless/DSLR bodies over the Sony MTP interface, ## Installation -Download a maintained build from [this fork’s releases](https://github.com/shurkanTwo/NINASonyCameraPlugin/releases) and follow the **[manual installation instructions](INSTALLATION.md)**. The first community release is a preview for manual testing. It replaces the upstream plugin; install one copy only. +Download a maintained build from [this fork’s releases](https://github.com/shurkanTwo/NINASonyCameraPlugin/releases) and follow the **[manual installation instructions](INSTALLATION.md)**. Community releases are previews for manual testing. It replaces the upstream plugin; install one copy only. The N.I.N.A. plugin manager currently distributes the upstream release. This fork has not been submitted as its update source. @@ -37,8 +37,8 @@ The N.I.N.A. plugin manager currently distributes the upstream release. This for - The driver chooses the nearest learnt built-in shutter speed; durations beyond the longest numeric speed use Bulb. The requested mode is verified before capture. - To keep a copy on the camera's memory card as well as sending it to NINA, use the camera's **PC Remote Settings → Still Img. Save Dest. → PC+Camera** setting (see below). - Powering off/unplugging the camera while connected can crash NINA via Windows' MTP stack (PortableDeviceApi.dll access violation); disconnect in NINA first to avoid it. -- Native driver calls are serialized across camera and lens operations. If a native call stops responding, restart NINA and power-cycle the camera before reconnecting. Automatic USB reconnection is not included. -- Shutter changes are read back and retried before capture. If a readable shutter remains outside supported-step rounding after the retry budget, capture fails instead of silently using the old exposure. Cameras without a learnt shutter list retain their manually selected shutter setting; learn that list before using automatic flats. Sky Flat behavior is not claimed to be resolved. +- Native driver calls are serialized across camera and lens operations. If a native call stops responding, restart NINA and power-cycle the camera before reconnecting. After three consecutive qualifying native errors, the camera is marked disconnected and further native access is also blocked until NINA restarts. Never-working properties and explicit unsupported-property results do not trigger this guard. Automatic USB reconnection is not included. +- Shutter changes are read back and retried before capture. If the camera does not reach the nearest learnt shutter step after the retry budget, capture fails instead of silently using the old exposure. Cameras without a learnt shutter list retain their manually selected shutter setting; learn that list before using automatic flats. Sky Flat behavior is not claimed to be resolved. - “Enable native cancel” is off by default and saved per NINA profile. With it off, abort cancels the wait and discards the exposure, while the camera finishes capturing; another exposure can start once the camera finishes. Enabling it allows native cancellation during aborts before image readout/processing, which may crash NINA on some camera models and Windows drivers. New exposure requests never cancel an existing capture. ## Simulator development diff --git a/SonyCameraPluginNative/ExposureTimeController.cs b/SonyCameraPluginNative/ExposureTimeController.cs index 76a2562..200dcdd 100644 --- a/SonyCameraPluginNative/ExposureTimeController.cs +++ b/SonyCameraPluginNative/ExposureTimeController.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Linq; using System.Threading; namespace Sony { @@ -9,50 +10,57 @@ namespace Sony { internal static class ExposureTimeController { private const uint NotSupported = 50; - internal static double LongestNonBulb(IEnumerable shutterSpeeds) { - double longest = 0; - if (shutterSpeeds != null) { - foreach (uint raw in shutterSpeeds) { - uint numerator = raw >> 16; - uint denominator = raw & 0xFFFF; - if (raw != uint.MaxValue && numerator > 0 && denominator > 0) { - longest = Math.Max(longest, (double)numerator / denominator); - } + private static bool IsNumeric(uint raw) => raw != uint.MaxValue && (raw >> 16) > 0 && (raw & 0xFFFF) > 0; + private static float Seconds(uint raw) => (float)(raw >> 16) / (raw & 0xFFFF); + + internal static uint? SelectShutter(double requested, IEnumerable shutterSpeeds) { + if (!double.IsFinite(requested) || requested < 0 || requested > float.MaxValue) { + throw new ArgumentOutOfRangeException(nameof(requested)); + } + var numeric = shutterSpeeds?.Where(IsNumeric).OrderByDescending(Seconds).ToArray(); + if (numeric == null || numeric.Length == 0) return null; + + // Match SetExposureTime's float arithmetic and five-decimal rounding. + // Equal distances choose the shorter step in the descending ladder. + float target = MathF.Round((float)requested * 100000f, MidpointRounding.AwayFromZero) / 100000f; + if (requested == 0 || target > Seconds(numeric[0])) return 0; + uint nearest = numeric[0]; + float distance = MathF.Abs(target - Seconds(nearest)); + foreach (uint raw in numeric.Skip(1)) { + float candidate = MathF.Abs(target - Seconds(raw)); + if (candidate <= distance) { + nearest = raw; + distance = candidate; } } - return longest > 0 ? longest : 30; + return nearest; } - internal static bool Apply(double requested, Func setExposure, Func readShutter, - int timeoutMs = 12000, Action pause = null, double longestNonBulb = 30) { - if (!double.IsFinite(requested) || requested < 0) throw new ArgumentOutOfRangeException(nameof(requested)); + internal static bool Apply(double requested, IEnumerable shutterSpeeds, + Func setExposure, Func readShutter, int timeoutMs = 12000, Action pause = null) { + uint? expected = SelectShutter(requested, shutterSpeeds); pause ??= () => Thread.Sleep(300); Stopwatch elapsed = Stopwatch.StartNew(); - double actual = double.NaN; - bool requiresBulb = requested == 0 || requested > longestNonBulb; + uint actual = uint.MaxValue; do { uint result = setExposure(); if (result == NotSupported) return false; - if (result != 0) throw new InvalidOperationException($"Unable to set exposure time to {requested}s, error {result}."); + if (result != 0) throw new SonyException($"Unable to set exposure time to {requested}s, error {result}.", result); + if (!expected.HasValue) throw new InvalidOperationException("The learnt shutter-speed list is unavailable; the requested shutter cannot be verified."); - uint raw = readShutter(); - // Sony uses zero for Bulb. Zero-duration captures retain the - // native driver's existing Bulb behavior. - if (raw == 0 && requiresBulb) return true; - uint numerator = raw >> 16; - uint denominator = raw & 0xFFFF; - if (numerator == 0 || denominator == 0 || raw == uint.MaxValue) { + actual = readShutter(); + // Zero is valid Bulb, including a temporarily unchanged old mode. + if (actual == 0 && expected == 0) return true; + if (actual != 0 && !IsNumeric(actual)) { throw new InvalidOperationException("The camera did not report a verifiable shutter speed."); } - actual = (double)numerator / denominator; - // Sony's irregular ladder includes a 4/3 gap. The native - // driver chooses by absolute distance: its worst half-step - // ratio is 7/6, below 1.17 and the smallest whole step (1.20). - if (!requiresBulb && requested > 0 && Math.Max(actual / requested, requested / actual) <= 1.17) return true; + if (actual != 0 && expected != 0 && + (ulong)(actual >> 16) * (expected.Value & 0xFFFF) == + (ulong)(expected.Value >> 16) * (actual & 0xFFFF)) return true; if (elapsed.ElapsedMilliseconds >= timeoutMs) break; pause(); } while (elapsed.ElapsedMilliseconds < timeoutMs); - throw new InvalidOperationException($"Exposure time was not applied: requested {requested}s, camera reports {actual}s. Capture was not started."); + throw new InvalidOperationException($"Exposure time was not applied: requested {requested}s, expected shutter 0x{expected:X8}, camera reports 0x{actual:X8}. Capture was not started."); } } } diff --git a/SonyCameraPluginNative/NativeCallExecutor.cs b/SonyCameraPluginNative/NativeCallExecutor.cs index 5078ae8..bfb760e 100644 --- a/SonyCameraPluginNative/NativeCallExecutor.cs +++ b/SonyCameraPluginNative/NativeCallExecutor.cs @@ -11,6 +11,8 @@ internal sealed class NativeCallExecutor { internal bool IsUnavailable => Volatile.Read(ref unavailable) != 0; + internal void Stop() => Volatile.Write(ref unavailable, 1); + internal T Invoke(Func action, int timeoutMs = 30000, int lockTimeoutMs = 30000) { if (IsUnavailable) throw new InvalidOperationException("The Sony driver stopped responding. Restart NINA and power-cycle the camera before reconnecting."); if (!gate.Wait(lockTimeoutMs)) throw new TimeoutException("The Sony driver is busy with another operation."); @@ -19,7 +21,7 @@ internal T Invoke(Func action, int timeoutMs = 30000, int lockTimeoutMs = Task call = Task.Run(action); try { if (!call.Wait(timeoutMs)) { - Volatile.Write(ref unavailable, 1); + Stop(); _ = call.ContinueWith(failed => { _ = failed.Exception; }, TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously); throw new TimeoutException("The Sony driver stopped responding. Restart NINA and power-cycle the camera before reconnecting."); diff --git a/SonyCameraPluginNative/Sony.cs b/SonyCameraPluginNative/Sony.cs index 4d9812d..f1eb0f1 100644 --- a/SonyCameraPluginNative/Sony.cs +++ b/SonyCameraPluginNative/Sony.cs @@ -11,15 +11,22 @@ namespace Sony { // close/open/enumeration, because the old P/Invoke cannot be interrupted. public class SonyDriver { private static readonly Lazy _instance = new Lazy(() => new SonyDriver()); - private static readonly NativeCallExecutor calls = new NativeCallExecutor(); + private static readonly NativeCallExecutor sharedCalls = new NativeCallExecutor(); + private readonly NativeCallExecutor calls; private readonly SonyMTPCameraDll _sonydll; private volatile SonyCameraInfo _camera; private int timeoutNotified; + private int consecutiveFailures; + private readonly HashSet knownGoodProperties = new HashSet(); + private const int LostCameraFailureThreshold = 3; public static SonyDriver GetInstance() => _instance.Value; - public SonyDriver() { - _sonydll = new SonyMTPCameraDll(); + public SonyDriver() : this(new SonyMTPCameraDll(), sharedCalls) { } + + internal SonyDriver(SonyMTPCameraDll dll, NativeCallExecutor executor) { + _sonydll = dll; + calls = executor; } private T Invoke(string operation, Func action, int timeoutMs = 30000, int lockTimeoutMs = 30000) { @@ -38,12 +45,29 @@ private T Invoke(string operation, Func action, int timeoutMs = 30000, int } } - private T InvokeCamera(uint handle, string operation, Func action, int timeoutMs = 30000, int lockTimeoutMs = 30000) { + private T InvokeCamera(uint handle, string operation, Func action, int timeoutMs = 30000, + int lockTimeoutMs = 30000, uint? propertyId = null) { return Invoke(operation, () => { if (_camera == null || _camera.Handle != handle) { throw new SonyException($"Cannot {operation}: the camera handle is no longer open."); } - return action(); + try { + T result = action(); + consecutiveFailures = 0; + if (propertyId.HasValue) knownGoodProperties.Add(propertyId.Value); + return result; + } catch (SonyException ex) when (ex.ErrorCode.HasValue && ex.ErrorCode != 50 && + (!propertyId.HasValue || knownGoodProperties.Contains(propertyId.Value))) { + // Count only returned native errors. An unsupported property, + // verification mismatch or lock wait is not a lost device. + if (++consecutiveFailures >= LostCameraFailureThreshold) { + _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); + } + throw; + } }, timeoutMs, lockTimeoutMs); } @@ -76,6 +100,8 @@ public SonyCameraInfo OpenCamera(string path) => Invoke("open camera", () => { // Even a late successful open stays inaccessible after timeout: // CameraInfo, IsOpen and every native entry check the sticky gate. _camera = camera; + consecutiveFailures = 0; + knownGoodProperties.Clear(); return camera; } catch { // Cleanup remains inside the same bounded operation. If this @@ -95,18 +121,17 @@ public void CloseCamera(uint handle) { } public PropertyValue GetProperty(uint handle, uint propertyId) => - InvokeCamera(handle, "read camera property", () => _sonydll.GetSinglePropertyValue(handle, propertyId), lockTimeoutMs: 500); + InvokeCamera(handle, "read camera property", () => _sonydll.GetSinglePropertyValue(handle, propertyId), lockTimeoutMs: 500, propertyId: propertyId); public void SetProperty(uint handle, uint propertyId, uint value) => - InvokeCamera(handle, "set camera property", () => { _sonydll.SetPropertyValue(handle, propertyId, value); return true; }); + InvokeCamera(handle, "set camera property", () => { _sonydll.SetPropertyValue(handle, propertyId, value); return true; }, propertyId: propertyId); public byte[] GetLiveView(uint handle) => InvokeCamera(handle, "download LiveView", () => _sonydll.GetPreviewImage(handle)); public uint GetCaptureStatus(uint handle) => InvokeCamera(handle, "read capture status", () => _sonydll.GetCaptureStatus(handle)); public void StartCapture(uint handle, float exposureTime) { InvokeCamera(handle, "start capture", () => { _sonydll.DiscardLastImage(); - double longestNonBulb = ExposureTimeController.LongestNonBulb( - _camera.GetPropertyInfo(0xFFFF)?.Options()?.Select(option => option.Value)); - return _sonydll.StartCapture(handle, exposureTime, longestNonBulb); + var shutterSpeeds = _camera.GetPropertyInfo(0xFFFF)?.Options()?.Select(option => option.Value).ToArray(); + return _sonydll.StartCapture(handle, exposureTime, shutterSpeeds); }, timeoutMs: 60000); } diff --git a/SonyCameraPluginNative/SonyMTPCameraDll.cs b/SonyCameraPluginNative/SonyMTPCameraDll.cs index 05b8b32..f2ac572 100644 --- a/SonyCameraPluginNative/SonyMTPCameraDll.cs +++ b/SonyCameraPluginNative/SonyMTPCameraDll.cs @@ -35,7 +35,8 @@ internal class Kernel32 { #endregion internal class SonyException : Exception { - internal SonyException(string message) : base(message) { } + internal uint? ErrorCode { get; } + internal SonyException(string message, uint? errorCode = null) : base(message) { ErrorCode = errorCode; } } #region DLL Class @@ -265,7 +266,7 @@ public byte[] GetPreviewImage(UInt32 hDevice) { uint hr = fGetPreviewImage(hDevice, ref info); if (hr != ERROR_SUCCESS) { - throw new SonyException($"Unable to get preview image, error {hr}"); + throw new SonyException($"Unable to get preview image, error {hr}", hr); } byte[] returndata = new byte[info.ImageSize]; @@ -285,11 +286,11 @@ public byte[] GetPreviewImage(UInt32 hDevice) { } } - public UInt32 StartCapture(UInt32 hDevice, float exposureTime, double longestNonBulb = 30) { + public UInt32 StartCapture(UInt32 hDevice, float exposureTime, IEnumerable shutterSpeeds = null) { PropertyValue value = new PropertyValue(); - bool applied = ExposureTimeController.Apply(exposureTime, + bool applied = ExposureTimeController.Apply(exposureTime, shutterSpeeds, () => fSetExposureTime(hDevice, exposureTime, ref value), - () => GetSinglePropertyValue(hDevice, 0xD20D).Value, longestNonBulb: longestNonBulb); + () => GetSinglePropertyValue(hDevice, 0xD20D).Value); if (!applied) { Logger.Warning("The camera has no learnt shutter-speed list. Its manually selected shutter speed will be used; run SonyCameraInfo to learn exposure settings before using the Flat Wizard."); } @@ -300,8 +301,9 @@ public UInt32 StartCapture(UInt32 hDevice, float exposureTime, double longestNon }; try { - if (fStartCapture(hDevice, ref info) != ERROR_SUCCESS) { - throw new SonyException("Problem starting capture"); + uint result = fStartCapture(hDevice, ref info); + if (result != ERROR_SUCCESS) { + throw new SonyException($"Problem starting capture, error {result}", result); } return info.Status; @@ -327,8 +329,9 @@ public UInt32 GetCaptureStatus(UInt32 hDevice) { ImageInfo info = new ImageInfo(); try { - if (fGetCaptureStatus(hDevice, ref info) != ERROR_SUCCESS) { - throw new SonyException("Unable to get image status"); + uint result = fGetCaptureStatus(hDevice, ref info); + if (result != ERROR_SUCCESS) { + throw new SonyException($"Unable to get image status, error {result}", result); } // If the capture is complete, copy the image @@ -428,8 +431,9 @@ public List GetPropertyValueOptions(UInt32 hCamera, Propert public PropertyValue GetSinglePropertyValue(UInt32 hCamera, UInt32 propertyId) { PropertyValue option = new PropertyValue(); - if (fGetSinglePropertyValue(hCamera, propertyId, ref option) != ERROR_SUCCESS) { - throw new SonyException("Problem getting property value for " + propertyId.ToString()); + uint result = fGetSinglePropertyValue(hCamera, propertyId, ref option); + if (result != ERROR_SUCCESS) { + throw new SonyException($"Problem getting property value for {propertyId}, error {result}", result); } return option; @@ -446,8 +450,9 @@ public PropertyValue SetExposureTime(UInt32 hCamera, float exposureTime) { } public void SetPropertyValue(UInt32 hCamera, UInt32 propertyId, UInt32 value) { - if (fSetPropertyValue(hCamera, propertyId, value) != ERROR_SUCCESS) { - throw new SonyException("Problem setting property " + propertyId.ToString() + " to " + value.ToString()); + uint result = fSetPropertyValue(hCamera, propertyId, value); + if (result != ERROR_SUCCESS) { + throw new SonyException($"Problem setting property {propertyId} to {value}, error {result}", result); } } @@ -482,7 +487,7 @@ public void SetFocusPosition(UInt32 hCamera, UInt32 focusPosition) { UInt32 hr = fSetFocusPosition(hCamera, ref focusPosition); if (hr != ERROR_SUCCESS) { - throw new SonyException($"Problem setting focus position to {focusPosition}, error {hr}"); + throw new SonyException($"Problem setting focus position to {focusPosition}, error {hr}", hr); } } diff --git a/Tests/CaptureController.Tests.csproj b/Tests/CaptureController.Tests.csproj index 2b733e6..b928110 100644 --- a/Tests/CaptureController.Tests.csproj +++ b/Tests/CaptureController.Tests.csproj @@ -18,6 +18,9 @@ + + + diff --git a/Tests/CaptureControllerTests.cs b/Tests/CaptureControllerTests.cs index b84e653..373e4d0 100644 --- a/Tests/CaptureControllerTests.cs +++ b/Tests/CaptureControllerTests.cs @@ -5,6 +5,42 @@ using Xunit; public class CaptureControllerTests { + [Theory] + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(true, true)] + public async Task AbortDuringPreflightPreventsStartAndSurvivesARejectedStart(bool nativeCancel, bool busy) { + var camera = new Camera { NativeCancelEnabled = nativeCancel }; + if (busy) camera.Capture.Start(1); + using var entered = new ManualResetEventSlim(); + using var release = new ManualResetEventSlim(); + camera.ReadStatus = () => { + entered.Set(); + Assert.True(release.Wait(TimeSpan.FromSeconds(5))); + return busy ? 1u : 2u; + }; + var start = Task.Factory.StartNew(() => Record.Exception(() => camera.Capture.Start(120)), + TaskCreationOptions.LongRunning); + try { + Assert.True(entered.Wait(TimeSpan.FromSeconds(5))); + Assert.Equal(CaptureController.AbortResult.SoftCancelRequested, camera.Capture.Abort()); + } finally { + release.Set(); + } + var error = await start.WaitAsync(TimeSpan.FromSeconds(5)); + if (busy) Assert.IsType(error); + else Assert.IsType(error); + Assert.Equal(busy ? 1 : 0, camera.Starts); + Assert.Equal(0, camera.Cancels); + camera.ReadStatus = null; + camera.Status = 4; + await Assert.ThrowsAsync(() => camera.Capture.WaitUntilReady(CancellationToken.None)); + camera.Capture.Start(1); + camera.Status = 4; + await camera.Capture.WaitUntilReady(CancellationToken.None); + } + [Theory] [InlineData(0x8002u)] [InlineData(0x8003u)] diff --git a/Tests/ExposureTimeControllerTests.cs b/Tests/ExposureTimeControllerTests.cs index c266729..3d413ad 100644 --- a/Tests/ExposureTimeControllerTests.cs +++ b/Tests/ExposureTimeControllerTests.cs @@ -3,66 +3,97 @@ using Xunit; public class ExposureTimeControllerTests { + private static readonly uint[] Shutters = { 0, 0x001E0001, 0x00040001, 0x00010001, + 0x00040005, 0x00030005, 0x00010002, 0x0001003C, 0x00010050, 0x00010064 }; + [Fact] public void BulbBoundaryUsesTheLearnedNumericShutterList() { - double longest = ExposureTimeController.LongestNonBulb(new uint[] { 0, 0x00040001, 0x00010002 }); - Assert.Equal(4, longest); - Assert.True(ExposureTimeController.Apply(5, () => 0, () => 0, longestNonBulb: longest)); + uint[] learned = { 0, 0x00040001, 0x00010002 }; + Assert.True(ExposureTimeController.Apply(5, learned, () => 0, () => 0)); Assert.Throws(() => ExposureTimeController.Apply( - 4.1, () => 0, () => 0x00040001, timeoutMs: 0, longestNonBulb: longest)); + 4.1, learned, () => 0, () => 0x00040001, timeoutMs: 0)); + Assert.Equal(0x00040001u, ExposureTimeController.SelectShutter(4.000001, learned)); } [Fact] public void BulbRequestCannotAcceptAStaleNumericShutter() { Assert.Throws(() => ExposureTimeController.Apply( - 31, () => 0, () => 0x001E0001u, timeoutMs: 0)); + 31, Shutters, () => 0, () => 0x001E0001u, timeoutMs: 0)); } [Fact] public void BackToBackShutterChangeIsRetriedUntilItTakes() { int attempts = 0; - Assert.True(ExposureTimeController.Apply(0.5, () => { attempts++; return 0; }, + Assert.True(ExposureTimeController.Apply(0.5, Shutters, () => { attempts++; return 0; }, () => attempts < 3 ? 0x00010001u : 0x00010002u, pause: () => { })); Assert.Equal(3, attempts); } [Theory] - [InlineData(1.0 / 80, 7.0 / 480)] - [InlineData(0.6, 0.699)] - public void NearestSupportedShutterIsAcceptedAcrossIrregularSteps(double actual, double requested) { - uint raw = actual < 0.1 ? 0x00010050u : 0x0006000Au; - Assert.True(ExposureTimeController.Apply(requested, () => 0, () => raw, timeoutMs: 0)); + [InlineData(0.011439, 0x00010050u)] + [InlineData(0.88, 0x00040005u)] + [InlineData(7.0 / 480, 0x00010050u)] + [InlineData(0.699, 0x00030005u)] + [InlineData(0.9, 0x00040005u)] + public void SelectionMatchesNativeRoundingAndNearestStep(double requested, uint expected) { + Assert.Equal(expected, ExposureTimeController.SelectShutter(requested, Shutters)); + Assert.True(ExposureTimeController.Apply(requested, Shutters, () => 0, () => expected, timeoutMs: 0)); + } + + [Theory] + [InlineData(0.011439, 0x00010064u, 0x00010050u)] + [InlineData(0.88, 0x00010001u, 0x00040005u)] + public void StaleAdjacentShutterIsRetriedDespiteBeingWithinTheOldRatio(double requested, uint stale, uint expected) { + int attempts = 0; + Assert.True(ExposureTimeController.Apply(requested, Shutters, + () => { attempts++; return 0; }, () => attempts == 1 ? stale : expected, pause: () => { })); + Assert.Equal(2, attempts); + Assert.Throws(() => ExposureTimeController.Apply( + requested, Shutters, () => 0, () => stale, timeoutMs: 0)); + } + + [Fact] + public void OldBulbIsRetriedBeforeAShortExposure() { + int attempts = 0; + Assert.True(ExposureTimeController.Apply(0.5, Shutters, () => { attempts++; return 0; }, + () => attempts == 1 ? 0u : 0x00010002u, pause: () => { })); + Assert.Equal(2, attempts); + Assert.Throws(() => ExposureTimeController.Apply( + 0.5, Shutters, () => 0, () => 0, timeoutMs: 0)); } [Fact] - public void StalledShutterFailsInsteadOfExposingAtTheOldSpeed() { - var error = Assert.Throws(() => ExposureTimeController.Apply( - 0.5, () => 0, () => 0x00010001, timeoutMs: 0)); - Assert.Contains("Capture was not started", error.Message); + public void EquivalentRationalReadbackIsAccepted() { + Assert.True(ExposureTimeController.Apply(0.5, Shutters, () => 0, () => 0x00020004u, timeoutMs: 0)); } [Fact] public void APartialMoveDoesNotCountAsSuccessfulSettling() { int attempts = 0; - Assert.Throws(() => ExposureTimeController.Apply(0.01, + Assert.Throws(() => ExposureTimeController.Apply(0.01, Shutters, () => ++attempts == 1 ? 0u : 31u, () => 0x00010002u, pause: () => { })); Assert.Equal(2, attempts); } [Fact] - public void UnsupportedExposureControlIsExplicit() { - Assert.False(ExposureTimeController.Apply(1, () => 50, () => throw new Exception("No readback expected"))); + public void UnsupportedExposureControlRetainsManualFallback() { + Assert.False(ExposureTimeController.Apply(1, null, () => 50, () => throw new Exception("No readback expected"))); + Assert.Throws(() => ExposureTimeController.Apply( + 1, null, () => 0, () => 0x00010001u)); } [Theory] - [InlineData(0u)] [InlineData(0xFFFFFFFFu)] - public void InvalidOrUnexpectedBulbReadbackCannotStartShortExposures(uint raw) { - Assert.Throws(() => ExposureTimeController.Apply(1, () => 0, () => raw)); + [InlineData(0x00010000u)] + [InlineData(0x00000001u)] + public void MalformedNonzeroReadbackCannotStartExposures(uint raw) { + Assert.Throws(() => ExposureTimeController.Apply(1, Shutters, () => 0, () => raw)); } - [Fact] - public void LongBulbExposureIsSupported() { - Assert.True(ExposureTimeController.Apply(120, () => 0, () => 0)); + [Theory] + [InlineData(0)] + [InlineData(120)] + public void BulbExposureIsSupported(double requested) { + Assert.True(ExposureTimeController.Apply(requested, Shutters, () => 0, () => 0)); } } diff --git a/Tests/NativeSonyDriverTests.cs b/Tests/NativeSonyDriverTests.cs new file mode 100644 index 0000000..1797b3f --- /dev/null +++ b/Tests/NativeSonyDriverTests.cs @@ -0,0 +1,161 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Runtime.CompilerServices; +using Sony; +using Xunit; + +public class NativeSonyDriverTests { + private sealed class Camera { + internal const uint Handle = 7; + internal readonly SonyDriver Driver; + internal uint StatusError; + internal uint StartError; + internal uint ExposureError = 50; + internal uint PropertyError; + internal Func Shutter = () => 0x00010002u; + internal int StatusCalls; + internal int StartCalls; + internal int ExposureCalls; + internal int PropertyCalls; + internal int CloseCalls; + + 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)); + SetField(dll, "fGetCaptureStatus", new SonyMTPCameraDll.dGetCaptureStatus((uint h, ref ImageInfo image) => { + StatusCalls++; + image.Status = 2; + return StatusError; + })); + SetField(dll, "fStartCapture", new SonyMTPCameraDll.dStartCapture((uint h, ref ImageInfo image) => { + StartCalls++; + image.Status = 1; + return StartError; + })); + SetField(dll, "fSetExposureTime", new SonyMTPCameraDll.dSetExposureTime((uint h, float seconds, ref PropertyValue value) => { + ExposureCalls++; + return ExposureError; + })); + SetField(dll, "fGetSinglePropertyValue", new SonyMTPCameraDll.dGetSinglePropertyValue((uint h, uint id, ref PropertyValue value) => { + PropertyCalls++; + value.Value = Shutter(); + return PropertyError; + })); + SetField(dll, "fCloseDevice", new SonyMTPCameraDll.dCloseDevice(h => { CloseCalls++; return 0; })); + Driver = new SonyDriver(dll, new NativeCallExecutor()); + uint[] shutters = { 0, 0x001E0001, 0x00010001, 0x00040005, 0x00010002, 0x00010050, 0x00010064 }; + var properties = new List { + new Sony.PropertyInfo(new PropertyDescriptor { Id = 0xFFFF, ValueCount = (uint)shutters.Length }, + shutters.Select(raw => new PropertyValueOption { Value = raw }).ToList()) + }; + SetField(Driver, "_camera", new SonyCameraInfo(Handle, new DeviceInfo(), new CameraInfo(), properties)); + } + + private static void SetField(object target, string name, object value) => + target.GetType().GetField(name, BindingFlags.Instance | BindingFlags.NonPublic).SetValue(target, value); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void ThreeNativeCaptureFailuresDisconnectAndBlockEveryLaterNativeEntry(bool starting) { + var camera = new Camera { StatusError = 31, StartError = 1223 }; + Action operation = () => { + if (starting) camera.Driver.StartCapture(Camera.Handle, 1); + else camera.Driver.GetCaptureStatus(Camera.Handle); + }; + for (int i = 0; i < 3; i++) { + Assert.Throws(operation); + 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()); + Assert.Equal(3, starting ? camera.StartCalls : camera.StatusCalls); + Assert.Equal(0, camera.CloseCalls); + } + + [Fact] + public void ExposureSettingNativeFailuresAlsoTripTheLossGuard() { + var camera = new Camera { ExposureError = 31 }; + for (int i = 0; i < 3; i++) Assert.Throws(() => camera.Driver.StartCapture(Camera.Handle, 1)); + Assert.False(camera.Driver.IsOpen(Camera.Handle)); + Assert.Equal(0, camera.StartCalls); + } + + [Fact] + public void ASuccessfulOperationResetsConsecutiveFailures() { + var camera = new Camera { StatusError = 31 }; + for (int i = 0; i < 2; i++) Assert.Throws(() => camera.Driver.GetCaptureStatus(Camera.Handle)); + camera.StatusError = 0; + Assert.Equal(2u, camera.Driver.GetCaptureStatus(Camera.Handle)); + camera.StatusError = 31; + for (int i = 0; i < 2; i++) Assert.Throws(() => camera.Driver.GetCaptureStatus(Camera.Handle)); + Assert.True(camera.Driver.IsOpen(Camera.Handle)); + Assert.Throws(() => camera.Driver.GetCaptureStatus(Camera.Handle)); + Assert.False(camera.Driver.IsOpen(Camera.Handle)); + } + + [Fact] + public void NeverSupportedPropertiesDoNotDisconnectTheCamera() { + var camera = new Camera { PropertyError = 31 }; + for (int i = 0; i < 10; i++) Assert.Throws(() => camera.Driver.GetProperty(Camera.Handle, 0xD20D)); + Assert.True(camera.Driver.IsOpen(Camera.Handle)); + Assert.Equal(2u, camera.Driver.GetCaptureStatus(Camera.Handle)); + } + + [Fact] + public void FailureOfAPreviouslyWorkingPropertyCountsTowardsLoss() { + var camera = new Camera(); + camera.Driver.GetProperty(Camera.Handle, 0xD20D); + camera.PropertyError = 31; + for (int i = 0; i < 3; i++) Assert.Throws(() => camera.Driver.GetProperty(Camera.Handle, 0xD20D)); + Assert.False(camera.Driver.IsOpen(Camera.Handle)); + Assert.Equal(4, camera.PropertyCalls); + } + + [Fact] + public void ExplicitUnsupportedResultsAreExcludedEvenForAKnownProperty() { + var camera = new Camera(); + camera.Driver.GetProperty(Camera.Handle, 0xD20D); + camera.PropertyError = 50; + for (int i = 0; i < 5; i++) Assert.Throws(() => camera.Driver.GetProperty(Camera.Handle, 0xD20D)); + Assert.True(camera.Driver.IsOpen(Camera.Handle)); + } + + [Fact] + public void InvalidHandlesDoNotCountAsNativeFailures() { + var camera = new Camera(); + for (int i = 0; i < 5; i++) Assert.Throws(() => camera.Driver.GetCaptureStatus(999)); + Assert.True(camera.Driver.IsOpen(Camera.Handle)); + Assert.Equal(0, camera.StatusCalls); + } + + [Theory] + [InlineData(0.011439f, 0x00010064u, 0x00010050u)] + [InlineData(0.88f, 0x00010001u, 0x00040005u)] + [InlineData(0.5f, 0u, 0x00010002u)] + public void NativeStartWaitsForTheExpectedShutterIncludingOldBulb(float requested, uint stale, uint expected) { + var camera = new Camera { ExposureError = 0 }; + camera.Shutter = () => camera.ExposureCalls == 1 ? stale : expected; + camera.Driver.StartCapture(Camera.Handle, requested); + Assert.Equal(2, camera.ExposureCalls); + Assert.Equal(1, camera.StartCalls); + Assert.True(camera.Driver.IsOpen(Camera.Handle)); + } + + [Fact] + public void InvalidShutterReadbackDoesNotStartOrMarkTheCameraLost() { + var camera = new Camera { ExposureError = 0, Shutter = () => uint.MaxValue }; + for (int i = 0; i < 4; i++) Assert.Throws(() => camera.Driver.StartCapture(Camera.Handle, 1)); + Assert.Equal(0, camera.StartCalls); + Assert.True(camera.Driver.IsOpen(Camera.Handle)); + } +} diff --git a/Tests/NativeTestLogging.cs b/Tests/NativeTestLogging.cs new file mode 100644 index 0000000..577117f --- /dev/null +++ b/Tests/NativeTestLogging.cs @@ -0,0 +1,15 @@ +// The portable suite compiles the native wrappers directly. Only NINA logging +// and notifications are replaced; all camera control and error handling is real. +namespace NINA.Core.Utility { + public static class Logger { + public static void Error(string message, System.Exception error) { } + public static void Warning(string message) { } + public static void Info(string message) { } + } +} + +namespace NINA.Core.Utility.Notification { + public static class Notification { + public static void ShowError(string message) { } + } +} diff --git a/docs/RELEASE-REVIEW-1.0.0.6.md b/docs/RELEASE-REVIEW-1.0.0.6.md new file mode 100644 index 0000000..2716efb --- /dev/null +++ b/docs/RELEASE-REVIEW-1.0.0.6.md @@ -0,0 +1,24 @@ +# Corrective review: 1.0.0.6 community preview + +This release addresses all four findings from the follow-up audit of 1.0.0.5. It retains that release's original contributor history and adapted work. Doug Henderson remains the original author; Robert Hasson's PR #13 remains credited as the source of the native stability and shutter retry work. The original MPL-2.0 license is unchanged. + +## Corrections + +1. **Abort during preflight:** a pending capture request is registered before the potentially blocking native status read. Abort cancels that request and the preceding capture's wait. If preflight succeeds, the cancelled pending request cannot start an exposure; if it rejects a busy camera, cancellation of the preceding capture remains intact. A delayed abort for a previous capture still cannot native-cancel a newer capture. +2. **Expected shutter step:** verification selects from the learned numeric shutter list using the native driver's float arithmetic, five-decimal rounding, absolute distance and shorter-step tie behavior. It compares the actual readback against the selected step, accepting equivalent fractions. The old global ratio could accept the wrong adjacent speed. The audit's 0.011439-second and 0.88-second examples now retry until they reach 1/80 and 0.8 seconds respectively. +3. **Old Bulb mode:** zero is a valid Bulb readback, including a temporary previous mode while switching to a numeric shutter. Such a mismatch now retries within the existing 12-second budget. Invalid nonzero encodings still fail immediately. A persistent mismatch fails without starting capture. Explicit native unsupported-exposure control retains the documented manual-shutter fallback; a successful native adjustment without a usable learned list cannot be verified and fails. +4. **Camera-loss guard:** three consecutive qualifying returned native errors stop the shared native executor, hide the camera from camera/focuser connection state, and block subsequent native access, including cleanup, enumeration and reopen, until NINA restarts. Successful camera operations reset the count. Property errors count only after that property has worked; explicit unsupported results are excluded. Managed validation/verification failures and lock waits are excluded. This restores the previously omitted protection from Robert's PR #13 while retaining the stricter recovery policy: no automatic close/retry of a failed handle and no revival of a timed-out worker. + +## Verification + +The portable tests now compile SonyDriver and SonyMTPCameraDll directly. They replace only P/Invoke delegates, logging and notifications; the capture, verification, error classification, failure counting and executor logic under test are production code. + +Regression cases cover blocked preflight with native cancel both enabled and disabled, busy rejection preserving cancellation, stale adjacent shutter steps, temporary and persistent Bulb mode, malformed readback, nearest-step rounding, equivalent fractions, unsupported exposure control, repeated status/start/exposure-setting errors, successful-operation reset, unknown/known property failures, explicit unsupported results, stale handles and blocking native access after loss. + +The Windows workflow runs the portable capture/native-policy suite, ARW temperature suite, full-plugin simulator integration suite and release build before packaging the manual-install DLL/ZIP and SHA256 checksums. SOURCE.txt records the exact source commit and build run. + +These checks do not establish physical Sony camera, USB/MTP, lens, LiveView or complete Sky Flat compatibility. This remains a preview for manual testing. Experimental USB reconnection, retries of still-running timed-out calls, forced cancellation of busy captures and closed WIP #12 video intervals remain deferred. + +## Installation and history + +Use [INSTALLATION.md](../INSTALLATION.md) and install one copy of the maintained plugin. This fork is not an automatic update source in NINA's plugin manager. Version 1.0.0.5 remains available as historical release evidence; its follow-up findings and these corrections are recorded explicitly rather than replacing its assets or tag. diff --git a/docs/UPSTREAM-REVIEW-1.0.0.5.md b/docs/UPSTREAM-REVIEW-1.0.0.5.md index 4808d7e..3d2c9ac 100644 --- a/docs/UPSTREAM-REVIEW-1.0.0.5.md +++ b/docs/UPSTREAM-REVIEW-1.0.0.5.md @@ -1,5 +1,7 @@ # Upstream review for community preview 1.0.0.5 +> Historical review of 1.0.0.5. A subsequent audit found gaps in cancellation, shutter verification and repeated-failure detection. These are corrected in [1.0.0.6](RELEASE-REVIEW-1.0.0.6.md); the original assessment below is retained as release history. + Reviewed against upstream `master`, commit `8bd9042bcd477fd028c2aa2adfe34260192de936` (upstream release 1.0.0.4). This fork preserves Doug Henderson's authorship, copyright notices, MPL-2.0, From 68d635bd9cefd930132a661c82cbc9de2a080e2b Mon Sep 17 00:00:00 2001 From: Lucas Lepski Date: Sat, 17 Oct 2026 14:59:15 +0200 Subject: [PATCH 2/2] Match native rounding of learned shutter candidates --- SonyCameraPluginNative/ExposureTimeController.cs | 5 +++-- Tests/ExposureTimeControllerTests.cs | 7 +++++++ docs/RELEASE-REVIEW-1.0.0.6.md | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/SonyCameraPluginNative/ExposureTimeController.cs b/SonyCameraPluginNative/ExposureTimeController.cs index 200dcdd..8c17b58 100644 --- a/SonyCameraPluginNative/ExposureTimeController.cs +++ b/SonyCameraPluginNative/ExposureTimeController.cs @@ -11,7 +11,8 @@ internal static class ExposureTimeController { private const uint NotSupported = 50; private static bool IsNumeric(uint raw) => raw != uint.MaxValue && (raw >> 16) > 0 && (raw & 0xFFFF) > 0; - private static float Seconds(uint raw) => (float)(raw >> 16) / (raw & 0xFFFF); + private static float NativeRounded(float seconds) => MathF.Round(seconds * 100000f, MidpointRounding.AwayFromZero) / 100000f; + private static float Seconds(uint raw) => NativeRounded((float)(raw >> 16) / (raw & 0xFFFF)); internal static uint? SelectShutter(double requested, IEnumerable shutterSpeeds) { if (!double.IsFinite(requested) || requested < 0 || requested > float.MaxValue) { @@ -22,7 +23,7 @@ internal static class ExposureTimeController { // Match SetExposureTime's float arithmetic and five-decimal rounding. // Equal distances choose the shorter step in the descending ladder. - float target = MathF.Round((float)requested * 100000f, MidpointRounding.AwayFromZero) / 100000f; + float target = NativeRounded((float)requested); if (requested == 0 || target > Seconds(numeric[0])) return 0; uint nearest = numeric[0]; float distance = MathF.Abs(target - Seconds(nearest)); diff --git a/Tests/ExposureTimeControllerTests.cs b/Tests/ExposureTimeControllerTests.cs index 3d413ad..e34735c 100644 --- a/Tests/ExposureTimeControllerTests.cs +++ b/Tests/ExposureTimeControllerTests.cs @@ -6,6 +6,13 @@ public class ExposureTimeControllerTests { private static readonly uint[] Shutters = { 0, 0x001E0001, 0x00040001, 0x00010001, 0x00040005, 0x00030005, 0x00010002, 0x0001003C, 0x00010050, 0x00010064 }; + [Fact] + public void NativeSelectionAlsoRoundsLearnedStepsBeforeComparingDistances() { + uint[] halfStop = { 0, 0x001E0001, 0x000100B4, 0x000100FA }; + Assert.Equal(0x000100FAu, ExposureTimeController.SelectShutter(0.00478f, halfStop)); + Assert.True(ExposureTimeController.Apply(0.00478f, halfStop, () => 0, () => 0x000100FAu, timeoutMs: 0)); + } + [Fact] public void BulbBoundaryUsesTheLearnedNumericShutterList() { uint[] learned = { 0, 0x00040001, 0x00010002 }; diff --git a/docs/RELEASE-REVIEW-1.0.0.6.md b/docs/RELEASE-REVIEW-1.0.0.6.md index 2716efb..4891d93 100644 --- a/docs/RELEASE-REVIEW-1.0.0.6.md +++ b/docs/RELEASE-REVIEW-1.0.0.6.md @@ -5,7 +5,7 @@ This release addresses all four findings from the follow-up audit of 1.0.0.5. It ## Corrections 1. **Abort during preflight:** a pending capture request is registered before the potentially blocking native status read. Abort cancels that request and the preceding capture's wait. If preflight succeeds, the cancelled pending request cannot start an exposure; if it rejects a busy camera, cancellation of the preceding capture remains intact. A delayed abort for a previous capture still cannot native-cancel a newer capture. -2. **Expected shutter step:** verification selects from the learned numeric shutter list using the native driver's float arithmetic, five-decimal rounding, absolute distance and shorter-step tie behavior. It compares the actual readback against the selected step, accepting equivalent fractions. The old global ratio could accept the wrong adjacent speed. The audit's 0.011439-second and 0.88-second examples now retry until they reach 1/80 and 0.8 seconds respectively. +2. **Expected shutter step:** verification selects from the learned numeric shutter list using the native driver's float arithmetic, five-decimal rounding of both requests and learned values, absolute distance and shorter-step tie behavior. It compares the actual readback against the selected step, accepting equivalent fractions. The old global ratio could accept the wrong adjacent speed. The audit's 0.011439-second and 0.88-second examples now retry until they reach 1/80 and 0.8 seconds respectively. 3. **Old Bulb mode:** zero is a valid Bulb readback, including a temporary previous mode while switching to a numeric shutter. Such a mismatch now retries within the existing 12-second budget. Invalid nonzero encodings still fail immediately. A persistent mismatch fails without starting capture. Explicit native unsupported-exposure control retains the documented manual-shutter fallback; a successful native adjustment without a usable learned list cannot be verified and fails. 4. **Camera-loss guard:** three consecutive qualifying returned native errors stop the shared native executor, hide the camera from camera/focuser connection state, and block subsequent native access, including cleanup, enumeration and reopen, until NINA restarts. Successful camera operations reset the count. Property errors count only after that property has worked; explicit unsupported results are excluded. Managed validation/verification failures and lock waits are excluded. This restores the previously omitted protection from Robert's PR #13 while retaining the stricter recovery policy: no automatic close/retry of a failed handle and no revival of a timed-out worker.