JAL-3949 Complete new abstracted logging framework in jalview.log. Updated log calls...
[jalview.git] / src / jalview / ext / rbvi / chimera / JalviewChimeraBinding.java
index 40b6059..d2c991f 100644 (file)
  */
 package jalview.ext.rbvi.chimera;
 
-import jalview.api.AlignmentViewPanel;
-import jalview.api.FeatureRenderer;
-import jalview.api.SequenceRenderer;
-import jalview.bin.Cache;
-import jalview.datamodel.AlignmentI;
-import jalview.datamodel.ColumnSelection;
-import jalview.datamodel.PDBEntry;
-import jalview.datamodel.SequenceI;
-import jalview.httpserver.AbstractRequestHandler;
-import jalview.schemes.ColourSchemeI;
-import jalview.schemes.ResidueProperties;
-import jalview.structure.AtomSpec;
-import jalview.structure.StructureMappingcommandSet;
-import jalview.structure.StructureSelectionManager;
-import jalview.structures.models.AAStructureBindingModel;
-import jalview.util.MessageManager;
-
-import java.awt.Color;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.PrintWriter;
 import java.net.BindException;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -48,25 +35,33 @@ import ext.edu.ucsf.rbvi.strucviz2.ChimeraManager;
 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.bin.Cache;
+import jalview.datamodel.PDBEntry;
+import jalview.datamodel.SearchResultMatchI;
+import jalview.datamodel.SearchResultsI;
+import jalview.datamodel.SequenceFeature;
+import jalview.datamodel.SequenceI;
+import jalview.gui.StructureViewer.ViewerType;
+import jalview.httpserver.AbstractRequestHandler;
+import jalview.io.DataSourceType;
+import jalview.structure.AtomSpec;
+import jalview.structure.AtomSpecModel;
+import jalview.structure.StructureCommand;
+import jalview.structure.StructureCommandI;
+import jalview.structure.StructureSelectionManager;
+import jalview.structures.models.AAStructureBindingModel;
 
 public abstract class JalviewChimeraBinding extends AAStructureBindingModel
 {
-  // Chimera clause to exclude alternate locations in atom selection
-  private static final String NO_ALTLOCS = "&~@.B-Z&~@.2-9";
-
-  private static final String COLOURING_CHIMERA = MessageManager
-          .getString("status.colouring_chimera");
-
-  private static final boolean debug = false;
-
-  private static final String PHOSPHORUS = "P";
+  public static final String CHIMERA_SESSION_EXTENSION = ".py";
 
-  private static final String ALPHACARBON = "CA";
+  public static final String CHIMERA_FEATURE_GROUP = "Chimera";
 
   /*
    * Object through which we talk to Chimera
    */
-  private ChimeraManager viewer;
+  private ChimeraManager chimeraManager;
 
   /*
    * Object which listens to Chimera notifications
@@ -74,47 +69,27 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
   private AbstractRequestHandler chimeraListener;
 
   /*
-   * set if chimera state is being restored from some source - instructs binding
-   * not to apply default display style when structure set is updated for first
-   * time.
-   */
-  private boolean loadingFromArchive = false;
-
-  /*
-   * flag to indicate if the Chimera viewer should ignore sequence colouring
-   * events from the structure manager because the GUI is still setting up
-   */
-  private boolean loadingFinished = true;
-
-  public String fileLoadingError;
-
-  /*
    * Map of ChimeraModel objects keyed by PDB full local file name
    */
-  private Map<String, List<ChimeraModel>> chimeraMaps = new LinkedHashMap<String, List<ChimeraModel>>();
-
-  /*
-   * the default or current model displayed if the model cannot be identified
-   * from the selection message
-   */
-  private int frameNo = 0;
-
-  private String lastCommand;
+  protected Map<String, List<ChimeraModel>> chimeraMaps = new LinkedHashMap<>();
 
   String lastHighlightCommand;
 
-  /*
-   * incremented every time a load notification is successfully handled -
-   * lightweight mechanism for other threads to detect when they can start
-   * referring to new structures.
+  /**
+   * Returns a model of the structure positions described by the Chimera format atomspec
+   * @param atomSpec
+   * @return
    */
-  private long loadNotifiesHandled = 0;
+  protected  AtomSpec parseAtomSpec(String atomSpec)
+  {
+    return AtomSpec.fromChimeraAtomspec(atomSpec);
+  }
 
   /**
    * Open a PDB structure file in Chimera and set up mappings from Jalview.
    * 
-   * We check if the PDB model id is already loaded in Chimera, if so don't
-   * reopen it. This is the case if Chimera has opened a saved session file.
+   * We check if the PDB model id is already loaded in Chimera, if so don't reopen
+   * it. This is the case if Chimera has opened a saved session file.
    * 
    * @param pe
    * @return
@@ -124,8 +99,8 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
     String file = pe.getFile();
     try
     {
-      List<ChimeraModel> modelsToMap = new ArrayList<ChimeraModel>();
-      List<ChimeraModel> oldList = viewer.getModelList();
+      List<ChimeraModel> modelsToMap = new ArrayList<>();
+      List<ChimeraModel> oldList = chimeraManager.getModelList();
       boolean alreadyOpen = false;
 
       /*
@@ -146,16 +121,8 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
        */
       if (!alreadyOpen)
       {
-        viewer.openModel(file, pe.getId(), ModelType.PDB_MODEL);
-        List<ChimeraModel> newList = viewer.getModelList();
-        // JAL-1728 newList.removeAll(oldList) does not work
-        for (ChimeraModel cm : newList)
-        {
-          if (cm.getModelName().equals(pe.getId()))
-          {
-            modelsToMap.add(cm);
-          }
-        }
+        chimeraManager.openModel(file, pe.getId(), ModelType.PDB_MODEL);
+        addChimeraModel(pe, modelsToMap);
       }
 
       chimeraMaps.put(file, modelsToMap);
@@ -163,13 +130,6 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
       if (getSsm() != null)
       {
         getSsm().addStructureViewerListener(this);
-        // ssm.addSelectionListener(this);
-        FeatureRenderer fr = getFeatureRenderer(null);
-        if (fr != null)
-        {
-          fr.featuresAdded();
-        }
-        refreshGUI();
       }
       return true;
     } catch (Exception q)
@@ -182,350 +142,97 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
   }
 
   /**
+   * Adds the ChimeraModel corresponding to the given PDBEntry, based on model
+   * name matching PDB id
+   * 
+   * @param pe
+   * @param modelsToMap
+   */
+  protected void addChimeraModel(PDBEntry pe,
+          List<ChimeraModel> modelsToMap)
+  {
+    /*
+     * Chimera: query for actual models and find the one with
+     * matching model name - already set in viewer.openModel()
+     */
+    List<ChimeraModel> newList = chimeraManager.getModelList();
+    // JAL-1728 newList.removeAll(oldList) does not work
+    for (ChimeraModel cm : newList)
+    {
+      if (cm.getModelName().equals(pe.getId()))
+      {
+        modelsToMap.add(cm);
+      }
+    }
+  }
+
+  /**
    * Constructor
    * 
    * @param ssm
    * @param pdbentry
    * @param sequenceIs
-   * @param chains
    * @param protocol
    */
   public JalviewChimeraBinding(StructureSelectionManager ssm,
-          PDBEntry[] pdbentry, SequenceI[][] sequenceIs, String[][] chains,
-          String protocol)
+          PDBEntry[] pdbentry, SequenceI[][] sequenceIs,
+          DataSourceType protocol)
+  {
+    super(ssm, pdbentry, sequenceIs, protocol);
+    boolean chimeraX = ViewerType.CHIMERAX.equals(getViewerType());
+    chimeraManager = chimeraX ? new ChimeraXManager(new StructureManager(true)) : new ChimeraManager(new StructureManager(true));
+    setStructureCommands(chimeraX ? new ChimeraXCommands() : new ChimeraCommands());
+  }
+
+  @Override
+  protected ViewerType getViewerType()
   {
-    super(ssm, pdbentry, sequenceIs, chains, protocol);
-    viewer = new ChimeraManager(
-            new ext.edu.ucsf.rbvi.strucviz2.StructureManager(true));
+    return ViewerType.CHIMERA;
   }
 
   /**
-   * Start a dedicated HttpServer to listen for Chimera notifications, and tell
-   * it to start listening
+   * Start a dedicated HttpServer to listen for Chimera notifications, and tell it
+   * to start listening
    */
   public void startChimeraListener()
   {
     try
     {
       chimeraListener = new ChimeraListener(this);
-      viewer.startListening(chimeraListener.getUri());
+      startListening(chimeraListener.getUri());
     } catch (BindException e)
     {
-      System.err.println("Failed to start Chimera listener: "
-              + e.getMessage());
-    }
-  }
-
-  /**
-   * Construct a title string for the viewer window based on the data Jalview
-   * knows about
-   * 
-   * @param verbose
-   * @return
-   */
-  public String getViewerTitle(boolean verbose)
-  {
-    return getViewerTitle("Chimera", verbose);
-  }
-
-  /**
-   * Tells Chimera to display only the specified chains
-   * 
-   * @param toshow
-   */
-  public void showChains(List<String> toshow)
-  {
-    /*
-     * Construct a chimera command like
-     * 
-     * ~display #*;~ribbon #*;ribbon :.A,:.B
-     */
-    StringBuilder cmd = new StringBuilder(64);
-    boolean first = true;
-    for (String chain : toshow)
-    {
-      if (!first)
-      {
-        cmd.append(",");
-      }
-      cmd.append(":.").append(chain);
-      first = false;
+      System.err.println(
+              "Failed to start Chimera listener: " + e.getMessage());
     }
-
-    /*
-     * could append ";focus" to this command to resize the display to fill the
-     * window, but it looks more helpful not to (easier to relate chains to the
-     * whole)
-     */
-    final String command = "~display #*; ~ribbon #*; ribbon "
-            + cmd.toString();
-    sendChimeraCommand(command, false);
   }
 
   /**
    * Close down the Jalview viewer and listener, and (optionally) the associated
    * Chimera window.
    */
+  @Override
   public void closeViewer(boolean closeChimera)
   {
-    getSsm().removeStructureViewerListener(this, this.getPdbFile());
-    if (closeChimera)
-    {
-      viewer.exitChimera();
-    }
+    super.closeViewer(closeChimera);
     if (this.chimeraListener != null)
     {
       chimeraListener.shutdown();
       chimeraListener = null;
     }
-    lastCommand = null;
-    viewer = null;
-
-    releaseUIResources();
-  }
-
-  public void colourByChain()
-  {
-    colourBySequence = false;
-    sendAsynchronousCommand("rainbow chain", COLOURING_CHIMERA);
-  }
-
-  /**
-   * Constructs and sends a Chimera command to colour by charge
-   * <ul>
-   * <li>Aspartic acid and Glutamic acid (negative charge) red</li>
-   * <li>Lysine and Arginine (positive charge) blue</li>
-   * <li>Cysteine - yellow</li>
-   * <li>all others - white</li>
-   * </ul>
-   */
-  public void colourByCharge()
-  {
-    colourBySequence = false;
-    String command = "color white;color red ::ASP;color red ::GLU;color blue ::LYS;color blue ::ARG;color yellow ::CYS";
-    sendAsynchronousCommand(command, COLOURING_CHIMERA);
-  }
-
-  /**
-   * Construct and send a command to align structures against a reference
-   * structure, based on one or more sequence alignments
-   * 
-   * @param _alignment
-   *          an array of alignments to process
-   * @param _refStructure
-   *          an array of corresponding reference structures (index into pdb
-   *          file array); if a negative value is passed, the first PDB file
-   *          mapped to an alignment sequence is used as the reference for
-   *          superposition
-   * @param _hiddenCols
-   *          an array of corresponding hidden columns for each alignment
-   */
-  public void superposeStructures(AlignmentI[] _alignment,
-          int[] _refStructure, ColumnSelection[] _hiddenCols)
-  {
-    StringBuilder allComs = new StringBuilder(128);
-    String[] files = getPdbFile();
-
-    if (!waitForFileLoad(files))
-    {
-      return;
-    }
-
-    refreshPdbEntries();
-    StringBuilder selectioncom = new StringBuilder(256);
-    for (int a = 0; a < _alignment.length; a++)
-    {
-      int refStructure = _refStructure[a];
-      AlignmentI alignment = _alignment[a];
-      ColumnSelection hiddenCols = _hiddenCols[a];
-
-      if (refStructure >= files.length)
-      {
-        System.err.println("Ignoring invalid reference structure value "
-                + refStructure);
-        refStructure = -1;
-      }
-
-      /*
-       * 'matched' array will hold 'true' for visible alignment columns where
-       * all sequences have a residue with a mapping to the PDB structure
-       */
-      boolean matched[] = new boolean[alignment.getWidth()];
-      for (int m = 0; m < matched.length; m++)
-      {
-        matched[m] = (hiddenCols != null) ? hiddenCols.isVisible(m) : true;
-      }
-
-      SuperposeData[] structures = new SuperposeData[files.length];
-      for (int f = 0; f < files.length; f++)
-      {
-        structures[f] = new SuperposeData(alignment.getWidth());
-      }
-
-      /*
-       * Calculate the superposable alignment columns ('matched'), and the
-       * corresponding structure residue positions (structures.pdbResNo)
-       */
-      int candidateRefStructure = findSuperposableResidues(alignment,
-              matched, structures);
-      if (refStructure < 0)
-      {
-        /*
-         * If no reference structure was specified, pick the first one that has
-         * a mapping in the alignment
-         */
-        refStructure = candidateRefStructure;
-      }
-
-      int nmatched = 0;
-      for (boolean b : matched)
-      {
-        if (b)
-        {
-          nmatched++;
-        }
-      }
-      if (nmatched < 4)
-      {
-        // TODO: bail out here because superposition illdefined?
-      }
-
-      /*
-       * Generate select statements to select regions to superimpose structures
-       */
-      String[] selcom = new String[files.length];
-      for (int pdbfnum = 0; pdbfnum < files.length; pdbfnum++)
-      {
-        String chainCd = "." + structures[pdbfnum].chain;
-        int lpos = -1;
-        boolean run = false;
-        StringBuilder molsel = new StringBuilder();
-        for (int r = 0; r < matched.length; r++)
-        {
-          if (matched[r])
-          {
-            int pdbResNum = structures[pdbfnum].pdbResNo[r];
-            if (lpos != pdbResNum - 1)
-            {
-              /*
-               * discontiguous - append last residue now
-               */
-              if (lpos != -1)
-              {
-                molsel.append(String.valueOf(lpos));
-                molsel.append(chainCd);
-                molsel.append(",");
-              }
-              run = false;
-            }
-            else
-            {
-              /*
-               * extending a contiguous run
-               */
-              if (!run)
-              {
-                /*
-                 * start the range selection
-                 */
-                molsel.append(String.valueOf(lpos));
-                molsel.append("-");
-              }
-              run = true;
-            }
-            lpos = pdbResNum;
-          }
-        }
-
-        /*
-         * and terminate final selection
-         */
-        if (lpos != -1)
-        {
-          molsel.append(String.valueOf(lpos));
-          molsel.append(chainCd);
-        }
-        if (molsel.length() > 1)
-        {
-          selcom[pdbfnum] = molsel.toString();
-          selectioncom.append("#").append(String.valueOf(pdbfnum))
-                  .append(":");
-          selectioncom.append(selcom[pdbfnum]);
-          selectioncom.append(" ");
-          if (pdbfnum < files.length - 1)
-          {
-            selectioncom.append("| ");
-          }
-        }
-        else
-        {
-          selcom[pdbfnum] = null;
-        }
-      }
-
-      StringBuilder command = new StringBuilder(256);
-      for (int pdbfnum = 0; pdbfnum < files.length; pdbfnum++)
-      {
-        if (pdbfnum == refStructure || selcom[pdbfnum] == null
-                || selcom[refStructure] == null)
-        {
-          continue;
-        }
-        if (command.length() > 0)
-        {
-          command.append(";");
-        }
-
-        /*
-         * Form Chimera match command, from the 'new' structure to the
-         * 'reference' structure e.g. (50 residues, chain B/A, alphacarbons):
-         * 
-         * match #1:1-30.B,81-100.B@CA #0:21-40.A,61-90.A@CA
-         * 
-         * @see
-         * https://www.cgl.ucsf.edu/chimera/docs/UsersGuide/midas/match.html
-         */
-        command.append("match ").append(getModelSpec(pdbfnum)).append(":");
-        command.append(selcom[pdbfnum]);
-        command.append("@").append(
-                structures[pdbfnum].isRna ? PHOSPHORUS : ALPHACARBON);
-        // JAL-1757 exclude alternate CA locations
-        command.append(NO_ALTLOCS);
-        command.append(" ").append(getModelSpec(refStructure)).append(":");
-        command.append(selcom[refStructure]);
-        command.append("@").append(
-                structures[refStructure].isRna ? PHOSPHORUS : ALPHACARBON);
-        command.append(NO_ALTLOCS);
-      }
-      if (selectioncom.length() > 0)
-      {
-        if (debug)
-        {
-          System.out.println("Select regions:\n" + selectioncom.toString());
-          System.out.println("Superimpose command(s):\n"
-                  + command.toString());
-        }
-        allComs.append("~display all; chain @CA|P; ribbon ")
-                .append(selectioncom.toString())
-                .append(";" + command.toString());
-      }
-    }
-    if (selectioncom.length() > 0)
+    
+    /*
+     * the following call is added to avoid a stack trace error in Chimera
+     * after "stop really" is sent; Chimera > 1.14 will not need it; see also 
+     * http://plato.cgl.ucsf.edu/trac/chimera/ticket/17597
+     */
+    if (closeChimera && (getViewerType() == ViewerType.CHIMERA))
     {
-      // TODO: visually distinguish regions that were superposed
-      if (selectioncom.substring(selectioncom.length() - 1).equals("|"))
-      {
-        selectioncom.setLength(selectioncom.length() - 1);
-      }
-      if (debug)
-      {
-        System.out.println("Select regions:\n" + selectioncom.toString());
-      }
-      allComs.append("; ~display all; chain @CA|P; ribbon ")
-              .append(selectioncom.toString()).append("; focus");
-      sendChimeraCommand(allComs.toString(), false);
+      chimeraManager.getChimeraProcess().destroy();
     }
 
+    chimeraManager.clearOnChimeraExit();
+    chimeraManager = null;
   }
 
   /**
@@ -545,7 +252,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
   {
     if (pdbfnum < 0 || pdbfnum >= getPdbCount())
     {
-      return "";
+      return "#" + pdbfnum; // temp hack for ChimeraX
     }
 
     /*
@@ -554,226 +261,113 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
      * to the Chimera command 'list models type molecule', see
      * ChimeraManager.getModelList().
      */
-    List<ChimeraModel> maps = chimeraMaps.get(getPdbFile()[pdbfnum]);
+    List<ChimeraModel> maps = chimeraMaps.get(getStructureFiles()[pdbfnum]);
     boolean hasSubModels = maps != null && maps.size() > 1;
     return "#" + String.valueOf(pdbfnum) + (hasSubModels ? ".1" : "");
   }
 
   /**
    * Launch Chimera, unless an instance linked to this object is already
-   * running. Returns true if chimera is successfully launched, or already
+   * running. Returns true if Chimera is successfully launched, or already
    * running, else false.
    * 
    * @return
    */
   public boolean launchChimera()
   {
-    if (!viewer.isChimeraLaunched())
-    {
-      return viewer.launchChimera(StructureManager.getChimeraPaths());
-    }
-    if (viewer.isChimeraLaunched())
+    if (chimeraManager.isChimeraLaunched())
     {
       return true;
     }
-    log("Failed to launch Chimera!");
-    return false;
-  }
 
-  /**
-   * Answers true if the Chimera process is still running, false if ended or not
-   * started.
-   * 
-   * @return
-   */
-  public boolean isChimeraRunning()
-  {
-    return viewer.isChimeraLaunched();
-  }
-
-  /**
-   * Send a command to Chimera, and optionally log any responses.
-   * 
-   * @param command
-   * @param logResponse
-   */
-  public void sendChimeraCommand(final String command, boolean logResponse)
-  {
-    if (viewer == null)
+    boolean launched = chimeraManager.launchChimera(getChimeraPaths());
+    if (launched)
     {
-      // ? thread running after viewer shut down
-      return;
+      startExternalViewerMonitor(chimeraManager.getChimeraProcess());
     }
-    viewerCommandHistory(false);
-    if (lastCommand == null || !lastCommand.equals(command))
+    else
     {
-      // trim command or it may never find a match in the replyLog!!
-      List<String> lastReply = viewer.sendChimeraCommand(command.trim(),
-              logResponse);
-      if (logResponse && debug)
-      {
-        log("Response from command ('" + command + "') was:\n" + lastReply);
-      }
+      log("Failed to launch Chimera!");
     }
-    viewerCommandHistory(true);
-    lastCommand = command;
+    return launched;
   }
 
   /**
-   * Send a Chimera command asynchronously in a new thread. If the progress
-   * message is not null, display this message while the command is executing.
+   * Returns a list of candidate paths to the Chimera program executable
    * 
-   * @param command
-   * @param progressMsg
-   */
-  protected abstract void sendAsynchronousCommand(String command,
-          String progressMsg);
-
-  /**
-   * colour any structures associated with sequences in the given alignment
-   * using the getFeatureRenderer() and getSequenceRenderer() renderers but only
-   * if colourBySequence is enabled.
+   * @return
    */
-  public void colourBySequence(boolean showFeatures,
-          jalview.api.AlignmentViewPanel alignmentv)
+  protected List<String> getChimeraPaths()
   {
-    if (!colourBySequence || !loadingFinished)
-    {
-      return;
-    }
-    if (getSsm() == null)
-    {
-      return;
-    }
-    String[] files = getPdbFile();
-
-    SequenceRenderer sr = getSequenceRenderer(alignmentv);
-
-    FeatureRenderer fr = null;
-    if (showFeatures)
-    {
-      fr = getFeatureRenderer(alignmentv);
-    }
-    AlignmentI alignment = alignmentv.getAlignment();
-
-    StructureMappingcommandSet colourBySequenceCommands = ChimeraCommands
-            .getColourBySequenceCommand(getSsm(), files, getSequence(), sr,
-                    fr, alignment);
-    for (String command : colourBySequenceCommands.commands)
-    {
-      sendAsynchronousCommand(command, COLOURING_CHIMERA);
-    }
+    return StructureManager.getChimeraPaths(false);
   }
 
   /**
-   * @param command
+   * Answers true if the Chimera process is still running, false if ended or not
+   * started.
+   * 
+   * @return
    */
-  protected void executeWhenReady(String command)
-  {
-    waitForChimera();
-    sendChimeraCommand(command, false);
-    waitForChimera();
-  }
-
-  private void waitForChimera()
+  @Override
+  public boolean isViewerRunning()
   {
-    while (viewer != null && viewer.isBusy())
-    {
-      try
-      {
-        Thread.sleep(15);
-      } catch (InterruptedException q)
-      {
-      }
-    }
+    return chimeraManager.isChimeraLaunched();
   }
 
-  // End StructureListener
-  // //////////////////////////
-
   /**
-   * returns the current featureRenderer that should be used to colour the
-   * structures
-   * 
-   * @param alignment
+   * Send a command to Chimera, and optionally log and return any responses.
    * 
-   * @return
-   */
-  public abstract FeatureRenderer getFeatureRenderer(
-          AlignmentViewPanel alignment);
-
-  /**
-   * instruct the Jalview binding to update the pdbentries vector if necessary
-   * prior to matching the viewer's contents to the list of structure files
-   * Jalview knows about.
+   * @param command
+   * @param getResponse
    */
-  public abstract void refreshPdbEntries();
-
-  private int getModelNum(String modelFileName)
+  @Override
+  public List<String> executeCommand(final StructureCommandI command,
+          boolean getResponse)
   {
-    String[] mfn = getPdbFile();
-    if (mfn == null)
+    if (chimeraManager == null || command == null)
     {
-      return -1;
+      // ? thread running after viewer shut down
+      return null;
+    }
+    List<String> reply = null;
+    // trim command or it may never find a match in the replyLog!!
+    String cmd = command.getCommand().trim();
+    List<String> lastReply = chimeraManager
+            .sendChimeraCommand(cmd, getResponse);
+    if (getResponse)
+    {
+      reply = lastReply;
+      if (Cache.isDebugEnabled()) {
+        Cache.debug(
+              "Response from command ('" + cmd + "') was:\n" + lastReply); 
+      }
     }
-    for (int i = 0; i < mfn.length; i++)
+    else
     {
-      if (mfn[i].equalsIgnoreCase(modelFileName))
+      if (Cache.isDebugEnabled())
       {
-        return i;
+        Cache.debug("Command executed: " + cmd);
       }
     }
-    return -1;
-  }
 
-  /**
-   * map between index of model filename returned from getPdbFile and the first
-   * index of models from this file in the viewer. Note - this is not trimmed -
-   * use getPdbFile to get number of unique models.
-   */
-  private int _modelFileNameMap[];
+    return reply;
+  }
 
-  // ////////////////////////////////
-  // /StructureListener
   @Override
-  public synchronized String[] getPdbFile()
+  public synchronized String[] getStructureFiles()
   {
-    if (viewer == null)
+    if (chimeraManager == null)
     {
       return new String[0];
     }
-    // if (modelFileNames == null)
-    // {
-    // Collection<ChimeraModel> chimodels = viewer.getChimeraModels();
-    // _modelFileNameMap = new int[chimodels.size()];
-    // int j = 0;
-    // for (ChimeraModel chimodel : chimodels)
-    // {
-    // String mdlName = chimodel.getModelName();
-    // }
-    // modelFileNames = new String[j];
-    // // System.arraycopy(mset, 0, modelFileNames, 0, j);
-    // }
 
-    return chimeraMaps.keySet().toArray(
-            modelFileNames = new String[chimeraMaps.size()]);
+    return chimeraMaps.keySet()
+            .toArray(modelFileNames = new String[chimeraMaps.size()]);
   }
 
   /**
-   * returns the current sequenceRenderer that should be used to colour the
-   * structures
-   * 
-   * @param alignment
-   * 
-   * @return
-   */
-  public abstract SequenceRenderer getSequenceRenderer(
-          AlignmentViewPanel alignment);
-
-  /**
-   * Construct and send a command to highlight zero, one or more atoms. We do
-   * this by sending an "rlabel" command to show the residue label at that
-   * position.
+   * Construct and send a command to highlight zero, one or more atoms. We do this
+   * by sending an "rlabel" command to show the residue label at that position.
    */
   @Override
   public void highlightAtoms(List<AtomSpec> atoms)
@@ -783,6 +377,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
       return;
     }
 
+    boolean forChimeraX = chimeraManager.isChimeraX();
     StringBuilder cmd = new StringBuilder(128);
     boolean first = true;
     boolean found = false;
@@ -797,18 +392,26 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
       {
         if (first)
         {
-          cmd.append("rlabel #").append(cms.get(0).getModelNumber())
-                  .append(":");
+          cmd.append(forChimeraX ? "label #" : "rlabel #");
         }
         else
         {
           cmd.append(",");
         }
         first = false;
-        cmd.append(pdbResNum);
-        if (!chain.equals(" "))
+        if (forChimeraX)
+        {
+          cmd.append(cms.get(0).getModelNumber())
+                  .append("/").append(chain).append(":").append(pdbResNum);
+        }
+        else
         {
-          cmd.append(".").append(chain);
+          cmd.append(cms.get(0).getModelNumber())
+                  .append(":").append(pdbResNum);
+          if (!chain.equals(" ") && !forChimeraX)
+          {
+            cmd.append(".").append(chain);
+          }
         }
         found = true;
       }
@@ -822,19 +425,29 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
     {
       return;
     }
-
+    if (!found)
+    {
+      // not a valid residue label command, so clear
+      cmd.setLength(0);
+    }
     /*
-     * unshow the label for the previous residue
+     * prepend with command
+     * to unshow the label for the previous residue
      */
     if (lastHighlightCommand != null)
     {
-      viewer.sendChimeraCommand("~" + lastHighlightCommand, false);
+      cmd.insert(0, ";");
+      cmd.insert(0,lastHighlightCommand);
+      cmd.insert(0,"~");
+      
     }
-    if (found)
-    {
-      viewer.sendChimeraCommand(command, false);
+    if (cmd.length()>0) {
+      executeCommand(true,  null,  new StructureCommand(cmd.toString()));
+    }
+    
+    if (found) {
+      this.lastHighlightCommand = command;
     }
-    this.lastHighlightCommand = command;
   }
 
   /**
@@ -845,65 +458,103 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
     /*
      * Ask Chimera for its current selection
      */
-    List<String> selection = viewer.getSelectedResidueSpecs();
-
-    /*
-     * Parse model number, residue and chain for each selected position,
-     * formatted as #0:123.A or #1.2:87.B (#model.submodel:residue.chain)
-     */
-    List<AtomSpec> atomSpecs = new ArrayList<AtomSpec>();
-    for (String atomSpec : selection)
+    StructureCommandI command = getCommandGenerator().getSelectedResidues();
+    
+    Runnable action = new Runnable()
     {
-      int colonPos = atomSpec.indexOf(":");
-      if (colonPos == -1)
+      @Override
+      public void run()
       {
-        continue; // malformed
+        List<String> chimeraReply = executeCommand(command, true);
+        
+        List<String> selectedResidues = new ArrayList<>();
+        if (chimeraReply != null)
+        {
+          /*
+           * expect 0, 1 or more lines of the format either
+           * Chimera:
+           * residue id #0:43.A type GLY
+           * ChimeraX:
+           * residue id /A:89 name THR index 88
+           * We are only interested in the atomspec (third token of the reply)
+           */
+          for (String inputLine : chimeraReply)
+          {
+            String[] inputLineParts = inputLine.split("\\s+");
+            if (inputLineParts.length >= 5)
+            {
+              selectedResidues.add(inputLineParts[2]);
+            }
+          }
+        }
+
+        /*
+         * Parse model number, residue and chain for each selected position,
+         * formatted as #0:123.A or #1.2:87.B (#model.submodel:residue.chain)
+         */
+        List<AtomSpec> atomSpecs = convertStructureResiduesToAlignment(
+                selectedResidues);
+
+        /*
+         * Broadcast the selection (which may be empty, if the user just cleared all
+         * selections)
+         */
+        getSsm().mouseOverStructure(atomSpecs);
+        
       }
+    };
+    new Thread(action).start();
+  }
 
-      int hashPos = atomSpec.indexOf("#");
-      String modelSubmodel = atomSpec.substring(hashPos + 1, colonPos);
-      int dotPos = modelSubmodel.indexOf(".");
-      int modelId = 0;
+  /**
+   * Converts a list of Chimera(X) atomspecs to a list of AtomSpec representing the
+   * corresponding residues (if any) in Jalview
+   * 
+   * @param structureSelection
+   * @return
+   */
+  protected List<AtomSpec> convertStructureResiduesToAlignment(
+          List<String> structureSelection)
+  {
+    List<AtomSpec> atomSpecs = new ArrayList<>();
+    for (String atomSpec : structureSelection)
+    {
       try
       {
-        modelId = Integer.valueOf(dotPos == -1 ? modelSubmodel
-                : modelSubmodel.substring(0, dotPos));
-      } catch (NumberFormatException e)
+        AtomSpec spec = parseAtomSpec(atomSpec);
+        String pdbfilename = getPdbFileForModel(spec.getModelNumber());
+        spec.setPdbFile(pdbfilename);
+        atomSpecs.add(spec);
+      } catch (IllegalArgumentException e)
       {
-        // ignore, default to model 0
+        Cache.error("Failed to parse atomspec: " + atomSpec);
       }
+    }
+    return atomSpecs;
+  }
 
-      String residueChain = atomSpec.substring(colonPos + 1);
-      dotPos = residueChain.indexOf(".");
-      int pdbResNum = Integer.parseInt(dotPos == -1 ? residueChain
-              : residueChain.substring(0, dotPos));
-
-      String chainId = dotPos == -1 ? "" : residueChain
-              .substring(dotPos + 1);
-
-      /*
-       * Work out the pdbfilename from the model number
-       */
-      String pdbfilename = modelFileNames[frameNo];
-      findfileloop: for (String pdbfile : this.chimeraMaps.keySet())
+  /**
+   * @param modelId
+   * @return
+   */
+  protected String getPdbFileForModel(int modelId)
+  {
+    /*
+     * Work out the pdbfilename from the model number
+     */
+    String pdbfilename = modelFileNames[0];
+    findfileloop: for (String pdbfile : this.chimeraMaps.keySet())
+    {
+      for (ChimeraModel cm : chimeraMaps.get(pdbfile))
       {
-        for (ChimeraModel cm : chimeraMaps.get(pdbfile))
+        if (cm.getModelNumber() == modelId)
         {
-          if (cm.getModelNumber() == modelId)
-          {
-            pdbfilename = pdbfile;
-            break findfileloop;
-          }
+          pdbfilename = pdbfile;
+          break findfileloop;
         }
       }
-      atomSpecs.add(new AtomSpec(pdbfilename, chainId, pdbResNum, 0));
     }
-
-    /*
-     * Broadcast the selection (which may be empty, if the user just cleared all
-     * selections)
-     */
-    getSsm().mouseOverStructure(atomSpecs);
+    return pdbfilename;
   }
 
   private void log(String message)
@@ -911,238 +562,241 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
     System.err.println("## Chimera log: " + message);
   }
 
-  private void viewerCommandHistory(boolean enable)
-  {
-    // log("(Not yet implemented) History "
-    // + ((debug || enable) ? "on" : "off"));
-  }
-
-  public long getLoadNotifiesHandled()
-  {
-    return loadNotifiesHandled;
-  }
-
-  public void setJalviewColourScheme(ColourSchemeI cs)
+  /**
+   * Constructs and send commands to Chimera to set attributes on residues for
+   * features visible in Jalview.
+   * <p>
+   * The syntax is: setattr r &lt;attName&gt; &lt;attValue&gt; &lt;atomSpec&gt;
+   * <p>
+   * For example: setattr r jv_chain "Ferredoxin-1, Chloroplastic" #0:94.A
+   * 
+   * @param avp
+   * @return
+   */
+  public int sendFeaturesToViewer(AlignmentViewPanel avp)
   {
-    colourBySequence = false;
+    // TODO refactor as required to pull up to an interface
 
-    if (cs == null)
+    Map<String, Map<Object, AtomSpecModel>> featureValues = buildFeaturesMap(
+            avp);
+    List<StructureCommandI> commands = getCommandGenerator()
+            .setAttributes(featureValues);
+    if (commands.size() > 10)
     {
-      return;
+      sendCommandsByFile(commands);
     }
-
-    // Chimera expects RBG values in the range 0-1
-    final double normalise = 255D;
-    viewerCommandHistory(false);
-    StringBuilder command = new StringBuilder(128);
-
-    List<String> residueSet = ResidueProperties.getResidues(isNucleotide(),
-            false);
-    for (String res : residueSet)
+    else
     {
-      Color col = cs.findColour(res.charAt(0));
-      command.append("color " + col.getRed() / normalise + ","
-              + col.getGreen() / normalise + "," + col.getBlue()
-              / normalise + " ::" + res + ";");
+      executeCommands(commands, false, null);
     }
-
-    sendAsynchronousCommand(command.toString(), COLOURING_CHIMERA);
-    viewerCommandHistory(true);
-  }
-
-  /**
-   * called when the binding thinks the UI needs to be refreshed after a Chimera
-   * state change. this could be because structures were loaded, or because an
-   * error has occurred.
-   */
-  public abstract void refreshGUI();
-
-  @Override
-  public void setLoadingFromArchive(boolean loadingFromArchive)
-  {
-    this.loadingFromArchive = loadingFromArchive;
+    return commands.size();
   }
 
   /**
+   * Write commands to a temporary file, and send a command to Chimera to open the
+   * file as a commands script. For use when sending a large number of separate
+   * commands would overload the REST interface mechanism.
    * 
-   * @return true if Chimeral is still restoring state or loading is still going
-   *         on (see setFinsihedLoadingFromArchive)
+   * @param commands
    */
-  @Override
-  public boolean isLoadingFromArchive()
+  protected void sendCommandsByFile(List<StructureCommandI> commands)
   {
-    return loadingFromArchive && !loadingFinished;
+    try
+    {
+      File tmp = File.createTempFile("chim", getCommandFileExtension());
+      tmp.deleteOnExit();
+      PrintWriter out = new PrintWriter(new FileOutputStream(tmp));
+      for (StructureCommandI command : commands)
+      {
+        out.println(command.getCommand());
+      }
+      out.flush();
+      out.close();
+      String path = tmp.getAbsolutePath();
+      StructureCommandI command = getCommandGenerator()
+              .openCommandFile(path);
+      executeCommand(false, null, command);
+    } catch (IOException e)
+    {
+      System.err.println("Sending commands to Chimera via file failed with "
+              + e.getMessage());
+    }
   }
 
   /**
-   * modify flag which controls if sequence colouring events are honoured by the
-   * binding. Should be true for normal operation
-   * 
-   * @param finishedLoading
+   * Returns the file extension required for a file of commands to be read by
+   * the structure viewer
+   * @return
    */
-  @Override
-  public void setFinishedLoadingFromArchive(boolean finishedLoading)
+  protected String getCommandFileExtension()
   {
-    loadingFinished = finishedLoading;
+    return ".com";
   }
 
   /**
-   * Send the Chimera 'background solid <color>" command.
+   * Create features in Jalview for the given attribute name and structure
+   * residues.
    * 
-   * @see https 
-   *      ://www.cgl.ucsf.edu/chimera/current/docs/UsersGuide/midas/background
-   *      .html
-   * @param col
-   */
-  public void setBackgroundColour(Color col)
-  {
-    viewerCommandHistory(false);
-    double normalise = 255D;
-    final String command = "background solid " + col.getRed() / normalise
-            + "," + col.getGreen() / normalise + "," + col.getBlue()
-            / normalise + ";";
-    viewer.sendChimeraCommand(command, false);
-    viewerCommandHistory(true);
-  }
-
-  /**
-   * Ask Chimera to save its session to the given file. Returns true if
-   * successful, else false.
+   * <pre>
+   * The residue list should be 0, 1 or more reply lines of the format: 
+   *     residue id #0:5.A isHelix -155.000836316 index 5 
+   * or 
+   *     residue id #0:6.A isHelix None
+   * </pre>
    * 
-   * @param filepath
-   * @return
+   * @param attName
+   * @param residues
+   * @return the number of features added
    */
-  public boolean saveSession(String filepath)
+  protected int createFeaturesForAttributes(String attName,
+          List<String> residues)
   {
-    if (isChimeraRunning())
+    int featuresAdded = 0;
+    String featureGroup = getViewerFeatureGroup();
+
+    for (String residue : residues)
     {
-      List<String> reply = viewer.sendChimeraCommand("save " + filepath,
-              true);
-      if (reply.contains("Session written"))
+      AtomSpec spec = null;
+      String[] tokens = residue.split(" ");
+      if (tokens.length < 5)
+      {
+        continue;
+      }
+      String atomSpec = tokens[2];
+      String attValue = tokens[4];
+
+      /*
+       * ignore 'None' (e.g. for phi) or 'False' (e.g. for isHelix)
+       */
+      if ("None".equalsIgnoreCase(attValue)
+              || "False".equalsIgnoreCase(attValue))
       {
-        return true;
+        continue;
       }
-      else
+
+      try
       {
-        Cache.log
-                .error("Error saving Chimera session: " + reply.toString());
+        spec = parseAtomSpec(atomSpec);
+      } catch (IllegalArgumentException e)
+      {
+        Cache.error("Problem parsing atomspec " + atomSpec);
+        continue;
+      }
+
+      String chainId = spec.getChain();
+      String description = attValue;
+      float score = Float.NaN;
+      try
+      {
+        score = Float.valueOf(attValue);
+        description = chainId;
+      } catch (NumberFormatException e)
+      {
+        // was not a float value
+      }
+
+      String pdbFile = getPdbFileForModel(spec.getModelNumber());
+      spec.setPdbFile(pdbFile);
+
+      List<AtomSpec> atoms = Collections.singletonList(spec);
+
+      /*
+       * locate the mapped position in the alignment (if any)
+       */
+      SearchResultsI sr = getSsm()
+              .findAlignmentPositionsForStructurePositions(atoms);
+
+      /*
+       * expect one matched alignment position, or none 
+       * (if the structure position is not mapped)
+       */
+      for (SearchResultMatchI m : sr.getResults())
+      {
+        SequenceI seq = m.getSequence();
+        int start = m.getStart();
+        int end = m.getEnd();
+        SequenceFeature sf = new SequenceFeature(attName, description,
+                start, end, score, featureGroup);
+        // todo: should SequenceFeature have an explicit property for chain?
+        // note: repeating the action shouldn't duplicate features
+        if (seq.addSequenceFeature(sf))
+        {
+          featuresAdded++;
+        }
       }
     }
-    return false;
+    return featuresAdded;
   }
 
   /**
-   * Ask Chimera to open a session file. Returns true if successful, else false.
-   * The filename must have a .py extension for this command to work.
+   * Answers the feature group name to apply to features created in Jalview from
+   * Chimera attributes
    * 
-   * @param filepath
    * @return
    */
-  public boolean openSession(String filepath)
+  protected String getViewerFeatureGroup()
+  {
+    // todo pull up to interface
+    return CHIMERA_FEATURE_GROUP;
+  }
+
+  @Override
+  public String getModelIdForFile(String pdbFile)
   {
-    sendChimeraCommand("open " + filepath, true);
-    // todo: test for failure - how?
-    return true;
+    List<ChimeraModel> foundModels = chimeraMaps.get(pdbFile);
+    if (foundModels != null && !foundModels.isEmpty())
+    {
+      return String.valueOf(foundModels.get(0).getModelNumber());
+    }
+    return "";
   }
 
   /**
-   * Returns a list of chains mapped in this viewer. Note this list is not
-   * currently scoped per structure.
+   * Answers a (possibly empty) list of attribute names in Chimera[X], excluding
+   * any which were added from Jalview
    * 
    * @return
    */
-  public List<String> getChainNames()
+  public List<String> getChimeraAttributes()
   {
-    List<String> names = new ArrayList<String>();
-    String[][] allNames = getChains();
-    if (allNames != null)
+    List<String> attributes = new ArrayList<>();
+    StructureCommandI command = getCommandGenerator().listResidueAttributes();
+    final List<String> reply = executeCommand(command, true);
+    if (reply != null)
     {
-      for (String[] chainsForPdb : allNames)
+      for (String inputLine : reply)
       {
-        if (chainsForPdb != null)
+        String[] lineParts = inputLine.split("\\s");
+        if (lineParts.length == 2 && lineParts[0].equals("resattr"))
         {
-          for (String chain : chainsForPdb)
+          String attName = lineParts[1];
+          /*
+           * exclude attributes added from Jalview
+           */
+          if (!attName.startsWith(ChimeraCommands.NAMESPACE_PREFIX))
           {
-            if (chain != null && !names.contains(chain))
-            {
-              names.add(chain);
-            }
+            attributes.add(attName);
           }
         }
       }
     }
-    return names;
+    return attributes;
   }
 
   /**
-   * Send a 'focus' command to Chimera to recentre the visible display
-   */
-  public void focusView()
-  {
-    sendChimeraCommand("focus", false);
-  }
-
-  /**
-   * Send a 'show' command for all atoms in the currently selected columns
-   * 
-   * TODO: pull up to abstract structure viewer interface
+   * Returns the file extension to use for a saved viewer session file (.py)
    * 
-   * @param vp
+   * @return
    */
-  public void highlightSelection(AlignmentViewPanel vp)
+  @Override
+  public String getSessionFileExtension()
   {
-    List<Integer> cols = vp.getAlignViewport().getColumnSelection()
-            .getSelected();
-    AlignmentI alignment = vp.getAlignment();
-    StructureSelectionManager sm = getSsm();
-    for (SequenceI seq : alignment.getSequences())
-    {
-      /*
-       * convert selected columns into sequence positions
-       */
-      int[] positions = new int[cols.size()];
-      int i = 0;
-      for (Integer col : cols)
-      {
-        positions[i++] = seq.findPosition(col);
-      }
-      sm.highlightStructure(this, seq, positions);
-    }
+    return CHIMERA_SESSION_EXTENSION;
   }
 
-  /**
-   * Constructs and send commands to Chimera to set attributes on residues for
-   * features visible in Jalview
-   * 
-   * @param avp
-   */
-  public void sendFeaturesToChimera(AlignmentViewPanel avp)
+  @Override
+  public String getHelpURL()
   {
-    // TODO send a command per feature with the range of residues it applies to
-    AlignmentI alignment = avp.getAlignment();
-    FeatureRenderer fr = getFeatureRenderer(avp);
-
-    /*
-     * fr is null if feature display is turned off
-     */
-    if (fr == null)
-    {
-      return;
-    }
-
-    String[] files = getPdbFile();
-    if (files == null)
-    {
-      return;
-    }
-
-    StructureMappingcommandSet commandSet = ChimeraCommands
-            .getSetAttributeCommandsForFeatures(getSsm(), files,
-                    getSequence(), fr, alignment);
-    for (String command : commandSet.commands)
-    {
-      sendAsynchronousCommand(command, null);
-    }
-
+    return "https://www.cgl.ucsf.edu/chimera/docs/UsersGuide";
   }
 }