From aad3cb1e3d4250bebbf709624e3e15530a377df0 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Mon, 29 Jun 2020 16:53:51 +0100 Subject: [PATCH] JAL-3674 more finessed asynchronous structure commands --- .../edu/ucsf/rbvi/strucviz2/ChimeraManager.java | 66 ++++---- .../ext/rbvi/chimera/JalviewChimeraBinding.java | 13 +- src/jalview/gui/JalviewChimeraBindingModel.java | 1 - .../structures/models/AAStructureBindingModel.java | 164 ++++++++------------ 4 files changed, 112 insertions(+), 132 deletions(-) diff --git a/src/ext/edu/ucsf/rbvi/strucviz2/ChimeraManager.java b/src/ext/edu/ucsf/rbvi/strucviz2/ChimeraManager.java index 375fa4a..d7e7937 100644 --- a/src/ext/edu/ucsf/rbvi/strucviz2/ChimeraManager.java +++ b/src/ext/edu/ucsf/rbvi/strucviz2/ChimeraManager.java @@ -174,10 +174,11 @@ public class ChimeraManager return hasChimeraModel(modelNubmer, 0); } - public boolean hasChimeraModel(Integer modelNubmer, Integer subModelNumber) + public boolean hasChimeraModel(Integer modelNubmer, + Integer subModelNumber) { - return currentModelsMap.containsKey(ChimUtils.makeModelKey(modelNubmer, - subModelNumber)); + return currentModelsMap.containsKey( + ChimUtils.makeModelKey(modelNubmer, subModelNumber)); } public void addChimeraModel(Integer modelNumber, Integer subModelNumber, @@ -187,7 +188,8 @@ public class ChimeraManager ChimUtils.makeModelKey(modelNumber, subModelNumber), model); } - public void removeChimeraModel(Integer modelNumber, Integer subModelNumber) + public void removeChimeraModel(Integer modelNumber, + Integer subModelNumber) { int modelKey = ChimUtils.makeModelKey(modelNumber, subModelNumber); if (currentModelsMap.containsKey(modelKey)) @@ -242,9 +244,8 @@ public class ChimeraManager if (!modelList.contains(newModel)) { newModel.setModelName(modelName); - sendChimeraCommand( - "setattr M name " + modelName + " #" - + newModel.getModelNumber(), false); + sendChimeraCommand("setattr M name " + modelName + " #" + + newModel.getModelNumber(), false); modelList.add(newModel); } } @@ -314,8 +315,8 @@ public class ChimeraManager { sendChimeraCommand("close " + model.toSpec(), false); // currentModelNamesMap.remove(model.getModelName()); - currentModelsMap.remove(ChimUtils.makeModelKey( - model.getModelNumber(), model.getSubModelNumber())); + currentModelsMap.remove(ChimUtils.makeModelKey(model.getModelNumber(), + model.getSubModelNumber())); // selectionList.remove(chimeraModel); } else @@ -328,7 +329,8 @@ public class ChimeraManager public void startListening() { - sendChimeraCommand("listen start models; listen start selection", false); + sendChimeraCommand("listen start models; listen start selection", + false); } public void stopListening() @@ -602,8 +604,8 @@ public class ChimeraManager if (error.length() == 0) { this.chimeraRestPort = getPortNumber(); - System.out.println("Chimera REST API started on port " - + chimeraRestPort); + System.out.println( + "Chimera REST API started on port " + chimeraRestPort); // structureManager.initChimTable(); structureManager.setChimeraPathProperty(workingPath); // TODO: [Optional] Check Chimera version and show a warning if below 1.8 @@ -621,6 +623,7 @@ public class ChimeraManager * Adds command-line arguments to start the REST server *

* Method extracted for Jalview to allow override in ChimeraXManager + * * @param args */ protected void addLaunchArguments(List args) @@ -636,8 +639,8 @@ public class ChimeraManager { int port = 0; InputStream readChan = chimera.getInputStream(); - BufferedReader lineReader = new BufferedReader(new InputStreamReader( - readChan)); + BufferedReader lineReader = new BufferedReader( + new InputStreamReader(readChan)); StringBuilder responses = new StringBuilder(); try { @@ -667,8 +670,8 @@ public class ChimeraManager } } catch (Exception e) { - logger.error("Failed to get REST port number from " + responses - + ": " + e.getMessage()); + logger.error("Failed to get REST port number from " + responses + ": " + + e.getMessage()); } finally { try @@ -680,11 +683,12 @@ public class ChimeraManager } if (port == 0) { - System.err - .println("Failed to start Chimera with REST service, response was: " + System.err.println( + "Failed to start Chimera with REST service, response was: " + responses); } - logger.info("Chimera REST service listening on port " + chimeraRestPort); + logger.info( + "Chimera REST service listening on port " + chimeraRestPort); return port; } @@ -770,8 +774,8 @@ public class ChimeraManager String[] lineParts = inputLine.split("\\s"); if (lineParts.length == 5) { - ChimeraResidue residue = ChimUtils - .getResidue(lineParts[2], model); + ChimeraResidue residue = ChimUtils.getResidue(lineParts[2], + model); String value = lineParts[4]; if (residue != null) { @@ -814,7 +818,8 @@ public class ChimeraManager */ public List sendChimeraCommand(String command, boolean reply) { - if (debug) { + if (debug) + { System.out.println("chimeradebug>> " + command); } if (!isChimeraLaunched() || command == null @@ -822,12 +827,18 @@ public class ChimeraManager { return null; } - // TODO do we need a maximum wait time before aborting? - while (busy) + /* + * set a maximum wait time before trying anyway + * to avoid hanging indefinitely + */ + int waited = 0; + int pause = 25; + while (busy && waited < 1001) { try { - Thread.sleep(25); + Thread.sleep(pause); + waited += pause; } catch (InterruptedException q) { } @@ -849,7 +860,6 @@ public class ChimeraManager + (System.currentTimeMillis() - startTime) + "ms: " + command); } - } } @@ -904,7 +914,9 @@ public class ChimeraManager } /** - * Returns "POST" as the HTTP request method to use for REST service calls to Chimera + * Returns "POST" as the HTTP request method to use for REST service calls to + * Chimera + * * @return */ protected String getHttpRequestMethod() diff --git a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java index 549636b..66420b0 100644 --- a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java +++ b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java @@ -27,7 +27,6 @@ import java.io.PrintWriter; import java.net.BindException; import java.util.ArrayList; import java.util.Collections; -import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -38,7 +37,6 @@ import ext.edu.ucsf.rbvi.strucviz2.StructureManager; import ext.edu.ucsf.rbvi.strucviz2.StructureManager.ModelType; import jalview.api.AlignmentViewPanel; import jalview.bin.Cache; -import jalview.datamodel.AlignmentI; import jalview.datamodel.PDBEntry; import jalview.datamodel.SearchResultMatchI; import jalview.datamodel.SearchResultsI; @@ -426,11 +424,11 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel */ if (lastHighlightCommand != null) { - chimeraManager.sendChimeraCommand("~" + lastHighlightCommand, false); + executeCommand(false, null, new StructureCommand("~" + lastHighlightCommand)); } if (found) { - chimeraManager.sendChimeraCommand(command, false); + executeCommand(false, null, new StructureCommand(command)); } this.lastHighlightCommand = command; } @@ -572,10 +570,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel } else { - for (StructureCommandI command : commands) - { - sendAsynchronousCommand(command, null); - } + executeCommands(commands, false, null); } return commands.size(); } @@ -603,7 +598,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel String path = tmp.getAbsolutePath(); StructureCommandI command = getCommandGenerator() .openCommandFile(path); - sendAsynchronousCommand(command, null); + executeCommand(false, null, command); } catch (IOException e) { System.err.println("Sending commands to Chimera via file failed with " diff --git a/src/jalview/gui/JalviewChimeraBindingModel.java b/src/jalview/gui/JalviewChimeraBindingModel.java index cef92a8..689106a 100644 --- a/src/jalview/gui/JalviewChimeraBindingModel.java +++ b/src/jalview/gui/JalviewChimeraBindingModel.java @@ -29,7 +29,6 @@ import jalview.datamodel.PDBEntry; import jalview.datamodel.SequenceI; import jalview.ext.rbvi.chimera.JalviewChimeraBinding; import jalview.io.DataSourceType; -import jalview.structure.AtomSpec; import jalview.structure.StructureSelectionManager; public class JalviewChimeraBindingModel extends JalviewChimeraBinding diff --git a/src/jalview/structures/models/AAStructureBindingModel.java b/src/jalview/structures/models/AAStructureBindingModel.java index 1afa15e..16dd96c 100644 --- a/src/jalview/structures/models/AAStructureBindingModel.java +++ b/src/jalview/structures/models/AAStructureBindingModel.java @@ -57,7 +57,6 @@ import jalview.schemes.ColourSchemeI; import jalview.schemes.ResidueProperties; import jalview.structure.AtomSpec; import jalview.structure.AtomSpecModel; -import jalview.structure.StructureCommand; import jalview.structure.StructureCommandI; import jalview.structure.StructureCommandsI; import jalview.structure.StructureListener; @@ -981,8 +980,8 @@ public abstract class AAStructureBindingModel // TODO: JAL-628 colour chains distinctly across all visible models - executeCommand(commandGenerator.colourByChain(), false, - COLOURING_STRUCTURES); + executeCommand(false, COLOURING_STRUCTURES, + commandGenerator.colourByChain()); } /** @@ -1040,34 +1039,66 @@ public abstract class AAStructureBindingModel public void setBackgroundColour(Color col) { StructureCommandI cmd = commandGenerator.setBackgroundColour(col); - executeCommand(cmd, false, null); + executeCommand(false, null, cmd); } /** - * Sends one command to the structure viewer. If {@code getReply} is true, the - * command is sent synchronously, otherwise in a deferred thread. - *

- * If a progress message is supplied, this is displayed before command - * execution, and removed afterwards. + * Execute one structure viewer command. If {@code getReply} is true, may + * optionally return one or more reply messages, else returns null. * * @param cmd * @param getReply + */ + protected abstract List executeCommand(StructureCommandI cmd, + boolean getReply); + + /** + * Executes one or more structure viewer commands + * + * @param commands + * @param getReply * @param msg - * @return */ - private List executeCommand(StructureCommandI cmd, + protected List executeCommands(List commands, boolean getReply, String msg) { - final JalviewStructureDisplayI theViewer = getViewer(); + return executeCommand(getReply, msg, + commands.toArray(new StructureCommandI[commands.size()])); + } + + /** + * Executes one or more structure viewer commands, optionally returning the + * reply, and optionally showing a status message while the command is being + * executed. + *

+ * If a reply is wanted, the execution is done synchronously (waits), + * otherwise it is done in a separate thread (doesn't wait). + * + * @param getReply + * @param msg + * @param cmds + * @return + */ + protected List executeCommand(boolean getReply, String msg, + StructureCommandI... cmds) + { + JalviewStructureDisplayI theViewer = getViewer(); final long handle = msg == null ? 0 : theViewer.startProgressBar(msg); + if (getReply) { /* - * synchronous (same thread) execution so reply can be returned + * execute and wait for reply */ + List response = new ArrayList<>(); try { - return executeCommand(cmd, true); + for (StructureCommandI cmd : cmds) + { + List replies = executeCommand(cmd, true); + response.addAll(replies); + } + return response; } finally { if (msg != null) @@ -1076,62 +1107,39 @@ public abstract class AAStructureBindingModel } } } - else + + /* + * fire and forget + */ + String threadName = msg == null ? "StructureCommand" : msg; + new Thread(new Runnable() { - /* - * asynchronous (new thread) execution if no reply needed - */ - SwingUtilities.invokeLater(new Runnable() + @Override + public void run() { - @Override - public void run() + try { - try + for (StructureCommandI cmd : cmds) { executeCommand(cmd, false); - } finally + } + } finally + { + if (msg != null) { - if (msg != null) + SwingUtilities.invokeLater(new Runnable() { - theViewer.stopProgressBar(null, handle); - } + @Override + public void run() + { + theViewer.stopProgressBar(null, handle); + } + }); } } - }); - return null; - } - } - - /** - * Execute one structure viewer command. If {@code getReply} is true, may - * optionally return one or more reply messages, else returns null. - * - * @param cmd - * @param getReply - */ - protected abstract List executeCommand(StructureCommandI cmd, - boolean getReply); - - /** - * Executes one or more structure viewer commands - * - * @param commands - * @param getReply - * @param msg - */ - protected List executeCommands(List commands, - boolean getReply, String msg) - { - List response = getReply ? new ArrayList<>() : null; - for (StructureCommandI cmd : commands) - { - List replies = executeCommand(cmd, getReply, msg); - if (replies != null) - { - response.addAll(replies); } - } - return response; + }, threadName).start(); + return null; } /** @@ -1157,7 +1165,7 @@ public abstract class AAStructureBindingModel */ public void focusView() { - executeCommand(commandGenerator.focusView(), false, null); + executeCommand(false, null, commandGenerator.focusView()); } /** @@ -1276,40 +1284,6 @@ public abstract class AAStructureBindingModel protected abstract ViewerType getViewerType(); /** - * Send a structure viewer command asynchronously in a new thread. If the - * progress message is not null, display this message while the command is - * executing. - * - * @param command - * @param progressMsg - */ - protected void sendAsynchronousCommand(StructureCommandI command, - String progressMsg) - { - final JalviewStructureDisplayI theViewer = getViewer(); - final long handle = progressMsg == null ? 0 - : theViewer.startProgressBar(progressMsg); - SwingUtilities.invokeLater(new Runnable() - { - @Override - public void run() - { - try - { - executeCommand(command, false, null); - } finally - { - if (progressMsg != null) - { - theViewer.stopProgressBar(null, handle); - } - } - } - }); - - } - - /** * Builds a data structure which records mapped structure residues for each * colour. From this we can easily generate the viewer commands for colour by * sequence. Constructs and returns a map of {@code Color} to -- 1.7.10.2