JAL-3674 more finessed asynchronous structure commands
authorgmungoc <g.m.carstairs@dundee.ac.uk>
Mon, 29 Jun 2020 15:53:51 +0000 (16:53 +0100)
committergmungoc <g.m.carstairs@dundee.ac.uk>
Mon, 29 Jun 2020 15:53:51 +0000 (16:53 +0100)
src/ext/edu/ucsf/rbvi/strucviz2/ChimeraManager.java
src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
src/jalview/gui/JalviewChimeraBindingModel.java
src/jalview/structures/models/AAStructureBindingModel.java

index 375fa4a..d7e7937 100644 (file)
@@ -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
    * <p>
    * Method extracted for Jalview to allow override in ChimeraXManager
+   * 
    * @param args
    */
   protected void addLaunchArguments(List<String> 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<String> 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()
index 549636b..66420b0 100644 (file)
@@ -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 "
index cef92a8..689106a 100644 (file)
@@ -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
index 1afa15e..16dd96c 100644 (file)
@@ -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.
-   * <p>
-   * 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<String> executeCommand(StructureCommandI cmd,
+          boolean getReply);
+
+  /**
+   * Executes one or more structure viewer commands
+   * 
+   * @param commands
+   * @param getReply
    * @param msg
-   * @return
    */
-  private List<String> executeCommand(StructureCommandI cmd,
+  protected List<String> executeCommands(List<StructureCommandI> 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.
+   * <p>
+   * 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<String> 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<String> response = new ArrayList<>();
       try
       {
-        return executeCommand(cmd, true);
+        for (StructureCommandI cmd : cmds)
+        {
+          List<String> 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<String> executeCommand(StructureCommandI cmd,
-          boolean getReply);
-
-  /**
-   * Executes one or more structure viewer commands
-   * 
-   * @param commands
-   * @param getReply
-   * @param msg
-   */
-  protected List<String> executeCommands(List<StructureCommandI> commands,
-          boolean getReply, String msg)
-  {
-    List<String> response = getReply ? new ArrayList<>() : null;
-    for (StructureCommandI cmd : commands)
-    {
-      List<String> 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