From 2fb924ec0d110eb3ca6c3fb06efa27acd34b2750 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Tue, 23 Jun 2020 12:10:42 +0100 Subject: [PATCH] JAL-3518 pull up closeViewer() and external viewer process monitor --- src/jalview/ext/jmol/JmolCommands.java | 6 ++ src/jalview/ext/pymol/PymolCommands.java | 7 ++ src/jalview/ext/pymol/PymolManager.java | 46 +++++------ src/jalview/ext/rbvi/chimera/ChimeraCommands.java | 7 ++ src/jalview/ext/rbvi/chimera/ChimeraXCommands.java | 7 ++ .../ext/rbvi/chimera/JalviewChimeraBinding.java | 48 +----------- src/jalview/gui/PymolBindingModel.java | 21 ++--- src/jalview/structure/StructureCommandsI.java | 6 ++ .../structures/models/AAStructureBindingModel.java | 82 ++++++++++++++------ 9 files changed, 120 insertions(+), 110 deletions(-) diff --git a/src/jalview/ext/jmol/JmolCommands.java b/src/jalview/ext/jmol/JmolCommands.java index 085fbd5..25f6aec 100644 --- a/src/jalview/ext/jmol/JmolCommands.java +++ b/src/jalview/ext/jmol/JmolCommands.java @@ -475,4 +475,10 @@ public class JmolCommands extends StructureCommandsBase { return loadFile(filepath); } + + @Override + public StructureCommandI closeViewer() + { + return null; // not an external viewer + } } diff --git a/src/jalview/ext/pymol/PymolCommands.java b/src/jalview/ext/pymol/PymolCommands.java index 3493d03..7e5ba2d 100644 --- a/src/jalview/ext/pymol/PymolCommands.java +++ b/src/jalview/ext/pymol/PymolCommands.java @@ -317,4 +317,11 @@ public class PymolCommands extends StructureCommandsBase return new StructureCommand("load", filepath, "", "0", "pse"); } + @Override + public StructureCommandI closeViewer() + { + // https://pymolwiki.org/index.php/Quit + return new StructureCommand("quit"); + } + } diff --git a/src/jalview/ext/pymol/PymolManager.java b/src/jalview/ext/pymol/PymolManager.java index e3b913b..26c780d 100644 --- a/src/jalview/ext/pymol/PymolManager.java +++ b/src/jalview/ext/pymol/PymolManager.java @@ -108,17 +108,6 @@ public class PymolManager return launched; } - public void exitPymol() - { - if (isPymolLaunched() && pymolProcess != null) - { - sendCommand(new StructureCommand("quit"), false); - } - pymolProcess = null; - // currentModelsMap.clear(); - this.pymolXmlRpcPort = 0; - } - /** * Sends the command to Pymol; if requested, tries to get and return any * replies, else returns null @@ -211,13 +200,13 @@ public class PymolManager return sb.toString(); } - public boolean launchPymol() + public Process launchPymol() { // todo pull up much of this // Do nothing if already launched if (isPymolLaunched()) { - return true; + return pymolProcess; } String error = "Error message: "; @@ -244,20 +233,28 @@ public class PymolManager break; } catch (Exception e) { - // pPymol could not be started using this path + // Pymol could not be started using this path error += e.getMessage(); } } - if (error.length() == 0) + + if (pymolProcess != null) { this.pymolXmlRpcPort = getPortNumber(); - System.out.println( - "PyMOL XMLRPC started on port " + pymolXmlRpcPort); - return (pymolXmlRpcPort > 0); + if (pymolXmlRpcPort > 0) + { + Cache.log.info("PyMOL XMLRPC started on port " + pymolXmlRpcPort); + } + else + { + error += "Failed to read PyMOL XMLRPC port number"; + Cache.log.error(error); + pymolProcess.destroy(); + pymolProcess = null; + } } - // logger.warn(error); - return false; + return pymolProcess; } private int getPortNumber() @@ -295,9 +292,8 @@ public class PymolManager } } catch (Exception e) { - System.err.println( - "Failed to get REST port number from " + responses + ": " - + e.getMessage()); + Cache.log.error("Failed to get REST port number from " + responses + + ": " + e.getMessage()); // logger.error("Failed to get REST port number from " + responses + ": " // + e.getMessage()); } finally @@ -311,10 +307,10 @@ public class PymolManager } if (port == 0) { - System.err.println("Failed to start PyMOL with XMLRPC, response was: " + Cache.log.error("Failed to start PyMOL with XMLRPC, response was: " + responses); } - System.err.println("PyMOL started with XMLRPC on port " + port); + Cache.log.error("PyMOL started with XMLRPC on port " + port); return port; } diff --git a/src/jalview/ext/rbvi/chimera/ChimeraCommands.java b/src/jalview/ext/rbvi/chimera/ChimeraCommands.java index 5beee56..dd7b446 100644 --- a/src/jalview/ext/rbvi/chimera/ChimeraCommands.java +++ b/src/jalview/ext/rbvi/chimera/ChimeraCommands.java @@ -406,4 +406,11 @@ public class ChimeraCommands extends StructureCommandsBase return new StructureCommand("open chimera:" + filepath); } + @Override + public StructureCommandI closeViewer() + { + // https://www.cgl.ucsf.edu/chimera/current/docs/UsersGuide/midas/stop.html + return new StructureCommand("stop really"); + } + } diff --git a/src/jalview/ext/rbvi/chimera/ChimeraXCommands.java b/src/jalview/ext/rbvi/chimera/ChimeraXCommands.java index 9da1738..889b1bc 100644 --- a/src/jalview/ext/rbvi/chimera/ChimeraXCommands.java +++ b/src/jalview/ext/rbvi/chimera/ChimeraXCommands.java @@ -223,4 +223,11 @@ public class ChimeraXCommands extends ChimeraCommands // this version of the command has no dependency on file extension return new StructureCommand("open " + filepath + " format session"); } + + @Override + public StructureCommandI closeViewer() + { + // https://www.cgl.ucsf.edu/chimerax/docs/user/commands/exit.html + return new StructureCommand("exit"); + } } diff --git a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java index 460b156..bc4eef4 100644 --- a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java +++ b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java @@ -37,7 +37,6 @@ import ext.edu.ucsf.rbvi.strucviz2.ChimeraModel; import ext.edu.ucsf.rbvi.strucviz2.StructureManager; import ext.edu.ucsf.rbvi.strucviz2.StructureManager.ModelType; import jalview.api.AlignmentViewPanel; -import jalview.api.structures.JalviewStructureDisplayI; import jalview.bin.Cache; import jalview.datamodel.AlignmentI; import jalview.datamodel.PDBEntry; @@ -78,8 +77,6 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel String lastHighlightCommand; - private Thread chimeraMonitor; - /** * Open a PDB structure file in Chimera and set up mappings from Jalview. * @@ -186,37 +183,6 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel } /** - * Starts a thread that waits for the Chimera process to finish, so that we can - * then close the associated resources. This avoids leaving orphaned Chimera - * viewer panels in Jalview if the user closes Chimera. - */ - protected void startChimeraProcessMonitor() - { - final Process p = chimeraManager.getChimeraProcess(); - chimeraMonitor = new Thread(new Runnable() - { - - @Override - public void run() - { - try - { - p.waitFor(); - JalviewStructureDisplayI display = getViewer(); - if (display != null) - { - display.closeViewer(false); - } - } catch (InterruptedException e) - { - // exit thread if Chimera Viewer is closed in Jalview - } - } - }); - chimeraMonitor.start(); - } - - /** * Start a dedicated HttpServer to listen for Chimera notifications, and tell it * to start listening */ @@ -241,21 +207,13 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel public void closeViewer(boolean closeChimera) { super.closeViewer(closeChimera); - if (closeChimera) - { - chimeraManager.exitChimera(); - } if (this.chimeraListener != null) { chimeraListener.shutdown(); chimeraListener = null; } + chimeraManager.clearOnChimeraExit(); chimeraManager = null; - - if (chimeraMonitor != null) - { - chimeraMonitor.interrupt(); - } } /** @@ -306,7 +264,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel boolean launched = chimeraManager.launchChimera(getChimeraPaths()); if (launched) { - startChimeraProcessMonitor(); + startExternalViewerMonitor(chimeraManager.getChimeraProcess()); } else { @@ -500,7 +458,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel atomSpecs.add(spec); } catch (IllegalArgumentException e) { - System.err.println("Failed to parse atomspec: " + atomSpec); + Cache.log.error("Failed to parse atomspec: " + atomSpec); } } return atomSpecs; diff --git a/src/jalview/gui/PymolBindingModel.java b/src/jalview/gui/PymolBindingModel.java index 264a49c..538b101 100644 --- a/src/jalview/gui/PymolBindingModel.java +++ b/src/jalview/gui/PymolBindingModel.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Map; import jalview.api.AlignmentViewPanel; +import jalview.bin.Cache; import jalview.datamodel.PDBEntry; import jalview.datamodel.SequenceI; import jalview.ext.pymol.PymolCommands; @@ -29,8 +30,6 @@ public class PymolBindingModel extends AAStructureBindingModel private PymolManager pymolManager; - private Thread pymolMonitor; - /* * full paths to structure files opened in PyMOL */ @@ -139,16 +138,7 @@ public class PymolBindingModel extends AAStructureBindingModel public void closeViewer(boolean closePymol) { super.closeViewer(closePymol); - if (closePymol) - { - pymolManager.exitPymol(); - } pymolManager = null; - - if (pymolMonitor != null) - { - pymolMonitor.interrupt(); - } } public boolean launchPymol() @@ -158,16 +148,17 @@ public class PymolBindingModel extends AAStructureBindingModel return true; } - boolean launched = pymolManager.launchPymol(); - if (launched) + Process pymol = pymolManager.launchPymol(); + if (pymol != null) { // start listening for PyMOL selections - how?? + startExternalViewerMonitor(pymol); } else { - System.err.println("Failed to launch PyMOL!"); + Cache.log.error("Failed to launch PyMOL!"); } - return launched; + return pymol != null; } public void openFile(PDBEntry pe) diff --git a/src/jalview/structure/StructureCommandsI.java b/src/jalview/structure/StructureCommandsI.java index 5a0db0a..3a47f83 100644 --- a/src/jalview/structure/StructureCommandsI.java +++ b/src/jalview/structure/StructureCommandsI.java @@ -166,4 +166,10 @@ public interface StructureCommandsI * @return */ StructureCommandI openSession(String filepath); + + /** + * Returns a command to ask the viewer to close down + * @return + */ + StructureCommandI closeViewer(); } diff --git a/src/jalview/structures/models/AAStructureBindingModel.java b/src/jalview/structures/models/AAStructureBindingModel.java index 5949847..05cfd5a 100644 --- a/src/jalview/structures/models/AAStructureBindingModel.java +++ b/src/jalview/structures/models/AAStructureBindingModel.java @@ -175,6 +175,8 @@ public abstract class AAStructureBindingModel public String fileLoadingError; + protected Thread externalViewerMonitor; + /** * Constructor * @@ -1109,40 +1111,22 @@ public abstract class AAStructureBindingModel boolean getReply); /** - * A helper method that converts list of commands to a vararg array - * - * @param commands - * @param getReply - * @param msg - */ - protected List executeCommands( - List commands, - boolean getReply, String msg) - { - return executeCommands(getReply, msg, - commands.toArray(new StructureCommandI[commands.size()])); - } - - /** * Executes one or more structure viewer commands. If a progress message is * provided, it is shown first, and removed after all commands have been run. * + * @param commands * @param getReply * @param msg - * @param commands - * @return */ - protected List executeCommands(boolean getReply, String msg, - StructureCommandI[] commands) + protected List executeCommands(List commands, + boolean getReply, String msg) { - // todo: tidy this up - /* * show progress message if specified */ final JalviewStructureDisplayI theViewer = getViewer(); final long handle = msg == null ? 0 : theViewer.startProgressBar(msg); - + List response = getReply ? new ArrayList<>() : null; try { @@ -1583,7 +1567,7 @@ public abstract class AAStructureBindingModel /** * Closes Jalview's structure viewer panel and releases associated resources. * If it is managing an external viewer program, and {@code forceClose} is - * true, also shuts down that program. + * true, also asks that program to close. * * @param forceClose */ @@ -1592,8 +1576,23 @@ public abstract class AAStructureBindingModel getSsm().removeStructureViewerListener(this, this.getStructureFiles()); releaseUIResources(); - // add external viewer shutdown in overrides - // todo - or can maybe pull up to here + /* + * end the thread that closes this panel if the external viewer closes + */ + if (externalViewerMonitor != null) + { + externalViewerMonitor.interrupt(); + externalViewerMonitor = null; + } + + if (forceClose) + { + StructureCommandI cmd = getCommandGenerator().closeViewer(); + if (cmd != null) + { + executeCommand(cmd, false); + } + } } /** @@ -1868,4 +1867,37 @@ public abstract class AAStructureBindingModel String[] files = getStructureFiles(); return files == null ? 0 : files.length; } + + /** + * Starts a thread that waits for the external viewer program process to + * finish, so that we can then close the associated resources. This avoids + * leaving orphaned viewer panels in Jalview if the user closes the external + * viewer. + * + * @param p + */ + protected void startExternalViewerMonitor(Process p) + { + externalViewerMonitor = new Thread(new Runnable() + { + + @Override + public void run() + { + try + { + p.waitFor(); + JalviewStructureDisplayI display = getViewer(); + if (display != null) + { + display.closeViewer(false); + } + } catch (InterruptedException e) + { + // exit thread if Chimera Viewer is closed in Jalview + } + } + }); + externalViewerMonitor.start(); + } } -- 1.7.10.2