SerialMonitor suspend/resume: dealing with boards that change serial port

between uploads. Fixes #3255
Fixed a missing status management, leading IDE to believe Serial Monitor
was opened while it was not. See #3268

app/src/processing/app/AbstractMonitor.java

@@ -30,6 +30,7 @@ import javax.swing.Timer;
 import javax.swing.border.EmptyBorder;
 import javax.swing.text.DefaultCaret;
 
+import cc.arduino.packages.BoardPort;
 import processing.app.debug.TextAreaFIFO;
 import processing.app.legacy.PApplet;
 
@@ -50,8 +51,11 @@ public abstract class AbstractMonitor extends JFrame implements ActionListener {
   private Timer updateTimer;
   private StringBuffer updateBuffer;
 
-  public AbstractMonitor(String title) {
-    super(title);
+  private BoardPort boardPort;
+
+  public AbstractMonitor(BoardPort boardPort) {
+    super(boardPort.getLabel());
+    this.boardPort = boardPort;
 
     addWindowListener(new WindowAdapter() {
       public void windowClosing(WindowEvent event) {
@@ -136,10 +140,7 @@ public abstract class AbstractMonitor extends JFrame implements ActionListener {
     }
     lineEndings.setMaximumSize(lineEndings.getMinimumSize());
 
-    String[] serialRateStrings = {
-            "300", "1200", "2400", "4800", "9600",
-            "19200", "38400", "57600", "115200", "230400", "250000"
-    };
+    String[] serialRateStrings = {"300", "1200", "2400", "4800", "9600", "19200", "38400", "57600", "115200", "230400", "250000"};
 
     serialRates = new JComboBox();
     for (String rate : serialRateStrings) {
@@ -185,8 +186,7 @@ public abstract class AbstractMonitor extends JFrame implements ActionListener {
     closed = false;
   }
 
-  public void enableWindow(boolean enable)
-  {
+  public void enableWindow(boolean enable) {
     textArea.setEnabled(enable);
     scrollPane.setEnabled(enable);
     textField.setEnabled(enable);
@@ -200,33 +200,24 @@ public abstract class AbstractMonitor extends JFrame implements ActionListener {
 
   // Puts the window in suspend state, closing the serial port
   // to allow other entity (the programmer) to use it
-  public void suspend()
-  {
-   enableWindow(false);
-
-   try {
-        close();
-      }
-   catch(Exception e) {
-       //throw new SerialException("Failed closing the port");
-   }
+  public void suspend() throws Exception {
+    enableWindow(false);
 
+    close();
   }
 
-  public void resume() throws SerialException
-  {
+  public void resume(BoardPort boardPort) throws Exception {
+    setBoardPort(boardPort);
+
     // Enable the window
     enableWindow(true);
 
     // If the window is visible, try to open the serial port
-    if (isVisible())
-      try {
-        open();
-      }
-      catch(Exception e) {
-	  throw new SerialException("Failed opening the port");
-      }
+    if (!isVisible()) {
+      return;
+    }
 
+    open();
   }
 
   public void onSerialRateChange(ActionListener listener) {
@@ -275,12 +266,25 @@ public abstract class AbstractMonitor extends JFrame implements ActionListener {
   }
 
   public boolean isClosed() {
-      return closed;
+    return closed;
   }
 
-  public abstract void open() throws Exception;
+  public void open() throws Exception {
+    closed = false;
+  }
 
-  public abstract void close() throws Exception;
+  public void close() throws Exception {
+    closed = true;
+  }
+
+  public BoardPort getBoardPort() {
+    return boardPort;
+  }
+
+  public void setBoardPort(BoardPort boardPort) {
+    setTitle(boardPort.getLabel());
+    this.boardPort = boardPort;
+  }
 
   public synchronized void addToUpdateBuffer(char buff[], int n) {
     updateBuffer.append(buff, 0, n);
@@ -293,15 +297,18 @@ public abstract class AbstractMonitor extends JFrame implements ActionListener {
   }
 
   public void actionPerformed(ActionEvent e) {
-    final String s = consumeUpdateBuffer();
-    if (s.length() > 0) {
-      //System.out.println("gui append " + s.length());
-      if (autoscrollBox.isSelected()) {
-        textArea.appendTrim(s);
-        textArea.setCaretPosition(textArea.getDocument().getLength());
-      } else {
-        textArea.appendNoTrim(s);
-      }
+    String s = consumeUpdateBuffer();
+
+    if (s.isEmpty()) {
+      return;
+    }
+
+    //System.out.println("gui append " + s.length());
+    if (autoscrollBox.isSelected()) {
+      textArea.appendTrim(s);
+      textArea.setCaretPosition(textArea.getDocument().getLength());
+    } else {
+      textArea.appendNoTrim(s);
     }
   }
 

app/src/processing/app/Base.java

@@ -1173,8 +1173,9 @@ public class Base {
     BaseNoGui.onBoardOrPortChange();
 
     // Update editors status bar
-    for (Editor editor : editors)
+    for (Editor editor : editors) {
       editor.onBoardOrPortChange();
+    }
   }
 
   private void openManageLibrariesDialog() {

app/src/processing/app/Editor.java

@@ -1118,7 +1118,7 @@ public class Editor extends JFrame implements RunnerListener {
       }
     }
 
-    onBoardOrPortChange();
+    base.onBoardOrPortChange();
 
     //System.out.println("set to " + get("serial.port"));
   }
@@ -2533,7 +2533,6 @@ public class Editor extends JFrame implements RunnerListener {
           // error message will already be visible
         }
       } catch (SerialNotFoundException e) {
-        populatePortMenu();
         if (serialMenu.getItemCount() == 0) statusError(e);
         else if (serialPrompt()) run();
         else statusNotice(_("Upload canceled."));
@@ -2548,22 +2547,34 @@ public class Editor extends JFrame implements RunnerListener {
         statusError(e);
       } catch (Exception e) {
         e.printStackTrace();
+      } finally {
+        populatePortMenu();
       }
       status.unprogress();
       uploading = false;
       //toolbar.clear();
       toolbar.deactivate(EditorToolbar.EXPORT);
 
-      // Return the serial monitor window to its initial state
-      try {
-        if (serialMonitor != null)
-    	  serialMonitor.resume();
-      }
-      catch (SerialException e) {
-          statusError(e);
-      }
+      resumeOrCloseSerialMonitor();
+      base.onBoardOrPortChange();
+    }
+  }
 
-   }
+  private void resumeOrCloseSerialMonitor() {
+    // Return the serial monitor window to its initial state
+    if (serialMonitor != null) {
+      BoardPort boardPort = BaseNoGui.getDiscoveryManager().find(PreferencesData.get("serial.port"));
+      try {
+        if (boardPort == null) {
+          serialMonitor.close();
+          handleSerial();
+        } else {
+          serialMonitor.resume(boardPort);
+        }
+      } catch (Exception e) {
+        statusError(e);
+      }
+    }
   }
 
   // DAM: in Arduino, this is upload (with verbose output)
@@ -2584,7 +2595,6 @@ public class Editor extends JFrame implements RunnerListener {
           // error message will already be visible
         }
       } catch (SerialNotFoundException e) {
-        populatePortMenu();
         if (serialMenu.getItemCount() == 0) statusError(e);
         else if (serialPrompt()) run();
         else statusNotice(_("Upload canceled."));
@@ -2599,21 +2609,16 @@ public class Editor extends JFrame implements RunnerListener {
         statusError(e);
       } catch (Exception e) {
         e.printStackTrace();
+      } finally {
+        populatePortMenu();
       }
       status.unprogress();
       uploading = false;
       //toolbar.clear();
       toolbar.deactivate(EditorToolbar.EXPORT);
 
-      if (serialMonitor != null) {
-        try {
-          if (serialMonitor != null)
-    	    serialMonitor.resume();
-        }
-        catch (SerialException e) {
-            statusError(e);
-        }
-      }
+      resumeOrCloseSerialMonitor();
+      base.onBoardOrPortChange();
     }
   }
 
@@ -2685,8 +2690,13 @@ public class Editor extends JFrame implements RunnerListener {
 
     // If currently uploading, disable the monitor (it will be later
     // enabled when done uploading)
-    if (uploading)
-      serialMonitor.suspend();
+    if (uploading) {
+      try {
+        serialMonitor.suspend();
+      } catch (Exception e) {
+        statusError(e);
+      }
+    }
 
     boolean success = false;
     do {

app/src/processing/app/NetworkMonitor.java

@@ -26,18 +26,13 @@ public class NetworkMonitor extends AbstractMonitor implements MessageConsumer {
 
   private static final int MAX_CONNECTION_ATTEMPTS = 5;
 
-  private final BoardPort port;
-  private final String ipAddress;
-
   private MessageSiphon inputConsumer;
   private Session session;
   private Channel channel;
   private int connectionAttempts;
 
   public NetworkMonitor(BoardPort port) {
-    super(port.getLabel());
-    this.port = port;
-    this.ipAddress = port.getAddress();
+    super(port);
 
     onSendCommand(new ActionListener() {
       public void actionPerformed(ActionEvent event) {
@@ -61,16 +56,17 @@ public class NetworkMonitor extends AbstractMonitor implements MessageConsumer {
 
   @Override
   public String getAuthorizationKey() {
-    return "runtime.pwd." + ipAddress;
+    return "runtime.pwd." + getBoardPort().getAddress();
   }
 
   @Override
   public void open() throws Exception {
+    super.open();
     this.connectionAttempts = 0;
 
     JSch jSch = new JSch();
     SSHClientSetupChainRing sshClientSetupChain = new SSHConfigFileSetup(new SSHPwdSetup());
-    session = sshClientSetupChain.setup(port, jSch);
+    session = sshClientSetupChain.setup(getBoardPort(), jSch);
 
     session.setUserInfo(new NoInteractionUserInfo(PreferencesData.get(getAuthorizationKey())));
     session.connect(30000);
@@ -156,6 +152,8 @@ public class NetworkMonitor extends AbstractMonitor implements MessageConsumer {
 
   @Override
   public void close() throws Exception {
+    super.close();
+
     if (channel != null) {
       inputConsumer.stop();
       channel.disconnect();

app/src/processing/app/SerialMonitor.java

@@ -30,14 +30,11 @@ import static processing.app.I18n._;
 @SuppressWarnings("serial")
 public class SerialMonitor extends AbstractMonitor {
 
-  private final String port;
   private Serial serial;
   private int serialRate;
 
   public SerialMonitor(BoardPort port) {
-    super(port.getLabel());
-
-    this.port = port.getAddress();
+    super(port);
 
     serialRate = PreferencesData.getInteger("serial.debug_rate");
     serialRates.setSelectedItem(serialRate + " " + _("baud"));
@@ -89,9 +86,11 @@ public class SerialMonitor extends AbstractMonitor {
   }
 
   public void open() throws Exception {
+    super.open();
+
     if (serial != null) return;
 
-    serial = new Serial(port, serialRate) {
+    serial = new Serial(getBoardPort().getAddress(), serialRate) {
       @Override
       protected void message(char buff[], int n) {
         addToUpdateBuffer(buff, n);
@@ -101,6 +100,7 @@ public class SerialMonitor extends AbstractMonitor {
 
   public void close() throws Exception {
     if (serial != null) {
+      super.close();
       int[] location = getPlacement();
       String locationStr = PApplet.join(PApplet.str(location), ",");
       PreferencesData.set("last.serial.location", locationStr);

arduino-core/src/cc/arduino/packages/uploaders/SerialUploader.java

@@ -110,8 +110,9 @@ public class SerialUploader extends Uploader {
     t = prefs.get("upload.wait_for_upload_port");
     boolean waitForUploadPort = (t != null) && t.equals("true");
 
+    String uploadPort = prefs.getOrExcept("serial.port");
+
     if (doTouch) {
-      String uploadPort = prefs.getOrExcept("serial.port");
       try {
         // Toggle 1200 bps on selected serial port to force board reset.
         List<String> before = Serial.list();
@@ -135,26 +136,23 @@ public class SerialUploader extends Uploader {
         throw new RunnerException(e.getMessage());
       }
       prefs.put("serial.port", uploadPort);
-      if (uploadPort.startsWith("/dev/"))
+      if (uploadPort.startsWith("/dev/")) {
         prefs.put("serial.port.file", uploadPort.substring(5));
-      else
+      } else {
         prefs.put("serial.port.file", uploadPort);
+      }
     }
 
     prefs.put("build.path", buildPath);
     prefs.put("build.project_name", className);
-    if (verbose)
+    if (verbose) {
       prefs.put("upload.verbose", prefs.getOrExcept("upload.params.verbose"));
-    else
+    } else {
       prefs.put("upload.verbose", prefs.getOrExcept("upload.params.quiet"));
+    }
 
     boolean uploadResult;
     try {
-//      if (prefs.get("upload.disable_flushing") == null
-//          || prefs.get("upload.disable_flushing").toLowerCase().equals("false")) {
-//        flushSerialBuffer();
-//      }
-
       String pattern = prefs.getOrExcept("upload.pattern");
       String[] cmd = StringReplacer.formatAndSplit(pattern, prefs, true);
       uploadResult = executeUploadCommand(cmd);
@@ -164,9 +162,9 @@ public class SerialUploader extends Uploader {
       throw new RunnerException(e);
     }
 
-    try {
-      if (uploadResult && doTouch) {
-        String uploadPort = PreferencesData.get("serial.port");
+    if (uploadResult && doTouch) {
+      try {
+        String previousUploadPort = PreferencesData.get("serial.port");
         if (waitForUploadPort) {
           // For Due/Leonardo wait until the bootloader serial port disconnects and the
           // sketch serial port reconnects (or timeout after a few seconds if the
@@ -176,15 +174,18 @@ public class SerialUploader extends Uploader {
           long started = System.currentTimeMillis();
           while (System.currentTimeMillis() - started < 2000) {
             List<String> portList = Serial.list();
-            if (portList.contains(uploadPort))
+            if (portList.contains(previousUploadPort)) {
               break;
+            }
             Thread.sleep(250);
           }
         }
+      } catch (InterruptedException ex) {
+        // noop
       }
-    } catch (InterruptedException ex) {
-      // noop
     }
+
+    BaseNoGui.selectSerialPort(uploadPort);
     return uploadResult;
   }
 

arduino-core/src/processing/app/BaseNoGui.java

@@ -1082,10 +1082,11 @@ public class BaseNoGui {
 
   public static void selectSerialPort(String port) {
     PreferencesData.set("serial.port", port);
-    if (port.startsWith("/dev/"))
-      PreferencesData.set("serial.port.file", port.substring(5));
-    else
-      PreferencesData.set("serial.port.file", port);
+    String portFile = port;
+    if (port.startsWith("/dev/")) {
+      portFile = portFile.substring(5);
+    }
+    PreferencesData.set("serial.port.file", portFile);
   }
 
   public static void setBuildFolder(File newBuildFolder) {