From 6cb920bb7a7f285a165cd34b3429dabba71f7518 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Fri, 21 Feb 2020 16:43:48 +0000 Subject: [PATCH] JAL-3518 more extraction of ChimeraX commands as overrides --- .../edu/ucsf/rbvi/strucviz2/ChimeraManager.java | 2 +- src/jalview/ext/rbvi/chimera/ChimeraCommands.java | 88 ++++----- src/jalview/ext/rbvi/chimera/ChimeraXCommands.java | 149 ++++----------- .../ext/rbvi/chimera/JalviewChimeraBinding.java | 133 +++++++++----- src/jalview/gui/ChimeraViewFrame.java | 10 +- src/jalview/gui/JalviewChimeraXBindingModel.java | 75 ++++++++ src/jalview/structure/StructureCommandsBase.java | 11 ++ src/jalview/structure/StructureCommandsI.java | 13 ++ .../structures/models/AAStructureBindingModel.java | 5 + .../ext/rbvi/chimera/ChimeraCommandsTest.java | 19 +- .../ext/rbvi/chimera/ChimeraXCommandsTest.java | 194 ++++++++++++++++++++ .../ext/rbvi/chimera/JalviewChimeraView.java | 23 +-- 12 files changed, 491 insertions(+), 231 deletions(-) create mode 100644 test/jalview/ext/rbvi/chimera/ChimeraXCommandsTest.java diff --git a/src/ext/edu/ucsf/rbvi/strucviz2/ChimeraManager.java b/src/ext/edu/ucsf/rbvi/strucviz2/ChimeraManager.java index b208763..6bb3b71 100644 --- a/src/ext/edu/ucsf/rbvi/strucviz2/ChimeraManager.java +++ b/src/ext/edu/ucsf/rbvi/strucviz2/ChimeraManager.java @@ -760,7 +760,7 @@ public class ChimeraManager public List getAttrList() { List attributes = new ArrayList<>(); - String command = (isChimeraX ? "info " : "list") + "resattr"; + String command = (isChimeraX ? "info " : "list ") + "resattr"; final List reply = sendChimeraCommand(command, true); if (reply != null) { diff --git a/src/jalview/ext/rbvi/chimera/ChimeraCommands.java b/src/jalview/ext/rbvi/chimera/ChimeraCommands.java index 9342286..61adefe 100644 --- a/src/jalview/ext/rbvi/chimera/ChimeraCommands.java +++ b/src/jalview/ext/rbvi/chimera/ChimeraCommands.java @@ -33,7 +33,6 @@ import jalview.gui.Desktop; import jalview.renderer.seqfeatures.FeatureColourFinder; import jalview.structure.StructureCommandsBase; import jalview.structure.StructureMapping; -import jalview.structure.StructureMappingcommandSet; import jalview.structure.StructureSelectionManager; import jalview.util.ColorUtils; import jalview.util.Comparison; @@ -288,6 +287,11 @@ public class ChimeraCommands extends StructureCommandsBase return colourMap; } + /** + * Returns the lowest model number used by the structure viewer + * + * @return + */ protected static int getModelStartNo() { return 0; @@ -309,7 +313,8 @@ public class ChimeraCommands extends StructureCommandsBase * @param endPos * @param chain */ - protected static void addAtomSpecRange(Map map, + protected static final void addAtomSpecRange( + Map map, Object value, int model, int startPos, int endPos, String chain) { /* @@ -334,24 +339,19 @@ public class ChimeraCommands extends StructureCommandsBase * @param files * @param seqs * @param viewPanel - * @param isChimeraX * @return */ - public static StructureMappingcommandSet getSetAttributeCommandsForFeatures( + @Override + public String[] setAttributesForFeatures( StructureSelectionManager ssm, String[] files, SequenceI[][] seqs, - AlignmentViewPanel viewPanel, boolean isChimeraX) + AlignmentViewPanel viewPanel) { Map> featureMap = buildFeaturesMap( - ssm, files, seqs, viewPanel, isChimeraX); - - List commands = buildSetAttributeCommands(featureMap, - isChimeraX); + ssm, files, seqs, viewPanel); - StructureMappingcommandSet cs = new StructureMappingcommandSet( - ChimeraCommands.class, null, - commands.toArray(new String[commands.size()])); + List commands = buildSetAttributeCommands(featureMap); - return cs; + return commands.toArray(new String[commands.size()]); } /** @@ -364,12 +364,11 @@ public class ChimeraCommands extends StructureCommandsBase * @param files * @param seqs * @param viewPanel - * @param isChimeraX * @return */ protected static Map> buildFeaturesMap( StructureSelectionManager ssm, String[] files, SequenceI[][] seqs, - AlignmentViewPanel viewPanel, boolean isChimeraX) + AlignmentViewPanel viewPanel) { Map> theMap = new LinkedHashMap<>(); @@ -407,7 +406,7 @@ public class ChimeraCommands extends StructureCommandsBase AlignmentI alignment = viewPanel.getAlignment(); for (int pdbfnum = 0; pdbfnum < files.length; pdbfnum++) { - final int modelNumber = pdbfnum + (isChimeraX ? 1 : 0); + final int modelNumber = pdbfnum + getModelStartNo(); StructureMapping[] mapping = ssm.getMapping(files[pdbfnum]); if (mapping == null || mapping.length < 1) @@ -589,17 +588,15 @@ public class ChimeraCommands extends StructureCommandsBase * * 
    * 
 setattr r  " " #modelnumber:range.chain 
-   * e.g. setattr r jv:chain  #0:2.B,4.B,9-12.B|#1:1.A,2-6.A,...
+   * e.g. setattr r jv_chain <value> #0:2.B,4.B,9-12.B|#1:1.A,2-6.A,...
    * 

    *
* * @param featureMap - * @param isChimeraX * @return */ - protected static List buildSetAttributeCommands( - Map> featureMap, - boolean isChimeraX) + protected List buildSetAttributeCommands( + Map> featureMap) { List commands = new ArrayList<>(); for (String featureType : featureMap.keySet()) @@ -621,25 +618,11 @@ public class ChimeraCommands extends StructureCommandsBase * Put values in single quotes, encoding any embedded single quotes */ AtomSpecModel atomSpecModel = values.get(value); - StringBuilder sb = new StringBuilder(128); - sb.append("setattr"); - if (isChimeraX) - { - sb.append(" ").append(atomSpecModel.getAtomSpecX()); - } String featureValue = value.toString(); featureValue = featureValue.replaceAll("\\'", "'"); - sb.append(" res ").append(attributeName).append(" '") - .append(featureValue).append("' "); - if (isChimeraX) - { - sb.append(" create true"); - } - else - { - sb.append(atomSpecModel.getAtomSpec()); - } - commands.add(sb.toString()); + String cmd = getSetAttributeCommand(attributeName, featureValue, + atomSpecModel); + commands.add(cmd); } } @@ -647,16 +630,37 @@ public class ChimeraCommands extends StructureCommandsBase } /** + * Returns a viewer command to set the given residue attribute value on + * residues specified by the AtomSpecModel, for example + * + * 
+   * setatr res jv_chain 'primary' #1:12-34,48-55.B
+   *
+ * + * @param attributeName + * @param attributeValue + * @param atomSpecModel + * @return + */ + protected String getSetAttributeCommand(String attributeName, + String attributeValue, + AtomSpecModel atomSpecModel) + { + StringBuilder sb = new StringBuilder(128); + sb.append("setattr res ").append(attributeName).append(" '") + .append(attributeValue).append("' "); + sb.append(atomSpecModel.getAtomSpec()); + return sb.toString(); + } + + /** * Makes a prefixed and valid Chimera attribute name. A jv_ prefix is applied * for a 'Jalview' namespace, and any non-alphanumeric character is converted * to an underscore. * * @param featureType * @return - * - * 
-   * @see https://www.cgl.ucsf.edu/chimera/current/docs/UsersGuide/midas/setattr.html
-   *
+ * @see https://www.cgl.ucsf.edu/chimera/current/docs/UsersGuide/midas/setattr.html */ protected static String makeAttributeName(String featureType) { diff --git a/src/jalview/ext/rbvi/chimera/ChimeraXCommands.java b/src/jalview/ext/rbvi/chimera/ChimeraXCommands.java index 3947bb0..7693802 100644 --- a/src/jalview/ext/rbvi/chimera/ChimeraXCommands.java +++ b/src/jalview/ext/rbvi/chimera/ChimeraXCommands.java @@ -32,7 +32,6 @@ import jalview.datamodel.SequenceI; import jalview.gui.Desktop; import jalview.renderer.seqfeatures.FeatureColourFinder; import jalview.structure.StructureMapping; -import jalview.structure.StructureMappingcommandSet; import jalview.structure.StructureSelectionManager; import jalview.util.ColorUtils; import jalview.util.Comparison; @@ -50,13 +49,11 @@ import java.util.Map.Entry; */ public class ChimeraXCommands extends ChimeraCommands { - public static final String NAMESPACE_PREFIX = "jv_"; - private static final String CMD_COLOUR_BY_CHARGE = "color white;color :ASP,GLU red;color :LYS,ARG blue;color :CYS yellow"; /** * Traverses a map of { modelNumber, {chain, {list of from-to ranges} } } and - * builds a Chimera format atom spec + * builds a ChimeraX format atom spec * * @param modelAndChainRanges */ @@ -211,65 +208,6 @@ public class ChimeraXCommands extends ChimeraCommands } /** - * Helper method to add one contiguous range to the AtomSpec model for the given - * value (creating the model if necessary). As used by Jalview, {@code value} is - *
    - *
  • a colour, when building a 'colour structure by sequence' command
    • - *
  • a feature value, when building a 'set Chimera attributes from features' - * command
    • - *
- * - * @param map - * @param value - * @param model - * @param startPos - * @param endPos - * @param chain - */ - protected static void addAtomSpecRange(Map map, - Object value, int model, int startPos, int endPos, String chain) - { - /* - * Get/initialize map of data for the colour - */ - AtomSpecModel atomSpec = map.get(value); - if (atomSpec == null) - { - atomSpec = new AtomSpecModel(); - map.put(value, atomSpec); - } - - atomSpec.addRange(model, startPos, endPos, chain); - } - - /** - * Constructs and returns Chimera commands to set attributes on residues - * corresponding to features in Jalview. Attribute names are the Jalview - * feature type, with a "jv_" prefix. - * - * @param ssm - * @param files - * @param seqs - * @param viewPanel - * @return - */ - public static StructureMappingcommandSet getSetAttributeCommandsForFeatures( - StructureSelectionManager ssm, String[] files, SequenceI[][] seqs, - AlignmentViewPanel viewPanel) - { - Map> featureMap = buildFeaturesMap( - ssm, files, seqs, viewPanel); - - List commands = buildSetAttributeCommands(featureMap); - - StructureMappingcommandSet cs = new StructureMappingcommandSet( - ChimeraXCommands.class, null, - commands.toArray(new String[commands.size()])); - - return cs; - } - - /** * 
    * Helper method to build a map of 
    *   { featureType, { feature value, AtomSpecModel } }
@@ -495,56 +433,6 @@ public class ChimeraXCommands extends ChimeraCommands
   }
 
   /**
-   * Traverse the map of features/values/models/chains/positions to construct a
-   * list of 'setattr' commands (one per distinct feature type and value).
-   * 

-   * The format of each command is
-   * 
-   * 
-   * 
 setattr r  " " #modelnumber:range.chain 
-   * e.g. setattr r jv:chain  #0:2.B,4.B,9-12.B|#1:1.A,2-6.A,...
-   * 

-   * 

-   * 
-   * @param featureMap
-   * @return
-   */
-  protected static List buildSetAttributeCommands(
-          Map> featureMap)
-  {
-    List commands = new ArrayList<>();
-    for (String featureType : featureMap.keySet())
-    {
-      String attributeName = makeAttributeName(featureType);
-
-      /*
-       * clear down existing attributes for this feature
-       */
-      // 'problem' - sets attribute to None on all residues - overkill?
-      // commands.add("~setattr r " + attributeName + " :*");
-
-      Map values = featureMap.get(featureType);
-      for (Object value : values.keySet())
-      {
-        /*
-         * for each distinct value recorded for this feature type,
-         * add a command to set the attribute on the mapped residues
-         * Put values in single quotes, encoding any embedded single quotes
-         */
-        StringBuilder sb = new StringBuilder(128);
-        String featureValue = value.toString();
-        featureValue = featureValue.replaceAll("\\'", "'");
-        sb.append("setattr r ").append(attributeName).append(" '")
-                .append(featureValue).append("' ");
-        sb.append(values.get(value).getAtomSpec());
-        commands.add(sb.toString());
-      }
-    }
-
-    return commands;
-  }
-
-  /**
    * Makes a prefixed and valid Chimera attribute name. A jv_ prefix is applied
    * for a 'Jalview' namespace, and any non-alphanumeric character is converted
    * to an underscore.
@@ -624,4 +512,39 @@ public class ChimeraXCommands extends ChimeraCommands
     return "view";
   }
 
+  /**
+   * {@inheritDoc}
+   * 
+   * @return
+   */
+  protected static int getModelStartNo()
+  {
+    return 1;
+  }
+
+  /**
+   * Returns a viewer command to set the given residue attribute value on
+   * residues specified by the AtomSpecModel, for example
+   * 
+   * 
+   * setattr #0/A:3-9,14-20,39-43 res jv_strand 'strand' create true
+   * 

+   * 
+   * @param attributeName
+   * @param attributeValue
+   * @param atomSpecModel
+   * @return
+   */
+  @Override
+  protected String getSetAttributeCommand(String attributeName,
+          String attributeValue, AtomSpecModel atomSpecModel)
+  {
+    StringBuilder sb = new StringBuilder(128);
+    sb.append("setattr ").append(atomSpecModel.getAtomSpecX());
+    sb.append(" res ").append(attributeName).append(" '")
+            .append(attributeValue).append("'");
+    sb.append(" create true");
+    return sb.toString();
+  }
+
 }
diff --git a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
index 9695eae..be1de5a 100644
--- a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
+++ b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
@@ -35,7 +35,6 @@ import jalview.gui.StructureViewer.ViewerType;
 import jalview.httpserver.AbstractRequestHandler;
 import jalview.io.DataSourceType;
 import jalview.structure.AtomSpec;
-import jalview.structure.StructureMappingcommandSet;
 import jalview.structure.StructureSelectionManager;
 import jalview.structures.models.AAStructureBindingModel;
 import jalview.util.MessageManager;
@@ -84,7 +83,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
   /*
    * Map of ChimeraModel objects keyed by PDB full local file name
    */
-  private Map> chimeraMaps = new LinkedHashMap<>();
+  protected Map> chimeraMaps = new LinkedHashMap<>();
 
   String lastHighlightCommand;
 
@@ -127,34 +126,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
       if (!alreadyOpen)
       {
         chimeraManager.openModel(file, pe.getId(), ModelType.PDB_MODEL);
-        if (chimeraManager.isChimeraX())
-        {
-          /*
-           * ChimeraX hack: force chimera model name to pdbId
-           */
-          int modelNumber = chimeraMaps.size() + 1;
-          String command = "setattr #" + modelNumber + " models name "
-                  + pe.getId();
-          executeCommand(command, false);
-          modelsToMap.add(new ChimeraModel(pe.getId(), ModelType.PDB_MODEL,
-                  modelNumber, 0));
-        }
-        else
-        {
-          /*
-           * Chimera: query for actual models and find the one with
-           * matching model name - set in viewer.openModel()
-           */
-          List newList = chimeraManager.getModelList();
-          // JAL-1728 newList.removeAll(oldList) does not work
-          for (ChimeraModel cm : newList)
-          {
-            if (cm.getModelName().equals(pe.getId()))
-            {
-              modelsToMap.add(cm);
-            }
-          }
-        }
+        addChimeraModel(pe, modelsToMap);
       }
 
       chimeraMaps.put(file, modelsToMap);
@@ -174,6 +146,31 @@ 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 modelsToMap)
+  {
+    /*
+     * Chimera: query for actual models and find the one with
+     * matching model name - already set in viewer.openModel()
+     */
+    List 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
@@ -585,8 +582,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
       return true;
     }
 
-    boolean launched = chimeraManager.launchChimera(
-            StructureManager.getChimeraPaths(chimeraManager.isChimeraX()));
+    boolean launched = chimeraManager.launchChimera(getChimeraPaths());
     if (launched)
     {
       startChimeraProcessMonitor();
@@ -599,6 +595,16 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
   }
 
   /**
+   * Returns a list of candidate paths to the Chimera program executable
+   * 
+   * @return
+   */
+  protected List getChimeraPaths()
+  {
+    return StructureManager.getChimeraPaths(false);
+  }
+
+  /**
    * Answers true if the Chimera process is still running, false if ended or not
    * started.
    * 
@@ -856,9 +862,8 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
        * Chimera:  https://www.cgl.ucsf.edu/chimera/current/docs/UsersGuide/midas/save.html
        * ChimeraX: https://www.cgl.ucsf.edu/chimerax/docs/user/commands/save.html
        */
-      String command = isChimeraX() ? "save session " : "save ";
-      List reply = chimeraManager.sendChimeraCommand(command + filepath,
-              true);
+      String command = getSaveSessionCommand(filepath);
+      List reply = chimeraManager.sendChimeraCommand(command, true);
       if (reply.contains("Session written"))
       {
         return true;
@@ -873,6 +878,17 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
   }
 
   /**
+   * Returns the command to save the viewer session to the given file path
+   * 
+   * @param filepath
+   * @return
+   */
+  protected String getSaveSessionCommand(String filepath)
+  {
+    return "save " + filepath;
+  }
+
+  /**
    * Ask Chimera to open a session file. Returns true if successful, else false.
    * The filename must have a .py (Chimera) or .cxs (ChimeraX) extension for
    * this command to work.
@@ -935,10 +951,8 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
       return 0;
     }
 
-    StructureMappingcommandSet commandSet = ChimeraCommands
-            .getSetAttributeCommandsForFeatures(getSsm(), files,
-                    getSequence(), avp, chimeraManager.isChimeraX());
-    String[] commands = commandSet.commands;
+    String[] commands = getCommandGenerator()
+            .setAttributesForFeatures(getSsm(), files, getSequence(), avp);
     if (commands.length > 10)
     {
       sendCommandsByFile(commands);
@@ -962,10 +976,9 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
    */
   protected void sendCommandsByFile(String[] commands)
   {
-    boolean toChimeraX = chimeraManager.isChimeraX();
     try
     {
-      File tmp = File.createTempFile("chim", toChimeraX ? ".cxc" : ".com");
+      File tmp = File.createTempFile("chim", getCommandFileExtension());
       tmp.deleteOnExit();
       PrintWriter out = new PrintWriter(new FileOutputStream(tmp));
       for (String command : commands)
@@ -975,7 +988,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
       out.flush();
       out.close();
       String path = tmp.getAbsolutePath();
-      String command = "open " + (toChimeraX ? "" : "cmd:") + path;
+      String command = getOpenCommandFileCommand(path);
       sendAsynchronousCommand(command, null);
     } catch (IOException e)
     {
@@ -985,6 +998,28 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
   }
 
   /**
+   * Returns the command for the structure viewer to open a file of commands at
+   * the given file path
+   * 
+   * @param path
+   * @return
+   */
+  protected String getOpenCommandFileCommand(String path)
+  {
+    return "open cmd:" + path;
+  }
+
+  /**
+   * Returns the file extension required for a file of commands to be read by
+   * the structure viewer
+   * @return
+   */
+  protected String getCommandFileExtension()
+  {
+    return ".com";
+  }
+
+  /**
    * Get Chimera residues which have the named attribute, find the mapped
    * positions in the Jalview sequence(s), and set as sequence features
    * 
@@ -1153,8 +1188,18 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
     return atts;
   }
 
-  public boolean isChimeraX()
+  /**
+   * Returns the file extension to use for a saved viewer session file
+   * 
+   * @return
+   */
+  public String getSessionFileExtension()
+  {
+    return ".py";
+  }
+
+  public String getHelpURL()
   {
-    return chimeraManager.isChimeraX();
+    return "https://www.cgl.ucsf.edu/chimera/docs/UsersGuide";
   }
 }
diff --git a/src/jalview/gui/ChimeraViewFrame.java b/src/jalview/gui/ChimeraViewFrame.java
index 22336b9..ab5ee7a 100644
--- a/src/jalview/gui/ChimeraViewFrame.java
+++ b/src/jalview/gui/ChimeraViewFrame.java
@@ -164,9 +164,9 @@ public class ChimeraViewFrame extends StructureViewerBase
   /**
    * Send a command to Chimera to create residue attributes for Jalview features
    * 

-   * The syntax is: setattr r   
+   * The syntax is: setattr r <attName> <attValue> <atomSpec>
    * 

-   * For example: setattr r jv:chain "Ferredoxin-1, Chloroplastic" #0:94.A
+   * For example: setattr r jv_chain "Ferredoxin-1, Chloroplastic" #0:94.A
    */
   protected void sendFeaturesToChimera()
   {
@@ -650,9 +650,7 @@ public class ChimeraViewFrame extends StructureViewerBase
   {
     try
     {
-      String url = jmb.isChimeraX()
-              ? "http://www.rbvi.ucsf.edu/chimerax/docs/user/index.html"
-              : "https://www.cgl.ucsf.edu/chimera/docs/UsersGuide";
+      String url = jmb.getHelpURL();
       BrowserLauncher.openURL(url);
     } catch (IOException ex)
     {
@@ -682,7 +680,7 @@ public class ChimeraViewFrame extends StructureViewerBase
     {
       if (pathUsed == null)
       {
-        String suffix = jmb.isChimeraX() ? ".cxs" : ".py";
+        String suffix = jmb.getSessionFileExtension();
         File tempFile = File.createTempFile("chimera", suffix);
         tempFile.deleteOnExit();
         pathUsed = tempFile.getPath();
diff --git a/src/jalview/gui/JalviewChimeraXBindingModel.java b/src/jalview/gui/JalviewChimeraXBindingModel.java
index e2aaa65..0779bef 100644
--- a/src/jalview/gui/JalviewChimeraXBindingModel.java
+++ b/src/jalview/gui/JalviewChimeraXBindingModel.java
@@ -5,6 +5,12 @@ import jalview.datamodel.SequenceI;
 import jalview.io.DataSourceType;
 import jalview.structure.StructureSelectionManager;
 
+import java.util.List;
+
+import ext.edu.ucsf.rbvi.strucviz2.ChimeraModel;
+import ext.edu.ucsf.rbvi.strucviz2.StructureManager;
+import ext.edu.ucsf.rbvi.strucviz2.StructureManager.ModelType;
+
 public class JalviewChimeraXBindingModel extends JalviewChimeraBindingModel
 {
 
@@ -15,4 +21,73 @@ public class JalviewChimeraXBindingModel extends JalviewChimeraBindingModel
     super(chimeraViewFrame, ssm, pdbentry, sequenceIs, protocol);
   }
 
+  @Override
+  protected List getChimeraPaths()
+  {
+    return StructureManager.getChimeraPaths(true);
+  }
+
+  @Override
+  protected void addChimeraModel(PDBEntry pe,
+          List modelsToMap)
+  {
+    /*
+     * ChimeraX hack: force chimera model name to pdbId here
+     */
+    int modelNumber = chimeraMaps.size() + 1;
+    String command = "setattr #" + modelNumber + " models name "
+            + pe.getId();
+    executeCommand(command, false);
+    modelsToMap.add(new ChimeraModel(pe.getId(), ModelType.PDB_MODEL,
+            modelNumber, 0));
+  }
+
+  /**
+   * {@inheritDoc}
+   * 
+   * @return
+   */
+  @Override
+  protected String getCommandFileExtension()
+  {
+    return ".cxc";
+  }
+
+  /**
+   * {@inheritDoc}
+   * 
+   * @return
+   */
+  @Override
+  protected String getOpenCommandFileCommand(String path)
+  {
+    return "open " + path;
+  }
+
+  /**
+   * {@inheritDoc}
+   */
+  @Override
+  protected String getSaveSessionCommand(String filepath)
+  {
+    return "save session " + filepath;
+  }
+
+  /**
+   * Returns the file extension to use for a saved viewer session file
+   * 
+   * @return
+   */
+  @Override
+  public String getSessionFileExtension()
+  {
+    return ".cxs";
+  }
+
+  @Override
+  public String getHelpURL()
+  {
+    return "http://www.rbvi.ucsf.edu/chimerax/docs/user/index.html";
+  }
+
 }
diff --git a/src/jalview/structure/StructureCommandsBase.java b/src/jalview/structure/StructureCommandsBase.java
index 321fc25..119a27a 100644
--- a/src/jalview/structure/StructureCommandsBase.java
+++ b/src/jalview/structure/StructureCommandsBase.java
@@ -1,5 +1,8 @@
 package jalview.structure;
 
+import jalview.api.AlignmentViewPanel;
+import jalview.datamodel.SequenceI;
+
 /**
  * A base class holding methods useful to all classes that implement commands
  * for structure viewers
@@ -9,4 +12,12 @@ package jalview.structure;
  */
 public abstract class StructureCommandsBase implements StructureCommandsI
 {
+
+  @Override
+  public String[] setAttributesForFeatures(StructureSelectionManager ssm,
+          String[] files, SequenceI[][] sequence, AlignmentViewPanel avp)
+  {
+    // default does nothing, override where this is implemented
+    return null;
+  }
 }
diff --git a/src/jalview/structure/StructureCommandsI.java b/src/jalview/structure/StructureCommandsI.java
index a5a419c..eda5aa9 100644
--- a/src/jalview/structure/StructureCommandsI.java
+++ b/src/jalview/structure/StructureCommandsI.java
@@ -87,4 +87,17 @@ public interface StructureCommandsI
    */
   String showChains(List toShow);
 
+  /**
+   * Returns zero, one or more commands to set attributes on mapped residues in
+   * the structure viewer for any features present and displayed in Jalview
+   * 
+   * @param ssm
+   * @param files
+   * @param sequence
+   * @param avp
+   * @return
+   */
+  String[] setAttributesForFeatures(StructureSelectionManager ssm,
+          String[] files, SequenceI[][] sequence, AlignmentViewPanel avp);
+
 }
diff --git a/src/jalview/structures/models/AAStructureBindingModel.java b/src/jalview/structures/models/AAStructureBindingModel.java
index 49561d7..92b00c7 100644
--- a/src/jalview/structures/models/AAStructureBindingModel.java
+++ b/src/jalview/structures/models/AAStructureBindingModel.java
@@ -1146,4 +1146,9 @@ public abstract class AAStructureBindingModel
       colourBySequence(ap);
     }
   }
+
+  public StructureCommandsI getCommandGenerator()
+  {
+    return commandGenerator;
+  }
 }
diff --git a/test/jalview/ext/rbvi/chimera/ChimeraCommandsTest.java b/test/jalview/ext/rbvi/chimera/ChimeraCommandsTest.java
index 6a02576..0679098 100644
--- a/test/jalview/ext/rbvi/chimera/ChimeraCommandsTest.java
+++ b/test/jalview/ext/rbvi/chimera/ChimeraCommandsTest.java
@@ -83,8 +83,9 @@ public class ChimeraCommandsTest
     featuresMap.put("chain", featureValues);
     ChimeraCommands.addAtomSpecRange(featureValues, "X", 0, 8, 20, "A");
   
-    List commands = ChimeraCommands
-            .buildSetAttributeCommands(featuresMap, false);
+    ChimeraCommands commandGenerator = new ChimeraCommands();
+    List commands = commandGenerator
+            .buildSetAttributeCommands(featuresMap);
     assertEquals(1, commands.size());
 
     /*
@@ -97,8 +98,7 @@ public class ChimeraCommandsTest
     ChimeraCommands.addAtomSpecRange(featureValues, "X", 0, 3, 9, "A");
     // same feature value, contiguous range
     ChimeraCommands.addAtomSpecRange(featureValues, "X", 0, 21, 25, "A");
-    commands = ChimeraCommands.buildSetAttributeCommands(featuresMap,
-            false);
+    commands = commandGenerator.buildSetAttributeCommands(featuresMap);
     assertEquals(1, commands.size());
     assertEquals(commands.get(0), "setattr res jv_chain 'X' #0:3-25.A");
 
@@ -106,16 +106,14 @@ public class ChimeraCommandsTest
     ChimeraCommands.addAtomSpecRange(featureValues, "X", 0, 21, 25, "B");
     // same feature value and chain, different model
     ChimeraCommands.addAtomSpecRange(featureValues, "X", 1, 26, 30, "A");
-    commands = ChimeraCommands.buildSetAttributeCommands(featuresMap,
-            false);
+    commands = commandGenerator.buildSetAttributeCommands(featuresMap);
     assertEquals(1, commands.size());
     assertEquals(commands.get(0),
             "setattr res jv_chain 'X' #0:3-25.A,21-25.B|#1:26-30.A");
 
     // same feature, different value
     ChimeraCommands.addAtomSpecRange(featureValues, "Y", 0, 40, 50, "A");
-    commands = ChimeraCommands.buildSetAttributeCommands(featuresMap,
-            false);
+    commands = commandGenerator.buildSetAttributeCommands(featuresMap);
     assertEquals(2, commands.size());
     // commands are ordered by feature type but not by value
     // so use contains to test for the expected command:
@@ -132,8 +130,7 @@ public class ChimeraCommandsTest
             "A");
     // feature names are sanitised to change non-alphanumeric to underscore
     // feature values are sanitised to encode single quote characters
-    commands = ChimeraCommands.buildSetAttributeCommands(featuresMap,
-            false);
+    commands = commandGenerator.buildSetAttributeCommands(featuresMap);
     assertTrue(commands
             .contains(
                     "setattr res jv_side_chain_binding_ 'metal  'ion!' #0:7-15.A"));
@@ -160,7 +157,7 @@ public class ChimeraCommandsTest
   }
 
   @Test(groups = { "Functional" })
-  public void testGetColourBySequenceCommands_hiddenColumns()
+  public void testColourBySequence_hiddenColumns()
   {
     /*
      * load these sequences, coloured by Strand propensity,
diff --git a/test/jalview/ext/rbvi/chimera/ChimeraXCommandsTest.java b/test/jalview/ext/rbvi/chimera/ChimeraXCommandsTest.java
new file mode 100644
index 0000000..d8b60c2
--- /dev/null
+++ b/test/jalview/ext/rbvi/chimera/ChimeraXCommandsTest.java
@@ -0,0 +1,194 @@
+/*
+ * Jalview - A Sequence Alignment Editor and Viewer ($$Version-Rel$$)
+ * Copyright (C) $$Year-Rel$$ The Jalview Authors
+ * 
+ * This file is part of Jalview.
+ * 
+ * Jalview is free software: you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License 
+ * as published by the Free Software Foundation, either version 3
+ * of the License, or (at your option) any later version.
+ *  
+ * Jalview is distributed in the hope that it will be useful, but 
+ * WITHOUT ANY WARRANTY; without even the implied warranty 
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR 
+ * PURPOSE.  See the GNU General Public License for more details.
+ * 
+ * You should have received a copy of the GNU General Public License
+ * along with Jalview.  If not, see .
+ * The Jalview Authors are detailed in the 'AUTHORS' file.
+ */
+package jalview.ext.rbvi.chimera;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
+import jalview.datamodel.Alignment;
+import jalview.datamodel.AlignmentI;
+import jalview.datamodel.ColumnSelection;
+import jalview.datamodel.Sequence;
+import jalview.datamodel.SequenceI;
+import jalview.gui.AlignFrame;
+import jalview.gui.SequenceRenderer;
+import jalview.schemes.JalviewColourScheme;
+import jalview.structure.StructureMapping;
+import jalview.structure.StructureSelectionManager;
+
+import java.awt.Color;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.testng.annotations.Test;
+
+public class ChimeraXCommandsTest
+{
+
+  @Test(groups = { "Functional" })
+  public void testBuildColourCommands()
+  {
+
+    Map map = new LinkedHashMap<>();
+    ChimeraCommands.addAtomSpecRange(map, Color.blue, 0, 2, 5, "A");
+    ChimeraCommands.addAtomSpecRange(map, Color.blue, 0, 7, 7, "B");
+    ChimeraCommands.addAtomSpecRange(map, Color.blue, 0, 9, 23, "A");
+    ChimeraCommands.addAtomSpecRange(map, Color.blue, 1, 1, 1, "A");
+    ChimeraCommands.addAtomSpecRange(map, Color.blue, 1, 4, 7, "B");
+    ChimeraCommands.addAtomSpecRange(map, Color.yellow, 1, 8, 8, "A");
+    ChimeraCommands.addAtomSpecRange(map, Color.yellow, 1, 3, 5, "A");
+    ChimeraCommands.addAtomSpecRange(map, Color.red, 0, 3, 5, "A");
+    ChimeraCommands.addAtomSpecRange(map, Color.red, 0, 6, 9, "A");
+
+    // Colours should appear in the Chimera command in the order in which
+    // they were added; within colour, by model, by chain, ranges in start order
+    String command = new ChimeraXCommands().buildColourCommands(map).get(0);
+    assertEquals(
+            command,
+            "color #0/A:2-5,9-23/B:7|#1/A:1/B:4-7 #0000ff; color #1/A:3-5,8 #ffff00; color #0/A:3-9 #ff0000");
+  }
+
+  @Test(groups = { "Functional" })
+  public void testBuildSetAttributeCommands()
+  {
+    /*
+     * make a map of { featureType, {featureValue, {residue range specification } } }
+     */
+    Map> featuresMap = new LinkedHashMap<>();
+    Map featureValues = new HashMap<>();
+    
+    /*
+     * start with just one feature/value...
+     */
+    featuresMap.put("chain", featureValues);
+    ChimeraCommands.addAtomSpecRange(featureValues, "X", 0, 8, 20, "A");
+  
+    ChimeraXCommands commandGenerator = new ChimeraXCommands();
+    List commands = commandGenerator
+            .buildSetAttributeCommands(featuresMap);
+    assertEquals(1, commands.size());
+
+    /*
+     * feature name gets a jv_ namespace prefix
+     * feature value is quoted in case it contains spaces
+     */
+    assertEquals(commands.get(0),
+            "setattr #0/A:8-20 res jv_chain 'X' create true");
+
+    // add same feature value, overlapping range
+    ChimeraCommands.addAtomSpecRange(featureValues, "X", 0, 3, 9, "A");
+    // same feature value, contiguous range
+    ChimeraCommands.addAtomSpecRange(featureValues, "X", 0, 21, 25, "A");
+    commands = commandGenerator.buildSetAttributeCommands(featuresMap);
+    assertEquals(1, commands.size());
+    assertEquals(commands.get(0),
+            "setattr #0/A:3-25 res jv_chain 'X' create true");
+
+    // same feature value and model, different chain
+    ChimeraCommands.addAtomSpecRange(featureValues, "X", 0, 21, 25, "B");
+    // same feature value and chain, different model
+    ChimeraCommands.addAtomSpecRange(featureValues, "X", 1, 26, 30, "A");
+    commands = commandGenerator.buildSetAttributeCommands(featuresMap);
+    assertEquals(1, commands.size());
+    assertEquals(commands.get(0),
+            "setattr #0/A:3-25/B:21-25|#1/A:26-30 res jv_chain 'X' create true");
+
+    // same feature, different value
+    ChimeraCommands.addAtomSpecRange(featureValues, "Y", 0, 40, 50, "A");
+    commands = commandGenerator.buildSetAttributeCommands(featuresMap);
+    assertEquals(2, commands.size());
+    // commands are ordered by feature type but not by value
+    // so use contains to test for the expected command:
+    assertTrue(commands
+            .contains(
+                    "setattr #0/A:3-25/B:21-25|#1/A:26-30 res jv_chain 'X' create true"));
+    assertTrue(commands
+            .contains("setattr #0/A:40-50 res jv_chain 'Y' create true"));
+
+    featuresMap.clear();
+    featureValues.clear();
+    featuresMap.put("side-chain binding!", featureValues);
+    ChimeraCommands.addAtomSpecRange(featureValues,
+            "metal  'ion!", 0, 7, 15,
+            "A");
+    // feature names are sanitised to change non-alphanumeric to underscore
+    // feature values are sanitised to encode single quote characters
+    commands = commandGenerator.buildSetAttributeCommands(featuresMap);
+    assertTrue(commands.contains(
+            "setattr #0/A:7-15 res jv_side_chain_binding_ 'metal  'ion!' create true"));
+  }
+
+  @Test(groups = { "Functional" })
+  public void testColourBySequence_hiddenColumns()
+  {
+    /*
+     * load these sequences, coloured by Strand propensity,
+     * with columns 2-4 hidden
+     */
+    SequenceI seq1 = new Sequence("seq1", "MHRSQSSSGG");
+    SequenceI seq2 = new Sequence("seq2", "MVRSNGGSSS");
+    AlignmentI al = new Alignment(new SequenceI[] { seq1, seq2 });
+    AlignFrame af = new AlignFrame(al, 800, 500);
+    af.changeColour_actionPerformed(JalviewColourScheme.Strand.toString());
+    ColumnSelection cs = new ColumnSelection();
+    cs.addElement(2);
+    cs.addElement(3);
+    cs.addElement(4);
+    af.getViewport().setColumnSelection(cs);
+    af.hideSelColumns_actionPerformed(null);
+    SequenceRenderer sr = new SequenceRenderer(af.getViewport());
+    SequenceI[][] seqs = new SequenceI[][] { { seq1 }, { seq2 } };
+    String[] files = new String[] { "seq1.pdb", "seq2.pdb" };
+    StructureSelectionManager ssm = new StructureSelectionManager();
+
+    /*
+     * map residues 1-10 to residues 21-30 (atoms 105-150) in structures
+     */
+    HashMap map = new HashMap<>();
+    for (int pos = 1; pos <= seq1.getLength(); pos++)
+    {
+      map.put(pos, new int[] { 20 + pos, 5 * (20 + pos) });
+    }
+    StructureMapping sm1 = new StructureMapping(seq1, "seq1.pdb", "pdb1",
+            "A", map, null);
+    ssm.addStructureMapping(sm1);
+    StructureMapping sm2 = new StructureMapping(seq2, "seq2.pdb", "pdb2",
+            "B", map, null);
+    ssm.addStructureMapping(sm2);
+
+    String[] commands = new ChimeraXCommands()
+            .colourBySequence(ssm, files, seqs, sr, af.alignPanel);
+    assertEquals(1, commands.length);
+    String theCommand = commands[0];
+    // M colour is #82827d (see strand.html help page)
+    assertTrue(theCommand.contains("color #0/A:21|#1/B:21 #82827d"));// #0:21.A|#1:21.B"));
+    // H colour is #60609f
+    assertTrue(theCommand.contains("color #0/A:22 #60609f"));
+    // V colour is #ffff00
+    assertTrue(theCommand.contains("color #1/B:22 #ffff00"));
+    // hidden columns are Gray (128, 128, 128)
+    assertTrue(theCommand.contains("color #0/A:23-25|#1/B:23-25"));
+    // S and G are both coloured #4949b6
+    assertTrue(theCommand.contains("color #0/A:26-30|#1/B:26-30"));
+  }
+}
diff --git a/test/jalview/ext/rbvi/chimera/JalviewChimeraView.java b/test/jalview/ext/rbvi/chimera/JalviewChimeraView.java
index 725e15b..93ed555 100644
--- a/test/jalview/ext/rbvi/chimera/JalviewChimeraView.java
+++ b/test/jalview/ext/rbvi/chimera/JalviewChimeraView.java
@@ -402,8 +402,8 @@ public class JalviewChimeraView
     assertEquals(binding.getPdbCount(), 1);
   
     /*
-     * 'perform' menu action to copy visible features to
-     * attributes in Chimera
+     * 'perform' menu action to copy Chimera attributes
+     * to features in Jalview
      */
     // TODO rename and pull up method to binding interface
     // once functionality is added for Jmol as well
@@ -440,14 +440,9 @@ public class JalviewChimeraView
     binding.copyStructureAttributesToFeatures("phi", af.getViewport()
             .getAlignPanel());
     fr.setVisible("phi");
-    List fs = fer2Arath.getFeatures().findFeatures(54, 54);
-    assertEquals(fs.size(), 3);
-    /*
-     * order of returned features is not guaranteed
-     */
-    assertTrue("RESNUM".equals(fs.get(0).getType())
-            || "RESNUM".equals(fs.get(1).getType())
-            || "RESNUM".equals(fs.get(2).getType()));
+    List fs = fer2Arath.getFeatures().findFeatures(54, 54,
+            "phi");
+    assertEquals(fs.size(), 2);
     assertTrue(fs.contains(new SequenceFeature("phi", "A", 54, 54,
             -131.0713f, "Chimera")));
     assertTrue(fs.contains(new SequenceFeature("phi", "B", 54, 54,
@@ -473,11 +468,11 @@ public class JalviewChimeraView
           int res, String featureType)
   {
     String where = "at position " + res;
-    List fs = seq.getFeatures().findFeatures(res, res);
+    List fs = seq.getFeatures().findFeatures(res, res,
+            featureType);
 
-    assertEquals(fs.size(), 2, where);
-    assertEquals(fs.get(0).getType(), "RESNUM", where);
-    SequenceFeature sf = fs.get(1);
+    assertEquals(fs.size(), 1, where);
+    SequenceFeature sf = fs.get(0);
     assertEquals(sf.getType(), featureType, where);
     assertEquals(sf.getFeatureGroup(), "Chimera", where);
     assertEquals(sf.getDescription(), "True", where);
-- 
1.7.10.2