diff --git a/docs/release-notes/v1.9.2.md b/docs/release-notes/v1.9.2.md new file mode 100644 index 0000000..0c72e50 --- /dev/null +++ b/docs/release-notes/v1.9.2.md @@ -0,0 +1,12 @@ +# Gloam 1.9.2 + +Calibration no longer fails at instrument startup with "Failed to locate display type matching 'o'" (issue #7). + +- The spotread display-type selector (`-y`) is now taken from the table spotread prints for the connected instrument instead of a fixed letter per display type. ArgyllCMS only lists the technology letters (`o` OLED, `e` white LED, `b` RGB LED, `m` plasma, `p` projector) when matching X-Rite EDR corrections have been imported, which a fresh install never has. On a clean install the i1 Display Pro family accepts only `n|l` (Non-Refresh), `r|c` (Refresh) and a built-in CCFL row, so OLED, LCD LED and wide-gamut LCD all failed there. +- OLED, LCD LED and wide-gamut LCD now resolve to the instrument's Non-Refresh base calibration when no matching correction is installed; CRT, plasma and projector resolve to the Refresh base. When a matching EDR row exists, the same letters as before are used. One measured-output change: on a clean i1D3, CCFL LCD now uses the built-in "LCD CCFL PVA" spectral row (`-y 1`) instead of the Non-Refresh base it reached through `-y l` before, because it is the only CCFL row listed. If you calibrated a CCFL display with 1.9.1, re-run the calibration. When a `.ccss` meter correction is selected, it continues to supply both the spectral calibration and the refresh mode, as spotread already did. +- If spotread still rejects the selector, Gloam reads the table spotread prints on the way out and restarts once with a selector from that table. The failure message now lists the selectors the instrument accepts. +- With two meters connected, only the rows spotread printed for the meter Gloam opened are considered. +- The calibration setup screen now shows which calibration row the meter will use for the selected display type, and warns when the meter has no correction for that type and no CCSS/CCMX file is chosen. +- If X-Rite i1Profiler or Calibrite PROFILER is installed, Gloam now converts their i1Display EDR calibration files into ArgyllCMS corrections with Argyll's oeminst the first time it looks for the meter. This is what gives the meter its OLED, white LED and RGB LED calibration rows. Nothing is downloaded and nothing happens on PCs without that software. + +Validation: all 1,388 tests pass and the strict Release build has zero warnings or errors. The rejection and retry path was exercised against an i1 Display Pro on Argyll 3.5.0 by forcing an invalid selector; the retry restarted with a listed selector and reached the ready prompt. The reporter's clean-install table and a table with imported EDR rows are pinned as test fixtures. diff --git a/src/Gloam.Core/Calibration/BoundedProcess.cs b/src/Gloam.Core/Calibration/BoundedProcess.cs new file mode 100644 index 0000000..54ec103 --- /dev/null +++ b/src/Gloam.Core/Calibration/BoundedProcess.cs @@ -0,0 +1,74 @@ +using System; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; + +namespace Gloam.Core.Calibration +{ + /// Result of . + public sealed class BoundedProcessResult + { + public BoundedProcessResult(int exitCode, string stdout, string stderr) + { + ExitCode = exitCode; + Stdout = stdout; + Stderr = stderr; + } + + public int ExitCode { get; } + public string Stdout { get; } + public string Stderr { get; } + + /// stdout followed by stderr, the shape the Argyll output parsers expect. + public string Combined => Stdout + "\n" + Stderr; + } + + /// + /// Runs a console tool to completion with both streams captured and a hard time limit. + /// Shared by the Argyll one-shot invocations (spotread -?, oeminst). + /// + public static class BoundedProcess + { + /// + /// Starts and waits for exit. Throws + /// after killing the process tree when + /// elapses or fires. + /// + public static async Task RunAsync( + ProcessStartInfo psi, TimeSpan timeout, CancellationToken cancellationToken) + { + psi.UseShellExecute = false; + psi.CreateNoWindow = true; + psi.RedirectStandardOutput = true; + psi.RedirectStandardError = true; + + using var process = Process.Start(psi) + ?? throw new InvalidOperationException($"{psi.FileName} did not start."); + using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + cts.CancelAfter(timeout); + + var stdout = process.StandardOutput.ReadToEndAsync(cts.Token); + var stderr = process.StandardError.ReadToEndAsync(cts.Token); + try + { + await process.WaitForExitAsync(cts.Token); + return new BoundedProcessResult(process.ExitCode, await stdout, await stderr); + } + catch (OperationCanceledException) + { + try + { + process.Kill(entireProcessTree: true); + } + catch (Exception ex) + { + Log.DebugRateLimited( + "bounded-process-termination", + $"Could not terminate the timed-out process {psi.FileName}: {ex.Message}", + TimeSpan.FromMinutes(10)); + } + throw; + } + } + } +} diff --git a/src/Gloam.Core/Calibration/ColorimeterService.cs b/src/Gloam.Core/Calibration/ColorimeterService.cs index 0aaac50..993f1be 100644 --- a/src/Gloam.Core/Calibration/ColorimeterService.cs +++ b/src/Gloam.Core/Calibration/ColorimeterService.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Diagnostics; using System.Globalization; using System.IO; @@ -190,6 +191,12 @@ public async Task InitializeAsync(CancellationToken cancellationToken = de RaiseStatusChanged(ColorimeterStatus.Searching, "Searching for colorimeter..."); + // Convert any i1Display EDR files from installed X-Rite/Calibrite software that + // Argyll does not have yet, so detection below sees the technology rows. + // No-op after the first run and on machines without that software. + string argyllBin = Path.GetDirectoryName(_spotreadPath) ?? _argyllBinPath; + await OemCorrectionImporter.EnsureImportedAsync(argyllBin, Log, cancellationToken); + // Detect connected colorimeter _connectedColorimeter = await DetectColorimeterAsync(cancellationToken); @@ -248,9 +255,19 @@ public void SetDisplayIndex(int index) public void SetDisplayType(DisplayType type) { _displayType = type; - Log($"Display type set to: {type} (flag: -{type.ToSpotreadFlag()})"); + var choice = ResolveDisplayTypeChoice(type); + Log($"Display type set to: {type} (spotread -y {choice.Selector}: {choice.Reason})"); } + /// + /// How the connected instrument will measure : the selector + /// spotread gets and the table row it comes from. Uses the table captured at + /// detection (or adopted from a session), so it is exact for colorimeters with an + /// enumerated table and a best guess otherwise. + /// + public SpotreadDisplayTypeChoice ResolveDisplayTypeChoice(DisplayType type) + => SpotreadDisplayTypeTable.Resolve(type, _connectedColorimeter?.DisplayTypes); + /// /// Gets the current display type setting. /// @@ -348,9 +365,12 @@ public virtual async Task BeginMeasurementSessionAsync(bool hdrMode, Cancellatio Log($"Opening persistent spotread session (instrument {instrumentIndex}, HDR={hdrMode})"); try { + await RefreshDisplayTypeTableIfUnknownAsync(cancellationToken); _session = await SpotreadSession.StartAsync( _spotreadPath, instrumentIndex, _displayType, hdrMode, Log, cancellationToken, - _correctionFilePath); + _correctionFilePath, displayTypeTable: _connectedColorimeter?.DisplayTypes, + instrumentDescriptor: _connectedColorimeter?.InstrumentDescriptor); + AdoptDisplayTypeTable(_session.EffectiveDisplayTypeTable); _sessionHdrMode = hdrMode; RaiseStatusChanged(ColorimeterStatus.Ready, "Spotread session ready"); } @@ -367,6 +387,52 @@ public virtual async Task BeginMeasurementSessionAsync(bool hdrMode, Cancellatio } } + /// + /// Keeps the table a session actually resolved against (after a rejection retry it is + /// the one spotread printed for the opened instrument) so later sessions start right. + /// + private void AdoptDisplayTypeTable(IReadOnlyList table) + { + if (_connectedColorimeter == null || !SpotreadDisplayTypeTable.HasInstrumentRows(table)) + return; + if (ReferenceEquals(_connectedColorimeter.DisplayTypes, table)) + return; + _connectedColorimeter.DisplayTypes = table; + Log($"spotread -y table updated from session: {SpotreadDisplayTypeTable.Describe(table)}"); + } + + // The usage text lists the -y selectors the enumerated instrument accepts. Argyll + // builds that table per instrument and per installed correction, so it is the only + // reliable source for the display-type flag (issue #7). + private static IReadOnlyList ScopedDisplayTypeTable(string? usage, string? descriptor) + { + var table = SpotreadDisplayTypeTable.ForInstrument(SpotreadDisplayTypeTable.Parse(usage), descriptor); + Log($"spotread -y table: {SpotreadDisplayTypeTable.Describe(table)}"); + return table; + } + + private bool _displayTypeProbeRetried; + + /// + /// Detection only sees the -y table when the instrument is enumerated at that moment + /// (the meter may be plugged in after the app starts). If the cached table has no + /// instrument rows, re-run the usage probe once per service before opening a session. + /// + private async Task RefreshDisplayTypeTableIfUnknownAsync(CancellationToken cancellationToken) + { + if (_connectedColorimeter == null || _displayTypeProbeRetried + || SpotreadDisplayTypeTable.HasInstrumentRows(_connectedColorimeter.DisplayTypes)) + return; + var usage = await RunSpotreadCommandAsync(TimeSpan.FromSeconds(10), cancellationToken, "-?"); + // An interrupted probe (caller cancelled) has not used the one retry. + _displayTypeProbeRetried = !cancellationToken.IsCancellationRequested; + var table = ScopedDisplayTypeTable(usage, _connectedColorimeter.InstrumentDescriptor); + if (SpotreadDisplayTypeTable.HasInstrumentRows(table)) + _connectedColorimeter.DisplayTypes = table; + else + Log("spotread -y table still unknown (no instrument enumerated by -?); using the base selector and the rejection retry"); + } + /// /// Closes the persistent spotread session. Safe to call if no session is active. /// @@ -540,9 +606,12 @@ private async Task TakeMeasurementAsync(bool hdrMode, CancellationToken SpotreadSession transient; try { + await RefreshDisplayTypeTableIfUnknownAsync(cancellationToken); transient = await SpotreadSession.StartAsync( _spotreadPath, instrumentIndex, _displayType, hdrMode, Log, cancellationToken, - _correctionFilePath); + _correctionFilePath, displayTypeTable: _connectedColorimeter?.DisplayTypes, + instrumentDescriptor: _connectedColorimeter?.InstrumentDescriptor); + AdoptDisplayTypeTable(transient.EffectiveDisplayTypeTable); } catch (InvalidOperationException ex) when (UsbDriverHelper.IsDriverError(ex.Message)) { @@ -567,14 +636,17 @@ private async Task TakeMeasurementAsync(bool hdrMode, CancellationToken // Use help output to get the instrument list (spotread doesn't support -l) var listResult = await RunSpotreadCommandAsync(TimeSpan.FromSeconds(10), cancellationToken, "-?"); + IReadOnlyList displayTypes = Array.Empty(); if (listResult != null) { Log($"spotread -? output: {listResult}"); // Parse actual connected device from list var deviceInfo = ParseDeviceListOutput(listResult); + displayTypes = ScopedDisplayTypeTable(listResult, deviceInfo?.InstrumentDescriptor); if (deviceInfo != null) { + deviceInfo.DisplayTypes = displayTypes; Log($"Detected device from -?: {deviceInfo.Model}"); return deviceInfo; } @@ -589,7 +661,8 @@ private async Task TakeMeasurementAsync(bool hdrMode, CancellationToken return new ColorimeterInfo { Model = "Colorimeter Detected", - IsHdrCapable = true // Assume capable, actual capability tested during measurement + IsHdrCapable = true, // Assume capable, actual capability tested during measurement + DisplayTypes = displayTypes }; } @@ -605,65 +678,22 @@ private async Task TakeMeasurementAsync(bool hdrMode, CancellationToken if (string.IsNullOrEmpty(_spotreadPath)) return null; - var psi = new ProcessStartInfo(_spotreadPath) - { - UseShellExecute = false, - CreateNoWindow = true, - RedirectStandardOutput = true, - RedirectStandardError = true - }; + var psi = new ProcessStartInfo(_spotreadPath); foreach (string arg in args) - { psi.ArgumentList.Add(arg); - } try { - using var process = Process.Start(psi); - if (process == null) return null; - - using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - cts.CancelAfter(timeout); - - var stdoutTask = process.StandardOutput.ReadToEndAsync(cts.Token); - var stderrTask = process.StandardError.ReadToEndAsync(cts.Token); - - try - { - await process.WaitForExitAsync(cts.Token); - } - catch (OperationCanceledException) - { - try - { - process.Kill(entireProcessTree: true); - } - catch (Exception ex) - { - Gloam.Core.Log.DebugRateLimited( - "colorimeter-process-termination", - $"Could not terminate the cancelled instrument process: {ex.Message}", - TimeSpan.FromMinutes(10)); - } - return null; - } - - string output, error; - try - { - output = await stdoutTask; - error = await stderrTask; - } - catch (OperationCanceledException) - { - return null; - } - - return output + "\n" + error; + var result = await BoundedProcess.RunAsync(psi, timeout, cancellationToken); + return result.Combined; + } + catch (OperationCanceledException) + { + return null; } catch (Exception ex) { - Log($"Error running spotread {args}: {ex.Message}"); + Log($"Error running spotread {string.Join(" ", args)}: {ex.Message}"); return null; } } @@ -671,7 +701,9 @@ private async Task TakeMeasurementAsync(bool hdrMode, CancellationToken /// /// Parses device information from spotread output. /// - private static ColorimeterInfo? ParseDeviceListOutput(string output) + private static readonly Regex BareSerialPortPattern = new(@"^(COM\d+|/dev/tty\S*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase); + + internal static ColorimeterInfo? ParseDeviceListOutput(string output) { // Common colorimeter patterns in ArgyllCMS output var devicePatterns = new[] @@ -716,9 +748,13 @@ private async Task TakeMeasurementAsync(bool hdrMode, CancellationToken } } - if (listMatches.Count > 0) + // A bare serial port ("COM6") is what every PC with a COM header lists; it is not + // an instrument. Argyll cannot name a serial instrument without opening it. + foreach (Match entry in listMatches) { - var firstEntry = listMatches[0]; + var firstEntry = entry; + if (BareSerialPortPattern.IsMatch(firstEntry.Groups[2].Value.Trim())) + continue; if (int.TryParse(firstEntry.Groups[1].Value, NumberStyles.Integer, CultureInfo.InvariantCulture, out int index)) { string descriptor = firstEntry.Groups[2].Value; @@ -848,6 +884,13 @@ public class ColorimeterInfo /// public string? InstrumentDescriptor { get; init; } + /// + /// The -y display-type table spotread printed for the enumerated instrument(s) + /// at detection time. Empty when no instrument was enumerated. Settable because it is + /// attached after the descriptor parse that constructs this object. + /// + public IReadOnlyList DisplayTypes { get; set; } = Array.Empty(); + /// /// Whether this colorimeter supports HDR measurement modes. /// diff --git a/src/Gloam.Core/Calibration/DisplayType.cs b/src/Gloam.Core/Calibration/DisplayType.cs index cbd4b8b..620cf81 100644 --- a/src/Gloam.Core/Calibration/DisplayType.cs +++ b/src/Gloam.Core/Calibration/DisplayType.cs @@ -2,7 +2,8 @@ namespace Gloam.Core.Calibration { /// /// Display technology types for colorimeter measurement configuration. - /// These map to ArgyllCMS spotread -y flag values. + /// Resolved to an ArgyllCMS spotread -y selector per instrument by + /// . /// public enum DisplayType { @@ -34,30 +35,22 @@ public enum DisplayType public static class DisplayTypeExtensions { /// - /// Gets the ArgyllCMS spotread -y flag value for this display type. + /// True for technologies Argyll measures in refresh mode (CRT, plasma, DLP). OLED and + /// every LCD are non-refresh, matching Argyll's disptechs.c table. /// /// - /// CAVEAT: these letter codes are NOT a stable ArgyllCMS contract. The -y table is - /// generated per instrument and per Argyll version — spotread prints the letters - /// that are valid for the CONNECTED probe in its usage output, and the same letter - /// can mean different things for different instruments (or be absent entirely). - /// The mapping below matches Argyll V2.2+ with an i1 Display Pro/Plus class - /// colorimeter, which is the hardware this app targets. Parsing the actual -y - /// table from `spotread -?` output for the connected instrument is future work; - /// until then, an unknown letter degrades to a wrong generic correction rather - /// than a hard failure (spotread reports "unrecognised" and exits, which surfaces - /// through the session's fatal-error path). + /// The spotread -y selector for a display type is NOT a fixed letter: Argyll + /// builds the table per instrument at run time and the technology letters only exist + /// when matching corrections are installed. Use + /// with the table parsed from the + /// instrument's usage output. /// - public static string ToSpotreadFlag(this DisplayType type) => type switch + public static bool IsRefreshTechnology(this DisplayType type) => type switch { - DisplayType.LcdLed => "e", // LCD with white LED backlight - DisplayType.Oled => "o", // OLED (since Argyll V2.2.0) - DisplayType.LcdWideGamut => "b", // LCD with wide gamut LED backlight - DisplayType.LcdCcfl => "l", // LCD with CCFL backlight - DisplayType.Crt => "c", // CRT - DisplayType.Plasma => "m", // Plasma - DisplayType.Projector => "p", // DLP projector - _ => "e" // Default to LED LCD + DisplayType.Crt => true, + DisplayType.Plasma => true, + DisplayType.Projector => true, + _ => false }; /// diff --git a/src/Gloam.Core/Calibration/OemCorrectionImporter.cs b/src/Gloam.Core/Calibration/OemCorrectionImporter.cs new file mode 100644 index 0000000..32d6280 --- /dev/null +++ b/src/Gloam.Core/Calibration/OemCorrectionImporter.cs @@ -0,0 +1,289 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Text; +using System.Text.RegularExpressions; +using System.Threading; +using System.Threading.Tasks; + +namespace Gloam.Core.Calibration +{ + /// Outcome of an run. + public sealed class OemImportResult + { + public OemImportResult(bool success, IReadOnlyList installedFiles, string message, string output) + { + Success = success; + InstalledFiles = installedFiles; + Message = message; + Output = output; + } + + /// True when oeminst exited cleanly and wrote at least one file. + public bool Success { get; } + + /// Full paths oeminst reported writing. + public IReadOnlyList InstalledFiles { get; } + + /// One or two sentences for the log. + public string Message { get; } + + /// Raw oeminst output, for the log. + public string Output { get; } + } + + /// + /// Converts the i1Display EDR calibration files that X-Rite i1Profiler and Calibrite + /// PROFILER install into ArgyllCMS CCSS corrections, using Argyll's own oeminst. + /// Once installed, spotread lists them as technology rows (OLED, white LED, RGB LED, ...) + /// in its -y table, so a display type resolves to a matching correction instead + /// of the instrument's generic base calibration. Runs silently from + /// before instrument detection. + /// + /// + /// The EDR files are the vendor's; nothing is downloaded. They are only converted when + /// already present on this PC under the user's own software license. oeminst itself + /// searches one fixed X-Rite path, so the files are enumerated here (Calibrite and + /// 32-bit install roots included) and handed to it explicitly. + /// + public static class OemCorrectionImporter + { + // Vendor folders under each Program Files root that may hold i1d3 EDR files. + private static readonly string[] VendorFolders = { "X-Rite", "Calibrite", "I-O DATA" }; + + // oeminst -v prints one of these per installed file. + private static readonly Regex WrotePattern = new(@"^Wrote '([^']+)'", RegexOptions.Compiled | RegexOptions.Multiline); + + /// The Program Files roots to search on this machine. + public static IReadOnlyList DefaultSearchRoots() + { + var roots = new List(); + foreach (string variable in new[] { "ProgramFiles", "ProgramFiles(x86)", "ProgramW6432" }) + { + string? value = Environment.GetEnvironmentVariable(variable); + if (!string.IsNullOrEmpty(value) && !roots.Contains(value, StringComparer.OrdinalIgnoreCase)) + roots.Add(value); + } + return roots; + } + + /// Every .edr file under a known vendor folder of the given roots. + public static IReadOnlyList FindEdrFiles(IEnumerable? searchRoots = null) + { + var found = new List(); + foreach (string root in searchRoots ?? DefaultSearchRoots()) + { + foreach (string vendor in VendorFolders) + { + string dir = Path.Combine(root, vendor); + if (!Directory.Exists(dir)) continue; + try + { + found.AddRange(Directory.EnumerateFiles(dir, "*.edr", SearchOption.AllDirectories)); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + // A vendor folder we cannot read contributes nothing. + } + } + } + found.Sort(StringComparer.OrdinalIgnoreCase); + return found; + } + + /// Argyll's per-user and system data folders, where oeminst installs and spotread reads. + public static IReadOnlyList ArgyllDataDirs() + { + var dirs = new List(); + foreach (var folder in new[] { Environment.SpecialFolder.ApplicationData, Environment.SpecialFolder.CommonApplicationData }) + { + string basePath = Environment.GetFolderPath(folder); + if (!string.IsNullOrEmpty(basePath)) + dirs.Add(Path.Combine(basePath, "ArgyllCMS")); + } + return dirs; + } + + /// + /// The EDR files whose converted CCSS (oeminst names it after the EDR) is in none of + /// . Running oeminst on these is the only work left. + /// + public static IReadOnlyList PendingEdrFiles(IReadOnlyList edrFiles, IEnumerable argyllDataDirs) + { + var installed = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (string dir in argyllDataDirs) + { + if (!Directory.Exists(dir)) continue; + try + { + foreach (string f in Directory.EnumerateFiles(dir, "*.ccss")) + installed.Add(Path.GetFileNameWithoutExtension(f)); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + // Unreadable folder: treat as having nothing installed. + } + } + + var pending = new List(); + foreach (string edr in edrFiles) + { + if (!installed.Contains(Path.GetFileNameWithoutExtension(edr))) + pending.Add(edr); + } + return pending; + } + + // EDR files already handed to oeminst in this process. An EDR oeminst discards (or + // installs somewhere ArgyllDataDirs does not scan) would otherwise be retried on + // every meter initialization. + private static readonly HashSet Attempted = new(StringComparer.OrdinalIgnoreCase); + private static readonly object AttemptedLock = new(); + + /// + /// Silent one-shot: converts any EDR files on this PC that Argyll does not have yet. + /// Each file is tried at most once per process. Returns null when there was nothing + /// to do. Never throws; failures are logged. + /// + public static async Task EnsureImportedAsync(string argyllBinPath, Action? log, CancellationToken cancellationToken) + { + try + { + var pending = new List(); + lock (AttemptedLock) + { + foreach (string edr in PendingEdrFiles(FindEdrFiles(), ArgyllDataDirs())) + { + if (!Attempted.Contains(edr)) + pending.Add(edr); + } + } + if (pending.Count == 0) return null; + var result = await ImportAsync(argyllBinPath, pending, log, cancellationToken); + MarkAttempted(pending); + log?.Invoke($"EDR import: {result.Message}"); + return result; + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + // Interrupted (setup window closed): leave the files pending for next time. + throw; + } + catch (Exception ex) + { + log?.Invoke($"EDR import skipped: {ex.Message}"); + return null; + } + } + + private static void MarkAttempted(IEnumerable edrFiles) + { + lock (AttemptedLock) + { + foreach (string edr in edrFiles) + Attempted.Add(edr); + } + } + + /// Path of oeminst next to spotread, or null if the Argyll bundle lacks it. + public static string? FindOeminst(string argyllBinPath) + { + foreach (string name in new[] { "oeminst.exe", "oeminst" }) + { + string path = Path.Combine(argyllBinPath, name); + if (File.Exists(path)) return path; + } + return null; + } + + /// Message when the search found nothing to import. + public const string NothingFoundMessage = + "No i1Display EDR files were found on this PC (they ship with X-Rite i1Profiler and Calibrite PROFILER)."; + + /// + /// Runs oeminst -v on (found with + /// ) and reports what it wrote. + /// + public static async Task ImportAsync( + string argyllBinPath, + IReadOnlyList edrFiles, + Action? log, + CancellationToken cancellationToken) + { + if (edrFiles.Count == 0) + return new OemImportResult(false, Array.Empty(), NothingFoundMessage, ""); + + string? oeminst = FindOeminst(argyllBinPath); + if (oeminst == null) + { + return new OemImportResult(false, Array.Empty(), + "The bundled ArgyllCMS has no oeminst tool; EDR files were not converted.", ""); + } + + var psi = new ProcessStartInfo(oeminst) { WorkingDirectory = argyllBinPath }; + psi.ArgumentList.Add("-v"); + foreach (string file in edrFiles) + psi.ArgumentList.Add(file); + + log?.Invoke($"oeminst: importing {edrFiles.Count} EDR file(s) with {oeminst}"); + + string output; + int exitCode; + try + { + var run = await BoundedProcess.RunAsync(psi, TimeSpan.FromMinutes(2), cancellationToken); + output = run.Combined; + exitCode = run.ExitCode; + } + catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) + { + return new OemImportResult(false, Array.Empty(), + "oeminst did not finish within two minutes and was stopped.", ""); + } + + log?.Invoke($"oeminst exit {exitCode}:\n{output}"); + return ParseOutput(output, exitCode); + } + + /// Turns oeminst's verbose output and exit code into a result. + internal static OemImportResult ParseOutput(string output, int exitCode) + { + var installed = new List(); + foreach (Match m in WrotePattern.Matches(output)) + installed.Add(m.Groups[1].Value); + + if (exitCode != 0) + { + string lastLine = LastNonEmptyLine(output); + return new OemImportResult(false, installed, + $"oeminst failed (exit {exitCode}): {lastLine}", output); + } + if (installed.Count == 0) + return new OemImportResult(false, installed, "oeminst ran but reported no installed files.", output); + + // oeminst also installs Argyll's own ref/CRT.ccss alongside any EDR set; count + // only the files that came from the EDRs for the message. + int edrCount = 0; + foreach (string path in installed) + { + if (!Path.GetFileName(path).Equals("CRT.ccss", StringComparison.OrdinalIgnoreCase)) + edrCount++; + } + string dir = Path.GetDirectoryName(installed[0]) ?? ""; + return new OemImportResult(true, installed, + $"Imported {edrCount} meter correction(s) into {dir}.", output); + } + + private static string LastNonEmptyLine(string text) + { + string last = ""; + foreach (string line in text.Split('\n')) + { + string t = line.Trim(); + if (t.Length > 0) last = t; + } + return last; + } + } +} diff --git a/src/Gloam.Core/Calibration/SpotreadDisplayTypes.cs b/src/Gloam.Core/Calibration/SpotreadDisplayTypes.cs new file mode 100644 index 0000000..73c1ded --- /dev/null +++ b/src/Gloam.Core/Calibration/SpotreadDisplayTypes.cs @@ -0,0 +1,465 @@ +using System; +using System.Collections.Generic; +using System.Text; +using System.Text.RegularExpressions; + +namespace Gloam.Core.Calibration +{ + /// + /// One row of the -y table that spotread prints in its usage output for the + /// connected instrument, e.g. n|l i1D3: Non-Refresh display [Default,CB1]. + /// + public sealed class SpotreadDisplayTypeEntry + { + public SpotreadDisplayTypeEntry(IReadOnlyList selectors, string instrument, string description) + { + Selectors = selectors; + Instrument = instrument; + Description = description; + } + + /// Selector tokens accepted by -y for this row ("n", "l", "1", "_X"). + public IReadOnlyList Selectors { get; } + + /// Instrument short name printed before the colon ("i1D3"), or "Other" for the generic row. + public string Instrument { get; } + + /// Calibration description, including any trailing "[Default,CB1]" marker. + public string Description { get; } + + /// True for the instrument's default calibration ("[Default" marker). + public bool IsDefault => Description.Contains("[Default", StringComparison.OrdinalIgnoreCase); + + /// The description without Argyll's trailing "[Default,CB1]" marker, for UI text. + public string DisplayDescription + { + get + { + int bracket = Description.LastIndexOf(" [", StringComparison.Ordinal); + return bracket > 0 && Description.EndsWith(']') + ? Description.Substring(0, bracket) + : Description; + } + } + + /// + /// Argyll's technology name for the row: the description up to the parenthesised + /// free-text descriptor ("LCD White LED IPS" from "LCD White LED IPS (WLED AC LG + /// Samsung)"). Technology matching uses only this part, so a descriptor that happens + /// to mention another technology cannot hijack a display type. + /// + public string TechnologySegment + { + get + { + string d = DisplayDescription; + int paren = d.IndexOf(" (", StringComparison.Ordinal); + return paren > 0 ? d.Substring(0, paren) : d; + } + } + + // Words that mark a row as technology-specific. A default row that names a technology + // (some drivers' "LCD, CCFL Backlight [Default,CB1]") is a correction, not a base. + private static readonly string[] TechnologyWords = + { "lcd", "led", "oled", "ccfl", "crt", "plasma", "projector", "dlp", "phosphor" }; + + private bool NamesTechnology + { + get + { + foreach (string word in TechnologyWords) + { + if (Description.Contains(word, StringComparison.OrdinalIgnoreCase)) + return true; + } + return false; + } + } + + /// True for the instrument's generic non-refresh base calibration. + public bool IsNonRefreshBase => + Description.Contains("non-refresh", StringComparison.OrdinalIgnoreCase) || + (IsDefault && !NamesTechnology && !IsRefreshBase); + + /// True for the instrument's generic refresh (CRT-style) base calibration. + public bool IsRefreshBase => + !Description.Contains("non-refresh", StringComparison.OrdinalIgnoreCase) && + Description.Contains("refresh display", StringComparison.OrdinalIgnoreCase); + + /// True for the generic "Other: l = LCD, c = CRT" row spotread prints for serial ports. + public bool IsGeneric => string.Equals(Instrument, "Other", StringComparison.OrdinalIgnoreCase); + + /// True when the row is a base or generic calibration rather than a technology-specific one. + public bool IsBaseOrGeneric => IsGeneric || IsNonRefreshBase || IsRefreshBase; + + public bool HasSelector(string selector) + { + foreach (string s in Selectors) + { + if (string.Equals(s, selector, StringComparison.Ordinal)) + return true; + } + return false; + } + + /// The selector to pass on the command line: the first single-character one, else the first. + public string PreferredSelector + { + get + { + foreach (string s in Selectors) + { + if (s.Length == 1) return s; + } + return Selectors[0]; + } + } + + public override string ToString() => $"{string.Join("|", Selectors)} {Instrument}: {Description}"; + } + + /// + /// The outcome of resolving a against an instrument's table. + /// + public sealed class SpotreadDisplayTypeChoice + { + public SpotreadDisplayTypeChoice(string selector, string reason, SpotreadDisplayTypeEntry? entry) + { + Selector = selector; + Reason = reason; + Entry = entry; + } + + /// The value passed to spotread's -y. + public string Selector { get; } + + /// Why it was chosen, for logs and the setup screen. + public string Reason { get; } + + /// The table row the selector came from; null when the table was unknown. + public SpotreadDisplayTypeEntry? Entry { get; } + + /// True when a technology-specific correction row was found on the instrument. + public bool IsTechnologyMatch => Entry != null && !Entry.IsBaseOrGeneric; + } + + /// + /// Parses spotread's per-instrument -y table and resolves a + /// to a selector that instrument actually accepts. + /// + /// + /// The selector letters are not a stable ArgyllCMS contract. Argyll builds the table per + /// instrument at run time: a driver's static base calibrations (the i1D3 has only + /// Non-Refresh n|l and Refresh r|c) plus one row per installed CCSS/CCMX + /// file. The technology letters (e white LED, o OLED, b RGB LED, + /// m plasma, p projector) only appear when an X-Rite EDR-derived + /// correction of that technology has been imported into Argyll's data directory, which a + /// fresh Gloam install never does. Passing one of those letters to a clean install makes + /// spotread exit with "Failed to locate display type matching 'o'" (issue #7). + /// + /// When a -X file.ccss correction is also passed, spotread replaces both the + /// spectral calibration and the refresh mode with the ones implied by the CCSS + /// (i1d3_col_cal_spec_set in Argyll's i1d3.c), so -y then only has to be + /// a selector the instrument accepts. + /// + public static class SpotreadDisplayTypeTable + { + // First row of the block: " -y n|l i1D3: Non-Refresh display [Default,CB1]" + // Continuation rows: " r|c i1D3: Refresh display [CB2]" + // Generic row (serial ports): " l|c Other: l = LCD, c = CRT" + // Selector tokens are one character, or '_' plus one character for the two-character form. + private const string SelectorGroup = @"((?:_?[A-Za-z0-9])(?:\|_?[A-Za-z0-9])*)"; + private static readonly Regex FirstRowPattern = new( + @"^\s*-y\s+" + SelectorGroup + @"\s{2,}(\S.*)$", RegexOptions.Compiled); + private static readonly Regex ContinuationRowPattern = new( + @"^\s{2,}" + SelectorGroup + @"\s{2,}(\S.*)$", RegexOptions.Compiled); + + /// + /// Extracts the -y table from spotread usage text. Returns an empty list when + /// the text contains no table (no instrument was enumerated, or the output is not a + /// usage dump). Lines may carry a "stderr> " style prefix from a log. + /// + public static IReadOnlyList Parse(string? usageOutput) + { + var entries = new List(); + if (string.IsNullOrEmpty(usageOutput)) + return entries; + + bool inBlock = false; + foreach (string rawLine in usageOutput.Split('\n')) + { + string line = StripLogPrefix(rawLine.TrimEnd('\r')); + + Match m; + if (!inBlock) + { + m = FirstRowPattern.Match(line); + if (!m.Success) continue; + inBlock = true; + } + else + { + m = ContinuationRowPattern.Match(line); + if (!m.Success) + { + // The block ends at the next option (" -I illum ...") or any other line. + inBlock = false; + continue; + } + } + + string[] selectors = m.Groups[1].Value.Split('|'); + string rest = m.Groups[2].Value.Trim(); + string instrument; + string description; + int colon = rest.IndexOf(": ", StringComparison.Ordinal); + if (colon > 0) + { + instrument = rest.Substring(0, colon).Trim(); + description = rest.Substring(colon + 2).Trim(); + } + else + { + instrument = ""; + description = rest; + } + entries.Add(new SpotreadDisplayTypeEntry(selectors, instrument, description)); + } + return entries; + } + + private static string StripLogPrefix(string line) + { + // Colorimeter log lines look like "[2026-09-15 09:59:06.930] stderr> -y n|l ...". + int idx = line.IndexOf("stderr> ", StringComparison.Ordinal); + if (idx < 0) idx = line.IndexOf("stdout> ", StringComparison.Ordinal); + if (idx < 0) idx = line.IndexOf("[e] ", StringComparison.Ordinal); + if (idx < 0) idx = line.IndexOf("[o] ", StringComparison.Ordinal); + if (idx < 0) return line; + int prefixLen = line[idx] == '[' ? 4 : 8; + return line.Substring(idx + prefixLen); + } + + private readonly struct TypeRule + { + public TypeRule(DisplayType type, string[] preferredSelectors, string[] keywords) + { + PreferredSelectors = preferredSelectors; + Keywords = keywords; + Refresh = type.IsRefreshTechnology(); + } + public string[] PreferredSelectors { get; } + public string[] Keywords { get; } + public bool Refresh { get; } + } + + // Preferred selectors are the letters Argyll's disptechs.c assigns to the matching + // technology; keywords match the description of any installed correction of that + // technology. + private static TypeRule RuleFor(DisplayType type) => type switch + { + DisplayType.LcdLed => new TypeRule(type, new[] { "e" }, new[] { "white led", "wled" }), + DisplayType.Oled => new TypeRule(type, new[] { "o" }, new[] { "oled" }), + // No "wide gamut" keyword: it would match the CCFL wide-gamut EDR row. + DisplayType.LcdWideGamut => new TypeRule(type, new[] { "b" }, + new[] { "rgb led", "rg phosphor", "pfs phosphor", "gb-r phosphor" }), + DisplayType.LcdCcfl => new TypeRule(type, new[] { "l" }, new[] { "ccfl" }), + DisplayType.Crt => new TypeRule(type, new[] { "c" }, new[] { "crt" }), + DisplayType.Plasma => new TypeRule(type, new[] { "m" }, new[] { "plasma" }), + DisplayType.Projector => new TypeRule(type, new[] { "p" }, new[] { "projector", "dlp" }), + _ => new TypeRule(type, Array.Empty(), Array.Empty()) + }; + + /// + /// The selector to pass as -y for when the instrument's + /// table is unknown: n (Non-Refresh) or r (Refresh). Argyll keeps these + /// on the i1D3 and Spyder 3+ base rows even when corrections are installed, whereas + /// l/c get reassigned to CCFL and CRT corrections. Older drivers that + /// only know l/c reject n/r, and the session's rejection + /// retry then resolves from the table the instrument prints. + /// + public static string GenericSelector(DisplayType type) => RuleFor(type).Refresh ? "r" : "n"; + + /// + /// Picks the -y selector for from the instrument's + /// table: a technology-matched correction if one is installed, else the instrument's + /// base calibration for the refresh class, else the generic selector. + /// + public static string Resolve(DisplayType type, IReadOnlyList? table, out string reason) + { + var choice = Resolve(type, table); + reason = choice.Reason; + return choice.Selector; + } + + /// + public static SpotreadDisplayTypeChoice Resolve(DisplayType type, IReadOnlyList? table) + { + var rule = RuleFor(type); + string generic = GenericSelector(type); + string className = rule.Refresh ? "Refresh" : "Non-Refresh"; + + if (table == null || !HasInstrumentRows(table)) + { + return new SpotreadDisplayTypeChoice(generic, + $"the instrument's display-type table is unknown; using the {className} base selector", null); + } + + // 1. Technology-specific rows (installed EDR/CCSS corrections) whose technology + // name matches this type. Prefer the row carrying Argyll's own letter for it. + SpotreadDisplayTypeEntry? keywordMatch = null; + foreach (var entry in table) + { + if (entry.IsBaseOrGeneric || !MatchesKeyword(entry.TechnologySegment, rule.Keywords)) + continue; + keywordMatch ??= entry; + foreach (string sel in rule.PreferredSelectors) + { + if (entry.HasSelector(sel)) + return new SpotreadDisplayTypeChoice(sel, $"matched the installed correction '{entry.Description}'", entry); + } + } + if (keywordMatch != null) + { + return new SpotreadDisplayTypeChoice(keywordMatch.PreferredSelector, + $"matched the installed correction '{keywordMatch.Description}'", keywordMatch); + } + + // 2. The instrument's base calibration for this refresh class. + foreach (var entry in table) + { + if (rule.Refresh ? entry.IsRefreshBase : entry.IsNonRefreshBase) + { + return new SpotreadDisplayTypeChoice(entry.PreferredSelector, + $"no {type}-specific correction is installed for this instrument; using its base calibration '{entry.Description}'", + entry); + } + } + + // 3. A driver without named base rows: the instrument row that accepts the + // classic l (LCD) or c (CRT) selector. + string classic = rule.Refresh ? "c" : "l"; + foreach (var entry in table) + { + if (!entry.IsGeneric && entry.HasSelector(classic)) + { + return new SpotreadDisplayTypeChoice(classic, + $"no {type}-specific correction is installed for this instrument; using '{entry.Description}'", entry); + } + } + + // 4. Nothing usable listed (unexpected); the base selector plus the rejection + // retry is the most likely to succeed. + return new SpotreadDisplayTypeChoice(generic, + $"the instrument's display-type table lists no base calibration; using the {className} base selector", null); + } + + // Argyll insttypes.c: inst_name(), printed inside the parentheses of the -c port + // list, and inst_sname(), printed before every -y row. Matched longest name first so + // "SpyderX2" is not read as "SpyderX". + private static readonly (string LongName, string ShortName)[] InstrumentNames = Sorted(new[] + { + ("X-Rite DTP20", "DTP20"), ("X-Rite DTP22", "DTP22"), ("X-Rite DTP41", "DTP41"), + ("X-Rite DTP51", "DTP51"), ("X-Rite DTP92", "DTP92"), ("X-Rite DTP94", "DTP94"), + ("GretagMacbeth Spectrolino", "Spectrolino"), ("GretagMacbeth SpectroScanT", "SpectroScanT"), + ("GretagMacbeth SpectroScan", "SpectroScan"), ("Spectrocam", "Spectrocam"), + ("GretagMacbeth i1 Display 1", "i1D1"), ("GretagMacbeth i1 Display 2", "i1D2"), + ("X-Rite i1 DisplayPro, ColorMunki Display", "i1D3"), ("GretagMacbeth i1 Monitor", "i1 Monitor"), + ("GretagMacbeth i1 Pro", "i1Pro"), ("X-Rite i1 Pro 2", "i1Pro2"), ("X-Rite i1 Pro 3", "i1Pro3"), + ("X-Rite ColorMunki", "ColorMunki"), ("Colorimtre HCFR", "HCFR"), + ("ColorVision Spyder1", "Spyder1"), ("ColorVision Spyder2", "Spyder2"), + ("Datacolor Spyder3", "Spyder3"), ("Datacolor Spyder4", "Spyder4"), ("Datacolor Spyder5", "Spyder5"), + ("Datacolor SpyderX2", "SpyderX2"), ("Datacolor SpyderX", "SpyderX"), ("Datacolor Spyder 2024", "Spyder 2024"), + ("GretagMacbeth Huey", "Huey"), ("ColorMunki Smile", "Smile"), + ("JETI specbos 1201", "specbos 1201"), ("JETI specbos 2501", "specbos 2501"), ("JETI specbos", "specbos"), + ("JETI spectraval", "spectraval"), ("Klein K-10", "K-10"), ("Image Engineering EX1", "EX1"), + ("SwatchMate Cube", "Cube"), ("Hughski ColorHug2", "ColorHug2"), ("Hughski ColorHug", "ColorHug"), + }); + + private static (string LongName, string ShortName)[] Sorted((string LongName, string ShortName)[] names) + { + Array.Sort(names, (a, b) => b.LongName.Length.CompareTo(a.LongName.Length)); + return names; + } + + /// + /// The short instrument name spotread prints before each -y row ("i1D3") for a + /// port-list descriptor such as "hid:/33 (X-Rite i1 DisplayPro, ColorMunki Display)". + /// Null when the descriptor names no known instrument (serial ports, unknown models). + /// + public static string? ShortNameForDescriptor(string? descriptor) + { + if (string.IsNullOrWhiteSpace(descriptor)) return null; + foreach (var (longName, shortName) in InstrumentNames) + { + if (descriptor.Contains(longName, StringComparison.OrdinalIgnoreCase)) + return shortName; + } + return null; + } + + /// + /// Restricts a table to the rows printed by the instrument named in + /// (plus the generic row). spotread prints one merged + /// block for every enumerated instrument, so with two meters connected a row from the + /// other meter would otherwise be selected. Returns the table unchanged when the + /// instrument is unknown or none of its rows are present. + /// + public static IReadOnlyList ForInstrument( + IReadOnlyList table, string? descriptor) + { + string? shortName = ShortNameForDescriptor(descriptor); + if (shortName == null || table.Count == 0) return table; + + var scoped = new List(); + bool anyInstrumentRow = false; + foreach (var entry in table) + { + if (entry.IsGeneric) + { + scoped.Add(entry); + } + else if (string.Equals(entry.Instrument, shortName, StringComparison.OrdinalIgnoreCase)) + { + scoped.Add(entry); + anyInstrumentRow = true; + } + } + return anyInstrumentRow ? scoped : table; + } + + private static bool MatchesKeyword(string description, string[] keywords) + { + foreach (string keyword in keywords) + { + if (description.Contains(keyword, StringComparison.OrdinalIgnoreCase)) + return true; + } + return false; + } + + /// True when the table has at least one row printed by an instrument (not just the generic row). + public static bool HasInstrumentRows(IReadOnlyList? table) + { + if (table == null) return false; + foreach (var entry in table) + { + if (!entry.IsGeneric) return true; + } + return false; + } + + /// One-line summary for logs: "n|l Non-Refresh display [Default,CB1]; r|c Refresh display [CB2]; ...". + public static string Describe(IReadOnlyList? table) + { + if (table == null || table.Count == 0) return "(none)"; + var sb = new StringBuilder(); + foreach (var entry in table) + { + if (sb.Length > 0) sb.Append("; "); + sb.Append(string.Join("|", entry.Selectors)).Append(' ').Append(entry.Description); + } + return sb.ToString(); + } + } +} diff --git a/src/Gloam.Core/Calibration/SpotreadSession.cs b/src/Gloam.Core/Calibration/SpotreadSession.cs index 410acd7..eeea268 100644 --- a/src/Gloam.Core/Calibration/SpotreadSession.cs +++ b/src/Gloam.Core/Calibration/SpotreadSession.cs @@ -1,7 +1,9 @@ using System; +using System.Collections.Generic; using System.Diagnostics; using System.Globalization; using System.IO; +using System.Linq; using System.Text; using System.Text.RegularExpressions; using System.Threading; @@ -48,6 +50,12 @@ internal sealed class SpotreadSession : IAsyncDisposable @"XYZ:\s*([+-]?(?:\d+\.?\d*|\.\d+)(?:[eE][+-]?\d+)?)\s+([+-]?(?:\d+\.?\d*|\.\d+)(?:[eE][+-]?\d+)?)\s+([+-]?(?:\d+\.?\d*|\.\d+)(?:[eE][+-]?\d+)?)", RegexOptions.Compiled | RegexOptions.IgnoreCase); + // "Diagnostic: Failed to locate display type matching 'o'" (spotread.c usage() call + // after inst_get_disptype_index fails). The selector is quoted as printed. + private static readonly Regex DisplayTypeRejectedPattern = new( + @"Failed to locate display type matching '([^']+)'", + RegexOptions.Compiled | RegexOptions.IgnoreCase); + // Spectral header emitted by spotread -s with a spectrometer, e.g. // "Spectrum from 380.000000 to 730.000000 nm in 36 steps" // Wording/precision varies across Argyll versions: the trailing noun after the count @@ -199,6 +207,14 @@ private static bool IsFatalErrorLine(string lowerLine) private volatile string? _fatalError; private volatile bool _disposed; + + // Set when spotread rejects our -y selector ("Failed to locate display type matching + // 'o'"). spotread then prints its usage, including the connected instrument's own -y + // table, and exits. Everything after the diagnostic is kept in _usageCapture so the + // table can be parsed and the session restarted with a selector the instrument lists. + private volatile bool _displayTypeRejected; + private readonly StringBuilder _usageCapture = new(); + private const int MaxUsageCaptureChars = 32_000; private volatile bool _attemptedHidOpen; private volatile bool _sharingViolationSeen; private volatile bool _poisoned; @@ -293,7 +309,9 @@ public static async Task StartAsync( Action log, CancellationToken cancellationToken, string? correctionFilePath = null, - bool spectralMode = false) + bool spectralMode = false, + IReadOnlyList? displayTypeTable = null, + string? instrumentDescriptor = null) { // NOTE (flag hygiene): spotread's -H flag is HIGH-RESOLUTION SPECTRAL mode and // only applies to spectrometers (i1 Pro etc.) — it is NOT an "HDR mode". This @@ -306,8 +324,73 @@ public static async Task StartAsync( // OS display-mode flip. _ = hdrMode; - var args = BuildSpotreadArguments(instrumentIndex, displayType, spectralMode, correctionFilePath); + var args = BuildSpotreadArguments(instrumentIndex, displayType, spectralMode, correctionFilePath, + displayTypeTable, out string? selectorReason); + if (selectorReason != null) + log($"spotread -y selector for {displayType}: {selectorReason}"); + + try + { + return await StartProcessAsync(spotreadPath, args, spectralMode, displayTypeTable, log, cancellationToken); + } + catch (DisplayTypeRejectedException rejected) + { + // The table spotread printed while rejecting the selector comes from the + // instrument it actually opened, so it outranks the one captured at detection + // (which may be empty, stale, or from another instrument). Retry once with it. + var table = SpotreadDisplayTypeTable.ForInstrument(rejected.Table, instrumentDescriptor); + var retryArgs = BuildSpotreadArguments(instrumentIndex, displayType, spectralMode, correctionFilePath, + table, out string? retryReason); + if (retryArgs.SequenceEqual(args)) + { + // Same selector again (a merged two-meter table the descriptor could not + // scope): drop every row that carries it and resolve once more. + string rejectedSelector = args[args.IndexOf("-y") + 1]; + table = table.Where(e => !e.HasSelector(rejectedSelector)).ToList(); + retryArgs = BuildSpotreadArguments(instrumentIndex, displayType, spectralMode, correctionFilePath, + table, out retryReason); + } + if (table.Count == 0 || retryArgs.SequenceEqual(args)) + throw; + + log($"spotread rejected the -y selector; retrying with {retryReason}. " + + $"Instrument -y table: {SpotreadDisplayTypeTable.Describe(table)}"); + return await StartProcessAsync(spotreadPath, retryArgs, spectralMode, table, log, cancellationToken); + } + } + + /// + /// Thrown by when spotread exited because it did not + /// accept the -y selector. Carries the -y table spotread printed on + /// the way out (empty if the output was cut off). Derives from + /// so existing callers need no new handling. + /// + internal sealed class DisplayTypeRejectedException : InvalidOperationException + { + public DisplayTypeRejectedException(string message, IReadOnlyList table) + : base(message) + { + Table = table; + } + + public IReadOnlyList Table { get; } + } + /// + /// The -y table the session's selector was resolved against. After a retry this + /// is the table spotread itself printed, which callers should keep for later sessions. + /// + public IReadOnlyList EffectiveDisplayTypeTable { get; private set; } + = Array.Empty(); + + private static async Task StartProcessAsync( + string spotreadPath, + List args, + bool spectralMode, + IReadOnlyList? displayTypeTable, + Action log, + CancellationToken cancellationToken) + { var psi = new ProcessStartInfo(spotreadPath) { UseShellExecute = false, @@ -333,7 +416,14 @@ public static async Task StartAsync( KillStraySpotread(log); var process = new Process { StartInfo = psi, EnableRaisingEvents = true }; - var session = new SpotreadSession(process, log) { SpectralMode = spectralMode }; + var session = new SpotreadSession(process, log) + { + SpectralMode = spectralMode, + EffectiveDisplayTypeTable = displayTypeTable ?? Array.Empty() + }; + // Disposed by the finally block unless the ready prompt arrives, which hands the + // session to the caller. + SpotreadSession? owned = session; log($"SpotreadSession starting: {spotreadPath} {string.Join(" ", args)}"); process.Start(); @@ -351,30 +441,89 @@ public static async Task StartAsync( { await session._readyTcs.Task.WaitAsync(startupCts.Token); log("SpotreadSession ready."); + owned = null; return session; } catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { - await session.DisposeAsync(); throw; } + catch (Exception ex) + { + // spotread's exit is observed before its last stderr lines are delivered, and + // the -y rejection diagnostic and usage table are among them. Drain first, + // then decide why it failed. + if (process.HasExited || session._displayTypeRejected) + await session.DrainOutputAsync(); + if (session._displayTypeRejected) + { + var table = session.UsageDisplayTypeTable; + throw new DisplayTypeRejectedException(session.BuildDisplayTypeRejectedMessage(table), table); + } + if (ex is OperationCanceledException) + { + throw new InvalidOperationException( + "Colorimeter did not become ready within 15 seconds. " + + "Another application (DisplayCAL's profile loader, i1Profiler) may be " + + "holding the device, or the USB driver needs to be installed.\n\n" + + "Recent spotread output:\n" + session.SnapshotRecentLog()); + } + throw; + } + finally + { + if (owned != null) + await owned.DisposeAsync(); + } + } + + /// + /// Waits (bounded) for the process to exit and its redirected output to reach EOF, so + /// every usage line has passed through . + /// + private async Task DrainOutputAsync() + { + if (_process == null) return; + try + { + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(3)); + await _process.WaitForExitAsync(cts.Token); + } catch (OperationCanceledException) { - string tail = session.SnapshotRecentLog(); - await session.DisposeAsync(); - throw new InvalidOperationException( - "Colorimeter did not become ready within 15 seconds. " + - "Another application (DisplayCAL's profile loader, i1Profiler) may be " + - "holding the device, or the USB driver needs to be installed.\n\n" + - "Recent spotread output:\n" + tail); + // Still running or still writing after 3 s; parse whatever was captured. } - catch + catch (InvalidOperationException) { - await session.DisposeAsync(); - throw; + // Process already released. } } + /// + /// The -y table parsed from the usage text spotread printed after rejecting + /// our selector. Empty until is set. + /// + internal IReadOnlyList UsageDisplayTypeTable + { + get + { + string captured; + lock (_stateLock) captured = _usageCapture.ToString(); + return SpotreadDisplayTypeTable.Parse(captured); + } + } + + /// True once spotread reported that it does not accept the -y selector. + internal bool DisplayTypeRejected => _displayTypeRejected; + + private string BuildDisplayTypeRejectedMessage(IReadOnlyList table) + { + string head = _fatalError ?? "spotread rejected the display type selector."; + return table.Count > 0 + ? $"{head} Selectors this instrument accepts: {SpotreadDisplayTypeTable.Describe(table)}" + : $"{head} spotread exited before listing the selectors this instrument accepts."; + } + /// /// Builds spotread's command-line arguments. Extracted so the colorimeter-vs- /// spectrometer flag differences can be unit-tested without spawning a process. @@ -398,13 +547,31 @@ public static async Task StartAsync( /// Spectral mode adds -s (print the raw emission spectrum after each reading) and -H /// (the instrument's high-resolution spectral mode, ~3.3 nm bins on an i1 Pro). /// - internal static System.Collections.Generic.List BuildSpotreadArguments( + internal static List BuildSpotreadArguments( int instrumentIndex, DisplayType displayType, bool spectralMode, - string? correctionFilePath) + string? correctionFilePath, + IReadOnlyList? displayTypeTable = null) + => BuildSpotreadArguments(instrumentIndex, displayType, spectralMode, correctionFilePath, + displayTypeTable, out _); + + /// + /// The connected instrument's -y table as parsed from spotread -?, or + /// null/empty when the instrument was not enumerated. The selector is resolved against + /// it so spotread is never handed a letter the instrument does not list. + /// + /// Why that selector was chosen (for the log); null in spectral mode. + internal static List BuildSpotreadArguments( + int instrumentIndex, + DisplayType displayType, + bool spectralMode, + string? correctionFilePath, + IReadOnlyList? displayTypeTable, + out string? displayTypeReason) { - var args = new System.Collections.Generic.List { "-v" }; + displayTypeReason = null; + var args = new List { "-v" }; if (!spectralMode) args.Add("-N"); // colorimeter-only: skip power-up dark calibration @@ -416,8 +583,10 @@ internal static System.Collections.Generic.List BuildSpotreadArguments( if (!spectralMode) { // colorimeter-only: display-type correction (a spectrometer needs none) + string selector = SpotreadDisplayTypeTable.Resolve(displayType, displayTypeTable, out string reason); + displayTypeReason = $"-y {selector} ({reason})"; args.Add("-y"); - args.Add(displayType.ToSpotreadFlag()); + args.Add(selector); } if (spectralMode) @@ -660,6 +829,27 @@ private void OnLine(string? line, bool isError) // Keep the recent-log window bounded so a very chatty spotread doesn't // let it grow without bound over a long calibration. if (_recentLog.Length > 4000) _recentLog.Remove(0, _recentLog.Length - 2000); + + // Everything spotread prints after rejecting the -y selector is its usage + // text, which carries the instrument's real -y table. Keep it whole. + if (_displayTypeRejected && _usageCapture.Length < MaxUsageCaptureChars) + _usageCapture.AppendLine(line); + } + + // 0. spotread rejected our -y selector. It prints its usage next and exits; the + // exit handler fails the session with _fatalError, and StartAsync retries with + // a selector taken from the table in that usage text. Usage lines are only + // captured, never classified as prompts or errors. + if (_displayTypeRejected) + return; + var rejected = DisplayTypeRejectedPattern.Match(line); + if (rejected.Success) + { + _fatalError = + $"spotread does not accept display type selector '{rejected.Groups[1].Value}' " + + $"for this instrument ({line.Trim()})."; + _displayTypeRejected = true; + return; } // 1. Did a measurement just complete? diff --git a/src/Gloam/CalibrationSetupWindow.xaml b/src/Gloam/CalibrationSetupWindow.xaml index 8fe4702..ca921b3 100644 --- a/src/Gloam/CalibrationSetupWindow.xaml +++ b/src/Gloam/CalibrationSetupWindow.xaml @@ -147,6 +147,9 @@ +