From e55d4145d00456253d6e8e5d370c2ab72b0e9897 Mon Sep 17 00:00:00 2001 From: Federico Fissore Date: Mon, 1 Jun 2015 11:27:52 +0200 Subject: [PATCH] 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 | 85 ++++++++++--------- app/src/processing/app/Base.java | 3 +- app/src/processing/app/Editor.java | 56 +++++++----- app/src/processing/app/NetworkMonitor.java | 14 ++- app/src/processing/app/SerialMonitor.java | 10 +-- .../packages/uploaders/SerialUploader.java | 33 +++---- .../src/processing/app/BaseNoGui.java | 9 +- 7 files changed, 114 insertions(+), 96 deletions(-) diff --git a/app/src/processing/app/AbstractMonitor.java b/app/src/processing/app/AbstractMonitor.java index 4a1dd8101..710e86115 100644 --- a/app/src/processing/app/AbstractMonitor.java +++ b/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); } } diff --git a/app/src/processing/app/Base.java b/app/src/processing/app/Base.java index a5359aa71..49b420a32 100644 --- a/app/src/processing/app/Base.java +++ b/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() { diff --git a/app/src/processing/app/Editor.java b/app/src/processing/app/Editor.java index 415436eeb..182b799ee 100644 --- a/app/src/processing/app/Editor.java +++ b/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 { diff --git a/app/src/processing/app/NetworkMonitor.java b/app/src/processing/app/NetworkMonitor.java index 850481ef7..716c9f0fc 100644 --- a/app/src/processing/app/NetworkMonitor.java +++ b/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(); diff --git a/app/src/processing/app/SerialMonitor.java b/app/src/processing/app/SerialMonitor.java index 9f48f82cb..e4d1455b5 100644 --- a/app/src/processing/app/SerialMonitor.java +++ b/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); diff --git a/arduino-core/src/cc/arduino/packages/uploaders/SerialUploader.java b/arduino-core/src/cc/arduino/packages/uploaders/SerialUploader.java index c98d3eab7..26d8b3cc3 100644 --- a/arduino-core/src/cc/arduino/packages/uploaders/SerialUploader.java +++ b/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 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 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; } diff --git a/arduino-core/src/processing/app/BaseNoGui.java b/arduino-core/src/processing/app/BaseNoGui.java index c939a0ffc..9fa933e2c 100644 --- a/arduino-core/src/processing/app/BaseNoGui.java +++ b/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) {