JAL-3518 more pull up / test coverage of structure command generation
[jalview.git] / src / jalview / ext / rbvi / chimera / JalviewChimeraBinding.java
index d0fa5ef..3656204 100644 (file)
@@ -21,7 +21,6 @@
 package jalview.ext.rbvi.chimera;
 
 import jalview.api.AlignmentViewPanel;
-import jalview.api.SequenceRenderer;
 import jalview.api.structures.JalviewStructureDisplayI;
 import jalview.bin.Cache;
 import jalview.datamodel.AlignmentI;
@@ -31,20 +30,15 @@ import jalview.datamodel.SearchResultMatchI;
 import jalview.datamodel.SearchResultsI;
 import jalview.datamodel.SequenceFeature;
 import jalview.datamodel.SequenceI;
-import jalview.gui.Preferences;
 import jalview.gui.StructureViewer.ViewerType;
 import jalview.httpserver.AbstractRequestHandler;
 import jalview.io.DataSourceType;
-import jalview.schemes.ColourSchemeI;
-import jalview.schemes.ResidueProperties;
 import jalview.structure.AtomSpec;
-import jalview.structure.StructureMappingcommandSet;
+import jalview.structure.StructureCommandsI.SuperposeData;
 import jalview.structure.StructureSelectionManager;
 import jalview.structures.models.AAStructureBindingModel;
-import jalview.util.ColorUtils;
 import jalview.util.MessageManager;
 
-import java.awt.Color;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
@@ -53,7 +47,6 @@ import java.net.BindException;
 import java.util.ArrayList;
 import java.util.BitSet;
 import java.util.Collections;
-import java.util.Hashtable;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -71,23 +64,16 @@ 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";
 
   private static final String ALPHACARBON = "CA";
 
-  private List<String> chainNames = new ArrayList<>();
-
-  private Hashtable<String, String> chainFile = new Hashtable<>();
-
   /*
    * Object through which we talk to Chimera
    */
-  private ChimeraManager viewer;
+  private ChimeraManager chimeraManager;
 
   /*
    * Object which listens to Chimera notifications
@@ -95,32 +81,12 @@ 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;
-
-  /*
    * Map of ChimeraModel objects keyed by PDB full local file name
    */
-  private Map<String, List<ChimeraModel>> chimeraMaps = new LinkedHashMap<>();
+  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.
-   */
-  private long loadNotifiesHandled = 0;
-
   private Thread chimeraMonitor;
 
   /**
@@ -138,7 +104,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
     try
     {
       List<ChimeraModel> modelsToMap = new ArrayList<>();
-      List<ChimeraModel> oldList = viewer.getModelList();
+      List<ChimeraModel> oldList = chimeraManager.getModelList();
       boolean alreadyOpen = false;
 
       /*
@@ -159,35 +125,8 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
        */
       if (!alreadyOpen)
       {
-        viewer.openModel(file, pe.getId(), ModelType.PDB_MODEL);
-        if (viewer.isChimeraX())
-        {
-          /*
-           * ChimeraX hack: force chimera model name to pdbId
-           */
-          int modelNumber = chimeraMaps.size() + 1;
-          String command = "setattr #" + modelNumber + " models name "
-                  + pe.getId();
-          sendChimeraCommand(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<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);
@@ -207,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<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
@@ -219,10 +183,15 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
           DataSourceType protocol)
   {
     super(ssm, pdbentry, sequenceIs, protocol);
-    viewer = new ChimeraManager(new StructureManager(true));
-    String viewerType = Cache.getProperty(Preferences.STRUCTURE_DISPLAY);
-    viewer.setChimeraX(ViewerType.CHIMERAX.name().equals(viewerType));
+    chimeraManager = new ChimeraManager(new StructureManager(true));
+    chimeraManager.setChimeraX(ViewerType.CHIMERAX.equals(getViewerType()));
+    setStructureCommands(new ChimeraCommands());
+  }
 
+  @Override
+  protected ViewerType getViewerType()
+  {
+    return ViewerType.CHIMERA;
   }
 
   /**
@@ -232,7 +201,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
    */
   protected void startChimeraProcessMonitor()
   {
-    final Process p = viewer.getChimeraProcess();
+    final Process p = chimeraManager.getChimeraProcess();
     chimeraMonitor = new Thread(new Runnable()
     {
 
@@ -265,7 +234,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
     try
     {
       chimeraListener = new ChimeraListener(this);
-      viewer.startListening(chimeraListener.getUri());
+      chimeraManager.startListening(chimeraListener.getUri());
     } catch (BindException e)
     {
       System.err.println(
@@ -274,43 +243,6 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
   }
 
   /**
-   * 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)
-    {
-      int modelNumber = getModelNoForChain(chain);
-      String showChainCmd = modelNumber == -1 ? ""
-              : modelNumber + ":." + chain.split(":")[1];
-      if (!first)
-      {
-        cmd.append(",");
-      }
-      cmd.append(showChainCmd);
-      first = false;
-    }
-
-    /*
-     * 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.
    */
@@ -319,14 +251,14 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
     getSsm().removeStructureViewerListener(this, this.getStructureFiles());
     if (closeChimera)
     {
-      viewer.exitChimera();
+      chimeraManager.exitChimera();
     }
     if (this.chimeraListener != null)
     {
       chimeraListener.shutdown();
       chimeraListener = null;
     }
-    viewer = null;
+    chimeraManager = null;
 
     if (chimeraMonitor != null)
     {
@@ -335,39 +267,13 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
     releaseUIResources();
   }
 
-  @Override
-  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>
-   */
-  @Override
-  public void colourByCharge()
-  {
-    colourBySequence = false;
-    String command = viewer.isChimeraX()
-            ? "color white;color :ASP,GLU red;color :LYS,ARG blue;color :CYS yellow"
-            : "color white;color red ::ASP;color red ::GLU;color blue ::LYS;color blue ::ARG;color yellow ::CYS";
-    sendAsynchronousCommand(command, COLOURING_CHIMERA);
-  }
-
   /**
    * {@inheritDoc}
    */
-  @Override
   public String superposeStructures(AlignmentI[] _alignment,
           int[] _refStructure, HiddenColumns[] _hiddenCols)
   {
+    // TODO delete method
     StringBuilder allComs = new StringBuilder(128);
     String[] files = getStructureFiles();
 
@@ -378,7 +284,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
 
     refreshPdbEntries();
     StringBuilder selectioncom = new StringBuilder(256);
-    boolean chimeraX = viewer.isChimeraX();
+    boolean chimeraX = chimeraManager.isChimeraX();
     for (int a = 0; a < _alignment.length; a++)
     {
       int refStructure = _refStructure[a];
@@ -408,7 +314,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
       SuperposeData[] structures = new SuperposeData[files.length];
       for (int f = 0; f < files.length; f++)
       {
-        structures[f] = new SuperposeData(alignment.getWidth());
+        structures[f] = new SuperposeData(alignment.getWidth(), f);
       }
 
       /*
@@ -618,12 +524,12 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
       }
       else
       {
-        allComs.append("chain @CA|P; ribbon ; focus");
-        allComs.append(selectioncom.toString());
+        allComs.append("chain @CA|P; ribbon ");
+        allComs.append(selectioncom.toString()).append("; focus");
       }
       // allComs.append("; ~display all; chain @CA|P; ribbon ")
       // .append(selectioncom.toString()).append("; focus");
-      List<String> chimeraReplies = sendChimeraCommand(allComs.toString(),
+      List<String> chimeraReplies = executeCommand(allComs.toString(),
               true);
       for (String reply : chimeraReplies)
       {
@@ -676,13 +582,12 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
    */
   public boolean launchChimera()
   {
-    if (viewer.isChimeraLaunched())
+    if (chimeraManager.isChimeraLaunched())
     {
       return true;
     }
 
-    boolean launched = viewer.launchChimera(
-            StructureManager.getChimeraPaths(viewer.isChimeraX()));
+    boolean launched = chimeraManager.launchChimera(getChimeraPaths());
     if (launched)
     {
       startChimeraProcessMonitor();
@@ -695,6 +600,16 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
   }
 
   /**
+   * Returns a list of candidate paths to the Chimera program executable
+   * 
+   * @return
+   */
+  protected List<String> getChimeraPaths()
+  {
+    return StructureManager.getChimeraPaths(false);
+  }
+
+  /**
    * Answers true if the Chimera process is still running, false if ended or not
    * started.
    * 
@@ -702,103 +617,53 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
    */
   public boolean isChimeraRunning()
   {
-    return viewer.isChimeraLaunched();
+    return chimeraManager.isChimeraLaunched();
   }
 
   /**
    * Send a command to Chimera, and optionally log and return any responses.
-   * <p>
-   * Does nothing, and returns null, if the command is the same as the last one
-   * sent [why?].
    * 
    * @param command
    * @param getResponse
    */
-  public List<String> sendChimeraCommand(final String command,
+  @Override
+  public List<String> executeCommand(final String command,
           boolean getResponse)
   {
-    if (viewer == null)
+    if (chimeraManager == null || command == null)
     {
       // ? thread running after viewer shut down
       return null;
     }
     List<String> reply = null;
-    viewerCommandHistory(false);
-    if (true /*lastCommand == null || !lastCommand.equals(command)*/)
+    // trim command or it may never find a match in the replyLog!!
+    List<String> lastReply = chimeraManager
+            .sendChimeraCommand(command.trim(), getResponse);
+    if (getResponse)
     {
-      // trim command or it may never find a match in the replyLog!!
-      List<String> lastReply = viewer.sendChimeraCommand(command.trim(),
-              getResponse);
-      if (getResponse)
+      reply = lastReply;
+      if (debug)
       {
-        reply = lastReply;
-        if (debug)
-        {
-          log("Response from command ('" + command + "') was:\n"
-                  + lastReply);
-        }
+        log("Response from command ('" + command + "') was:\n" + lastReply);
       }
     }
-    viewerCommandHistory(true);
 
     return reply;
   }
 
   /**
-   * Send a Chimera 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 abstract void sendAsynchronousCommand(String command,
-          String progressMsg);
-
-  /**
-   * Sends a set of colour commands to the structure viewer
-   * 
-   * @param colourBySequenceCommands
-   */
-  @Override
-  protected void colourBySequence(
-          StructureMappingcommandSet[] colourBySequenceCommands)
-  {
-    for (StructureMappingcommandSet cpdbbyseq : colourBySequenceCommands)
-    {
-      for (String command : cpdbbyseq.commands)
-      {
-        sendAsynchronousCommand(command, COLOURING_CHIMERA);
-      }
-    }
-  }
-
-  /**
-   * @param files
-   * @param sr
-   * @param viewPanel
-   * @return
-   */
-  @Override
-  protected StructureMappingcommandSet[] getColourBySequenceCommands(
-          String[] files, SequenceRenderer sr, AlignmentViewPanel viewPanel)
-  {
-    return ChimeraCommands.getColourBySequenceCommand(getSsm(), files,
-            getSequence(), sr, viewPanel, viewer.isChimeraX());
-  }
-
-  /**
    * @param command
    */
   protected void executeWhenReady(String command)
   {
     waitForChimera();
-    sendChimeraCommand(command, false);
+    executeCommand(command, false);
     waitForChimera();
   }
 
   private void waitForChimera()
   {
-    while (viewer != null && viewer.isBusy())
+    while (chimeraManager != null && chimeraManager.isBusy())
     {
       try
       {
@@ -809,29 +674,10 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
     }
   }
 
-  // End StructureListener
-  // //////////////////////////
-
-  /**
-   * 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.
-   */
-  public abstract void refreshPdbEntries();
-
-  /**
-   * 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[];
-
-  // ////////////////////////////////
-  // /StructureListener
   @Override
   public synchronized String[] getStructureFiles()
   {
-    if (viewer == null)
+    if (chimeraManager == null)
     {
       return new String[0];
     }
@@ -852,7 +698,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
       return;
     }
 
-    boolean forChimeraX = viewer.isChimeraX();
+    boolean forChimeraX = chimeraManager.isChimeraX();
     StringBuilder cmd = new StringBuilder(128);
     boolean first = true;
     boolean found = false;
@@ -906,11 +752,11 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
      */
     if (lastHighlightCommand != null)
     {
-      viewer.sendChimeraCommand("~" + lastHighlightCommand, false);
+      chimeraManager.sendChimeraCommand("~" + lastHighlightCommand, false);
     }
     if (found)
     {
-      viewer.sendChimeraCommand(command, false);
+      chimeraManager.sendChimeraCommand(command, false);
     }
     this.lastHighlightCommand = command;
   }
@@ -923,7 +769,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
     /*
      * Ask Chimera for its current selection
      */
-    List<String> selection = viewer.getSelectedResidueSpecs();
+    List<String> selection = chimeraManager.getSelectedResidueSpecs();
 
     /*
      * Parse model number, residue and chain for each selected position,
@@ -949,7 +795,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
   protected List<AtomSpec> convertStructureResiduesToAlignment(
           List<String> structureSelection)
   {
-    boolean chimeraX = viewer.isChimeraX();
+    boolean chimeraX = chimeraManager.isChimeraX();
     List<AtomSpec> atomSpecs = new ArrayList<>();
     for (String atomSpec : structureSelection)
     {
@@ -996,115 +842,6 @@ 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;
-  }
-
-  @Override
-  public void setJalviewColourScheme(ColourSchemeI cs)
-  {
-    colourBySequence = false;
-
-    if (cs == null)
-    {
-      return;
-    }
-
-    viewerCommandHistory(false);
-    StringBuilder command = new StringBuilder(128);
-
-    List<String> residueSet = ResidueProperties.getResidues(isNucleotide(),
-            false);
-
-    /*
-     * concatenate colour commands, one per residue symbol
-     * Chimera format:  color colorCode ::VAL
-     * ChimeraX format: color :VAL colourCode
-     */
-    boolean chimeraX = viewer.isChimeraX();
-    for (String resName : residueSet)
-    {
-      char res = resName.length() == 3
-              ? ResidueProperties.getSingleCharacterCode(resName)
-              : resName.charAt(0);
-      Color col = cs.findColour(res, 0, null, null, 0f);
-      command.append("color ");
-      String colorSpec = ColorUtils.toTkCode(col);
-      if (chimeraX)
-      {
-        command.append(":").append(resName).append(" ").append(colorSpec);
-      }
-      else
-      {
-        command.append(colorSpec).append(" ::").append(resName);
-      }
-      command.append(";");
-    }
-
-    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 true if Chimeral is still restoring state or loading is still going
-   *         on (see setFinsihedLoadingFromArchive)
-   */
-  @Override
-  public boolean isLoadingFromArchive()
-  {
-    return loadingFromArchive && !loadingFinished;
-  }
-
-  /**
-   * modify flag which controls if sequence colouring events are honoured by the
-   * binding. Should be true for normal operation
-   * 
-   * @param finishedLoading
-   */
-  @Override
-  public void setFinishedLoadingFromArchive(boolean finishedLoading)
-  {
-    loadingFinished = finishedLoading;
-  }
-
-  /**
-   * Send the Chimera 'background solid <color>" command.
-   * 
-   * @see https
-   *      ://www.cgl.ucsf.edu/chimera/current/docs/UsersGuide/midas/background
-   *      .html
-   * @param col
-   */
-  @Override
-  public void setBackgroundColour(Color col)
-  {
-    viewerCommandHistory(false);
-    String command = "set bgColor " + ColorUtils.toTkCode(col);
-    viewer.sendChimeraCommand(command, false);
-    viewerCommandHistory(true);
-  }
-
   /**
    * Ask Chimera to save its session to the given file. Returns true if
    * successful, else false.
@@ -1120,9 +857,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<String> reply = viewer.sendChimeraCommand(command + filepath,
-              true);
+      String command = getCommandGenerator().saveSession(filepath);
+      List<String> reply = chimeraManager.sendChimeraCommand(command, true);
       if (reply.contains("Session written"))
       {
         return true;
@@ -1150,32 +886,12 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
      * Chimera:  https://www.cgl.ucsf.edu/chimera/current/docs/UsersGuide/midas/open.html
      * ChimeraX: https://www.cgl.ucsf.edu/chimerax/docs/user/commands/open.html
      */
-    sendChimeraCommand("open " + filepath, true);
+    executeCommand("open " + filepath, true);
     // todo: test for failure - how?
     return true;
   }
 
   /**
-   * Returns a list of chains mapped in this viewer. Note this list is not
-   * currently scoped per structure.
-   * 
-   * @return
-   */
-  @Override
-  public List<String> getChainNames()
-  {
-    return chainNames;
-  }
-
-  /**
-   * Send a 'focus' command to Chimera to recentre the visible display
-   */
-  public void focusView()
-  {
-    sendChimeraCommand(viewer.isChimeraX() ? "view" : "focus", false);
-  }
-
-  /**
    * Send a 'show' command for all atoms in the currently selected columns
    * 
    * TODO: pull up to abstract structure viewer interface
@@ -1213,18 +929,14 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
   public int sendFeaturesToViewer(AlignmentViewPanel avp)
   {
     // TODO refactor as required to pull up to an interface
-    AlignmentI alignment = avp.getAlignment();
-
     String[] files = getStructureFiles();
     if (files == null)
     {
       return 0;
     }
 
-    StructureMappingcommandSet commandSet = ChimeraCommands
-            .getSetAttributeCommandsForFeatures(getSsm(), files,
-                    getSequence(), avp, viewer.isChimeraX());
-    String[] commands = commandSet.commands;
+    String[] commands = getCommandGenerator()
+            .setAttributesForFeatures(getSsm(), files, getSequence(), avp);
     if (commands.length > 10)
     {
       sendCommandsByFile(commands);
@@ -1248,10 +960,9 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
    */
   protected void sendCommandsByFile(String[] commands)
   {
-    boolean toChimeraX = viewer.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)
@@ -1261,7 +972,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
       out.flush();
       out.close();
       String path = tmp.getAbsolutePath();
-      String command = "open " + (toChimeraX ? "" : "cmd:") + path;
+      String command = getCommandGenerator().openCommandFile(path);
       sendAsynchronousCommand(command, null);
     } catch (IOException e)
     {
@@ -1271,6 +982,16 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
   }
 
   /**
+   * 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
    * 
@@ -1291,7 +1012,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
     // fails for 'average.bfactor' (which is bad):
 
     String cmd = "list residues attr '" + attName + "'";
-    List<String> residues = sendChimeraCommand(cmd, true);
+    List<String> residues = executeCommand(cmd, true);
 
     boolean featureAdded = createFeaturesForAttributes(attName, residues);
     if (featureAdded)
@@ -1320,7 +1041,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
   {
     boolean featureAdded = false;
     String featureGroup = getViewerFeatureGroup();
-    boolean chimeraX = viewer.isChimeraX();
+    boolean chimeraX = chimeraManager.isChimeraX();
 
     for (String residue : residues)
     {
@@ -1405,19 +1126,10 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
     return CHIMERA_FEATURE_GROUP;
   }
 
-  public Hashtable<String, String> getChainFile()
-  {
-    return chainFile;
-  }
-
-  public List<ChimeraModel> getChimeraModelByChain(String chain)
-  {
-    return chimeraMaps.get(chainFile.get(chain));
-  }
-
-  public int getModelNoForChain(String chain)
+  @Override
+  public int getModelNoForFile(String pdbFile)
   {
-    List<ChimeraModel> foundModels = getChimeraModelByChain(chain);
+    List<ChimeraModel> foundModels = chimeraMaps.get(pdbFile);
     if (foundModels != null && !foundModels.isEmpty())
     {
       return foundModels.get(0).getModelNumber();
@@ -1433,7 +1145,7 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
    */
   public List<String> getChimeraAttributes()
   {
-    List<String> atts = viewer.getAttrList();
+    List<String> atts = chimeraManager.getAttrList();
     Iterator<String> it = atts.iterator();
     while (it.hasNext())
     {
@@ -1448,8 +1160,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 viewer.isChimeraX();
+    return "https://www.cgl.ucsf.edu/chimera/docs/UsersGuide";
   }
 }