Skyline: Improve code for finding Thermo method export DLLs (#3352)

Skyline: Improve code for finding Thermo method export DLLs
- Thermo reported this code as failing on an Ascend at Thermo
- This code was written after a full review of 7 machines in the MacCoss lab and the Ascend at Thermo to do a more complete job of finding the necessary DLLs
- More recent machines have added Thermo.TNG.MethodXMLInterface2.dll to support Adaptive RT so it needs to be copied if it is found
- Added a fairly thorough unit test for this more complicated new code to make sure it works as expected and stays working
- added tests of DLL overwrite based on DLL last modified dates
- improved support for cases seen in the MacCoss lab
- but also turn off DLL copying, since Thermo feels this should not be necessary and the variety of DLLs we are seeing is not their intention.
- easier to test DLL combinations when this code is turned off
- fixes to Thermo method export
- made it possible to have just "Thermo" as a method type where Skyline auto-detects the instrument form the registry
- fixed command-line --exp-method-instrument to work with "Thermo"
- expanded testing to include command-line
- improved error messages for failure to find Thermo installation
- added error message testing in the ExportMethodDlg

Author: brendanx67

pwiz_tools/Skyline/CommandArgs.cs

@@ -2036,9 +2036,9 @@ public bool RunningBatchCommands
 
         // Export isolation / transition list
         public static readonly Argument ARG_EXP_ISOLATION_LIST_INSTRUMENT = new DocArgument(@"exp-isolationlist-instrument",
-            ExportInstrumentType.ISOLATION_LIST_TYPES, (c, p) => c.ParseExpIsolationListInstrumentType(p)) {WrapValue = true};
+            ExportInstrumentType.ISOLATION_LIST_TYPES, (c, p) => c.ParseExpIsolationListInstrumentType(p)) { WrapValue = true, HasValueChecking = true };
         public static readonly Argument ARG_EXP_TRANSITION_LIST_INSTRUMENT = new DocArgument(@"exp-translist-instrument",
-            ExportInstrumentType.TRANSITION_LIST_TYPES, (c, p) => c.ParseExpTransitionListInstrumentType(p)) {WrapValue = true};
+            ExportInstrumentType.TRANSITION_LIST_TYPES, (c, p) => c.ParseExpTransitionListInstrumentType(p)) { WrapValue = true, HasValueChecking = true };
         private static readonly ArgumentGroup GROUP_LISTS = new ArgumentGroup(() => CommandArgUsage.CommandArgs_GROUP_LISTS_Exporting_isolation_transition_lists, false,
             ARG_EXP_ISOLATION_LIST_INSTRUMENT, ARG_EXP_TRANSITION_LIST_INSTRUMENT) {LeftColumnWidth = 34};
 
@@ -2122,7 +2122,7 @@ private void WriteInstrumentValueError(NameValuePair pair, string[] listInstrume
 
         // Export method
         public static readonly Argument ARG_EXP_METHOD_INSTRUMENT = new DocArgument(@"exp-method-instrument",
-            ExportInstrumentType.METHOD_TYPES, (c, p) => c.ParseExpMethodInstrumentType(p)) { WrapValue = true };
+            ExportInstrumentType.METHOD_TYPES, (c, p) => c.ParseExpMethodInstrumentType(p)) { WrapValue = true, HasValueChecking = true };
         public static readonly Argument ARG_EXP_TEMPLATE = new DocArgument(@"exp-template", PATH_TO_FILE,
             (c, p) => c.TemplateFile = p.ValueFullPath);
         private static readonly ArgumentGroup GROUP_METHOD = new ArgumentGroup(() => CommandArgUsage.CommandArgs_GROUP_METHOD_Exporting_native_instrument_methods, false,
@@ -2134,7 +2134,8 @@ public string MethodInstrumentType
             get { return _methodInstrumentType; }
             set
             {
-                if (ExportInstrumentType.METHOD_TYPES.Any(inst => inst.Equals(value)))
+                if (ExportInstrumentType.METHOD_TYPES.Any(inst => inst.Equals(value)) ||
+                    ExportInstrumentType.ThermoInstallationType(value) != null) // Allow Thermo instrument names for which we know the installation type
                 {
                     _methodInstrumentType = value;
                 }
@@ -2708,6 +2709,7 @@ public string[] Values
             public bool WrapValue { get; set; }
             public bool OptionalValue { get; set; }
             public bool InternalUse { get; set; }
+            public bool HasValueChecking { get; set; }  // Set to avoid default checking against values listed for documentation
 
             public string ArgumentText
             {
@@ -3078,7 +3080,7 @@ public bool IsMatch(Argument arg)
                     else
                     {
                         var val = Value;
-                        if (arg.Values != null && !arg.Values.Any(v => v.Equals(val, StringComparison.CurrentCultureIgnoreCase)))
+                        if (arg.Values != null && !arg.HasValueChecking && !arg.Values.Any(v => v.Equals(val, StringComparison.CurrentCultureIgnoreCase)))
                             throw new ValueInvalidException(arg, Value, arg.Values);
                     }
                 }

pwiz_tools/Skyline/CommandLine.cs

@@ -3991,6 +3991,42 @@ private bool ExportInstrumentFile(ExportFileType type, CommandArgs args)
 
             if (Equals(type, ExportFileType.Method))
             {
+                // If the instrument type is either just Thermo or a Thermo instrument type that
+                // support the TNG XML method API
+                string thermoInstallationType = ExportInstrumentType.ThermoInstallationType(args.MethodInstrumentType);
+                if (thermoInstallationType != null || Equals(args.MethodInstrumentType, ExportInstrumentType.THERMO))
+                {
+                    var dllFinder = new ThermoDllFinder();
+                    var thermoSoftwareInfo = dllFinder.GetSoftwareInfo();   // CONSIDER: This behaves differently for tests on a computer with Thermo software installed
+                    if (thermoSoftwareInfo.InstrumentType == null)
+                    {
+                        _out.WriteLine(TextUtil.SpaceSeparate(Resources.CommandStatusWriter_WriteLine_Error_,
+                            ModelResources.ThermoMassListExporter_EnsureLibraries_Failed_to_find_a_valid_Thermo_instrument_installation_));
+                        _out.WriteLine(thermoSoftwareInfo.FailureReason);
+                        return false;
+                    }
+                    // If not generally exporting a Thermo method, and the instrument type
+                    // specified does not match the installed type, then error.
+                    else if (Equals(args.MethodInstrumentType, ExportInstrumentType.THERMO))
+                    {
+                        var instrumentType = ExportInstrumentType.ThermoTypeFromInstallationType(thermoSoftwareInfo.InstrumentType);
+                        if (instrumentType == null)
+                        {
+                            _out.WriteLine(TextUtil.SpaceSeparate(Resources.CommandStatusWriter_WriteLine_Error_, 
+                                string.Format(ModelResources.ThermoMassListExporter_EnsureLibraries_Unknown_Thermo_instrument_type___0___installed_, thermoSoftwareInfo.InstrumentType)));
+                            return false;
+                        }
+
+                        args.MethodInstrumentType = instrumentType;
+                    }
+                    else if (!Equals(thermoSoftwareInfo.InstrumentType, thermoInstallationType))
+                    {
+                        _out.WriteLine(ModelResources.CommandLine_ExportInstrumentFile_Error__The_specified_instrument_type___0___does_not_match_the_installed_software___1___,
+                            args.MethodInstrumentType, thermoSoftwareInfo.InstrumentType);
+                        _out.WriteLine(ModelResources.CommandLine_ExportInstrumentFile_Use_the_instrument_type__Thermo__to_export_a_method_with_the_installed_software_);
+                        return false;
+                    }
+                }
                 if (string.IsNullOrEmpty(args.TemplateFile))
                 {
                     _out.WriteLine(Resources.CommandLine_ExportInstrumentFile_Error__A_template_file_is_required_to_export_a_method_);

pwiz_tools/Skyline/Executables/BuildMethod/BuildThermoMethod/Program.cs

@@ -63,7 +63,7 @@ private static string GetErrorText(Exception x)
             // Thermo libraries can sometimes throw exceptions with multi-line errors
             // Any lines that do not get ERROR: prepended will not get reported
             var sb = new StringBuilder();
-            var reader = new StringReader(x.Message);
+            var reader = new StringReader(x.ToString());
             string line;
             while ((line = reader.ReadLine()) != null)
                 sb.AppendLine("ERROR: " + line);
@@ -77,14 +77,7 @@ static void Usage()
                     "   Takes template Thermo method file and a Skyline generated Thermo\n" +
                     "   scheduled transition list as inputs, to generate a method file\n" +
                     "   as output.\n" +
-                    "   -f               Fusion method [default]\n" +
-                    "   -e               Endura method\n" +
-                    "   -q               Quantiva method\n" +
-                    "   -a               Altis method\n" +
-                    "   -p               Exploris method\n" +
-                    "   -l               Fusion Lumos method\n" +
-                    "   -c               Eclipse method\n" +
-                    "   -t               Stellar method\n" +
+                    "   -t <inst type>   as in registry [default TSQAltis]\n" +
                     "   -o <output file> New method is written to the specified output file\n" +
                     "   -x               Export method XML to <basename>.xml file\n" +
                     "   -s               Transition list is read from stdin.\n" +
@@ -343,14 +336,25 @@ internal sealed class BuildThermoMethod
     {
         private const string ThermoMethodExt = ".meth";
         private const string InstrumentFusion = "OrbitrapFusion";
-        private const string InstrumentExploris = "OrbitrapExploris480";
         private const string InstrumentFusionLumos = "OrbitrapFusionLumos";
         private const string InstrumentEclipse = "OrbitrapEclipse";
+        private const string InstrumentAscend = "OrbitrapAscend";
+        private const string InstrumentAstral = "OrbitrapAstral";
+        private const string InstrumentExploris = "OrbitrapExploris480";
         private const string InstrumentEndura = "TSQEndura";
         private const string InstrumentQuantiva = "TSQQuantiva";
         private const string InstrumentAltis = "TSQAltis";
         private const string InstrumentStellar = "Stellar";
 
+        private static readonly string[] KnownInstruments =
+        {
+            InstrumentFusion, InstrumentFusionLumos, InstrumentEclipse, InstrumentAscend,   // tribrids
+            InstrumentAstral, // tofs
+            InstrumentExploris, // exactives
+            InstrumentEndura, InstrumentQuantiva, InstrumentAltis,  // tsqs
+            InstrumentStellar, // qits
+        };
+
         private string InstrumentType { get; set; }
         private string InstrumentVersion { get; set; }
         private string TemplateMethod { get; set; }
@@ -359,7 +363,7 @@ internal sealed class BuildThermoMethod
 
         public BuildThermoMethod()
         {
-            InstrumentType = InstrumentQuantiva;
+            InstrumentType = InstrumentAltis;
             InstrumentVersion = null;
             TemplateMethod = null;
             MethodTrans = new List<MethodTransitions>();
@@ -377,29 +381,12 @@ public void ParseCommandArgs(string[] args)
                 string arg = args[i++];
                 switch (arg[1])
                 {
-                    case 'f':
-                        InstrumentType = InstrumentFusion;
-                        break;
-                    case 'e':
-                        InstrumentType = InstrumentEndura;
-                        break;
-                    case 'q':
-                        InstrumentType = InstrumentQuantiva;
-                        break;
-                    case 'a':
-                        InstrumentType = InstrumentAltis;
-                        break;
-                    case 'p':
-                        InstrumentType = InstrumentExploris;
-                        break;
-                    case 'l':
-                        InstrumentType = InstrumentFusionLumos;
-                        break;
-                    case 'c':
-                        InstrumentType = InstrumentEclipse;
-                        break;
                     case 't':
-                        InstrumentType = InstrumentStellar;
+                        if (i >= args.Length)
+                            throw new UsageException();
+                        InstrumentType = args[i++];
+                        if (!KnownInstruments.Contains(InstrumentType))
+                            throw new UsageException(string.Format("Unknown instrument type {0}", InstrumentType));
                         break;
                     case 'o':
                         if (i >= args.Length)
@@ -423,7 +410,7 @@ public void ParseCommandArgs(string[] args)
             InstrumentVersion = MethodXMLFactory.GetLatestInstalledVersion(InstrumentType);
 
             if (multiFile && !string.IsNullOrEmpty(outputMethod))
-                Usage("Multi-file and specific output are not compatibile.");
+                Usage("Multi-file and specific output are not compatible.");
 
             int argcLeft = args.Length - i;
             if (argcLeft < 1 || (!readStdin && argcLeft < 2))
@@ -572,11 +559,11 @@ public void Build()
                             case InstrumentFusion:
                                 mx.ApplyMethodModificationsFromXML(GetFusionModificationXml(InstrumentType, listItems, outMeth));
                                 break;
+                            case InstrumentAstral:
                             case InstrumentExploris:
                                 mx.ApplyMethodModificationsFromXML(GetExplorisXml(InstrumentType, listItems, outMeth));
                                 break;
                             case InstrumentEclipse:
-                            // case InstrumentFusion:
                             case InstrumentFusionLumos:
                                 mx.ApplyMethodModificationsFromXML(GetCalciumXml(InstrumentType, listItems, outMeth));
                                 break;