Skip to content

Fix unneeded "Select port on upload" message #8194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Nov 14, 2018
Merged

Fix unneeded "Select port on upload" message #8194

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
205ce59
Removed dead-code in comments
cmaglie Nov 9, 2018
eefe8dd
Refactor: simplified TargetBoard retrieve
cmaglie Nov 9, 2018
0310ec1
Refactor: simplified bool prefs retrieval
cmaglie Nov 9, 2018
aebdba2
Refactor: removed useless parameter from StringReplacer.formatAndSplit
cmaglie Nov 9, 2018
6ea66ea
Refactor: Removed code duplication in Export(App)Handlers
cmaglie Nov 9, 2018
2cf63e7
Added StringReplacer.checkIfRequiredKeyIsMissingOrExcept helper method
cmaglie Nov 9, 2018
459c173
Fix 'Select port on upload' error message
cmaglie Nov 9, 2018
89ff7b2
Show port on status bar only when selected
cmaglie Nov 9, 2018
d903d6b
Nicer serial port selection dialog box
cmaglie Nov 9, 2018
c1c8d5f
Editor: the serial port selection dialog now works
cmaglie Nov 12, 2018
13e72be
Editor: fixed port ordering in tools menu
cmaglie Nov 12, 2018
d99a31c
Better error for 'no serial ports available'
cmaglie Nov 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 15 additions & 58 deletions app/src/processing/app/Editor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,8 @@ public boolean test(SketchController sketch) {
Runnable presentHandler;
private Runnable runAndSaveHandler;
private Runnable presentAndSaveHandler;
Runnable exportHandler;
private Runnable exportAppHandler;
private UploadHandler uploadHandler;
private UploadHandler uploadUsingProgrammerHandler;
private Runnable timeoutUploadHandler;

private Map<String, Tool> internalToolCache = new HashMap<String, Tool>();
Expand Down Expand Up @@ -1376,8 +1376,10 @@ private void resetHandlers() {
presentHandler = new BuildHandler(true);
runAndSaveHandler = new BuildHandler(false, true);
presentAndSaveHandler = new BuildHandler(true, true);
exportHandler = new DefaultExportHandler();
exportAppHandler = new DefaultExportAppHandler();
uploadHandler = new UploadHandler();
uploadHandler.setUsingProgrammer(false);
uploadUsingProgrammerHandler = new UploadHandler();
uploadUsingProgrammerHandler.setUsingProgrammer(true);
timeoutUploadHandler = new TimeoutUploadHandler();
}

Expand Down Expand Up @@ -2007,13 +2009,17 @@ synchronized public void handleExport(final boolean usingProgrammer) {
avoidMultipleOperations = true;

new Thread(timeoutUploadHandler).start();
new Thread(usingProgrammer ? exportAppHandler : exportHandler).start();
new Thread(usingProgrammer ? uploadUsingProgrammerHandler : uploadHandler).start();
}

// DAM: in Arduino, this is upload
class DefaultExportHandler implements Runnable {
public void run() {
class UploadHandler implements Runnable {
boolean usingProgrammer = false;

public void setUsingProgrammer(boolean usingProgrammer) {
this.usingProgrammer = usingProgrammer;
}

public void run() {
try {
removeAllLineHighlights();
if (serialMonitor != null) {
Expand All @@ -2025,7 +2031,7 @@ public void run() {

uploading = true;

boolean success = sketchController.exportApplet(false);
boolean success = sketchController.exportApplet(usingProgrammer);
if (success) {
statusNotice(tr("Done uploading."));
}
Expand Down Expand Up @@ -2108,55 +2114,6 @@ private void resumeOrCloseSerialPlotter() {
}
}

// DAM: in Arduino, this is upload (with verbose output)
class DefaultExportAppHandler implements Runnable {
public void run() {

try {
if (serialMonitor != null) {
serialMonitor.suspend();
}
if (serialPlotter != null) {
serialPlotter.suspend();
}

uploading = true;

boolean success = sketchController.exportApplet(true);
if (success) {
statusNotice(tr("Done uploading."));
}
} catch (SerialNotFoundException e) {
if (portMenu.getItemCount() == 0) statusError(e);
else if (serialPrompt()) run();
else statusNotice(tr("Upload canceled."));
} catch (PreferencesMapException e) {
statusError(I18n.format(
tr("Error while uploading: missing '{0}' configuration parameter"),
e.getMessage()));
} catch (RunnerException e) {
//statusError("Error during upload.");
//e.printStackTrace();
status.unprogress();
statusError(e);
} catch (Exception e) {
e.printStackTrace();
} finally {
avoidMultipleOperations = false;
populatePortMenu();
}
status.unprogress();
uploading = false;
//toolbar.clear();
toolbar.deactivateExport();

resumeOrCloseSerialMonitor();
resumeOrCloseSerialPlotter();

base.onBoardOrPortChange();
}
}

class TimeoutUploadHandler implements Runnable {

public void run() {
Expand Down
2 changes: 1 addition & 1 deletion arduino-core/src/cc/arduino/Compiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ private void runRecipe(String recipe, PreferencesMap prefs) throws RunnerExcepti
String[] cmdArray;
String cmd = prefs.getOrExcept(recipe);
try {
cmdArray = StringReplacer.formatAndSplit(cmd, dict, true);
cmdArray = StringReplacer.formatAndSplit(cmd, dict);
} catch (Exception e) {
throw new RunnerException(e);
}
Expand Down
8 changes: 3 additions & 5 deletions arduino-core/src/cc/arduino/UploaderUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
import processing.app.BaseNoGui;
import processing.app.PreferencesData;
import processing.app.Sketch;
import processing.app.debug.TargetPlatform;
import processing.app.debug.TargetBoard;

import java.util.LinkedList;
import java.util.List;
Expand All @@ -45,9 +45,6 @@
public class UploaderUtils {

public Uploader getUploaderByPreferences(boolean noUploadPort) {
TargetPlatform target = BaseNoGui.getTargetPlatform();
String board = PreferencesData.get("board");

BoardPort boardPort = null;
if (!noUploadPort) {
String port = PreferencesData.get("serial.port");
Expand All @@ -57,7 +54,8 @@ public Uploader getUploaderByPreferences(boolean noUploadPort) {
boardPort = BaseNoGui.getDiscoveryManager().find(port);
}

return new UploaderFactory().newUploader(target.getBoards().get(board), boardPort, noUploadPort);
TargetBoard board = BaseNoGui.getTargetBoard();
return new UploaderFactory().newUploader(board, boardPort, noUploadPort);
}

public boolean upload(Sketch data, Uploader uploader, String suggestedClassName, boolean usingProgrammer, boolean noUploadPort, List<String> warningsAccumulator) throws Exception {
Expand Down
2 changes: 1 addition & 1 deletion arduino-core/src/cc/arduino/packages/uploaders/GenericNetworkUploader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public boolean uploadUsingPreferences(File sourcePath, String buildPath, String
pattern = prefs.get("upload.network_pattern");
if(pattern == null)
pattern = prefs.getOrExcept("upload.pattern");
String[] cmd = StringReplacer.formatAndSplit(pattern, prefs, true);
String[] cmd = StringReplacer.formatAndSplit(pattern, prefs);
uploadResult = executeUploadCommand(cmd);
} catch (RunnerException e) {
throw e;
Expand Down
2 changes: 1 addition & 1 deletion arduino-core/src/cc/arduino/packages/uploaders/SSHUploader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ private boolean runUploadTool(SSH ssh, PreferencesMap prefs) throws Exception {
}

String pattern = prefs.getOrExcept("upload.pattern");
String command = StringUtils.join(StringReplacer.formatAndSplit(pattern, prefs, true), " ");
String command = StringUtils.join(StringReplacer.formatAndSplit(pattern, prefs), " ");
if (verbose) {
System.out.println(command);
}
Expand Down
23 changes: 7 additions & 16 deletions arduino-core/src/cc/arduino/packages/uploaders/SerialUploader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public boolean uploadUsingPreferences(File sourcePath, String buildPath, String
boolean uploadResult;
try {
String pattern = prefs.getOrExcept("upload.pattern");
String[] cmd = StringReplacer.formatAndSplit(pattern, prefs, true);
String[] cmd = StringReplacer.formatAndSplit(pattern, prefs);
uploadResult = executeUploadCommand(cmd);
} catch (Exception e) {
throw new RunnerException(e);
Expand All @@ -124,11 +124,8 @@ public boolean uploadUsingPreferences(File sourcePath, String buildPath, String
// this wait a moment for the bootloader to enumerate. On Windows, also must
// deal with the fact that the COM port number changes from bootloader to
// sketch.
String t = prefs.get("upload.use_1200bps_touch");
boolean doTouch = t != null && t.equals("true");

t = prefs.get("upload.wait_for_upload_port");
boolean waitForUploadPort = (t != null) && t.equals("true");
boolean doTouch = prefs.getBoolean("upload.use_1200bps_touch");
boolean waitForUploadPort = prefs.getBoolean("upload.wait_for_upload_port");

String userSelectedUploadPort = prefs.getOrExcept("serial.port");
String actualUploadPort = null;
Expand Down Expand Up @@ -203,7 +200,7 @@ public boolean uploadUsingPreferences(File sourcePath, String buildPath, String
boolean uploadResult;
try {
String pattern = prefs.getOrExcept("upload.pattern");
String[] cmd = StringReplacer.formatAndSplit(pattern, prefs, true);
String[] cmd = StringReplacer.formatAndSplit(pattern, prefs);
uploadResult = executeUploadCommand(cmd);
} catch (RunnerException e) {
throw e;
Expand Down Expand Up @@ -332,14 +329,8 @@ private boolean uploadUsingProgrammer(String buildPath, String className) throws
prefs.put("program.verify", prefs.get("program.params.noverify", ""));

try {
// if (prefs.get("program.disable_flushing") == null
// || prefs.get("program.disable_flushing").toLowerCase().equals("false"))
// {
// flushSerialBuffer();
// }

String pattern = prefs.getOrExcept("program.pattern");
String[] cmd = StringReplacer.formatAndSplit(pattern, prefs, true);
String[] cmd = StringReplacer.formatAndSplit(pattern, prefs);
return executeUploadCommand(cmd);
} catch (RunnerException e) {
throw e;
Expand Down Expand Up @@ -403,12 +394,12 @@ public boolean burnBootloader() throws Exception {
new LoadVIDPIDSpecificPreferences().load(prefs);

String pattern = prefs.getOrExcept("erase.pattern");
String[] cmd = StringReplacer.formatAndSplit(pattern, prefs, true);
String[] cmd = StringReplacer.formatAndSplit(pattern, prefs);
if (!executeUploadCommand(cmd))
return false;

pattern = prefs.getOrExcept("bootloader.pattern");
cmd = StringReplacer.formatAndSplit(pattern, prefs, true);
cmd = StringReplacer.formatAndSplit(pattern, prefs);
return executeUploadCommand(cmd);
}
}
2 changes: 1 addition & 1 deletion arduino-core/src/processing/app/debug/Sizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public long[] computeSize() throws RunnerException {
int r = 0;
try {
String pattern = prefs.get("recipe.size.pattern");
String cmd[] = StringReplacer.formatAndSplit(pattern, prefs, true);
String cmd[] = StringReplacer.formatAndSplit(pattern, prefs);

exception = null;
textSize = -1;
Expand Down
46 changes: 42 additions & 4 deletions arduino-core/src/processing/app/helpers/StringReplacer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,57 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.UUID;

public class StringReplacer {

public static String[] formatAndSplit(String src, Map<String, String> dict,
boolean recursive) throws Exception {
String res;
public static void checkIfRequiredKeyIsMissingOrExcept(String key, String src, PreferencesMap inDict) throws PreferencesMapException {
// If the key is not missing -> everything is OK
String checkedValue = inDict.get(key);
if (checkedValue != null && !checkedValue.isEmpty())
return;

PreferencesMap dict = new PreferencesMap(inDict);

// Find a random tag that is not contained in the dictionary and the src pattern
String tag;
while (true) {
tag = UUID.randomUUID().toString();
if (src.contains(tag))
continue;
if (dict.values().contains(tag))
continue;
if (dict.keySet().contains(tag))
continue;
break;
}

// Inject tag inside the dictionary
dict.put(key, tag);

// Recursive replace with a max depth of 10 levels.
String res;
for (int i = 0; i < 10; i++) {
// Do a replace with dictionary
res = StringReplacer.replaceFromMapping(src, dict);
if (!recursive)
if (res.equals(src))
break;
src = res;
}

// If the resulting string contains the tag, then the key is required
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not guaranteed: For example, of the string is "{foo}", a tag of "bar" is chosen (which is not in "{foo}"), but the "foo" variable contains "some string with bar", then this code will conclude that whatever key is passed is required, while it might not be.

Did you consider adding some instrumentation to replaceFromMapping to either check if a key is used, or more generically, let it return a list of keys it expanded?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can't happen because tag is guaranteed to be different from any key/value contained in the map and either from any part of the pattern recipe, see how it is choosen in the while loop just before this.

Copy link
Member Author

@cmaglie cmaglie Nov 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you consider adding some instrumentation to replaceFromMapping to either check if a key is used, or more generically, let it return a list of keys it expanded?

This is trickier than it looks :-)

  • returning the keys it expanded, won't help in detecting if a key is required, it could be that, for example, {serial.port} is required but it's not expanded because the key serial.port it's not present in the preferences map.
  • we could "detect" a list of keys that needs to be replaced using a regexp that looks at matching braces in the recipes, but there are some recipes where the braces are actually needed! see this openocd command for instance:
    tools.openocd.program.pattern="{path}/{cmd}" {program.verbose} -s "{path}/share/openocd/scripts/" -f "{runtime.platform.path}/variants/{build.variant}/{build.openocdscript}" -c "telnet_port disabled; program {{{build.path}/{build.project_name}.elf}} verify reset; shutdown"
    the part {{{build.path}/{build.project_name}.elf}} will translate to {{my/sketch.elf}} but this doesn't mean that my/sketch.elf is a required field.

if (src.contains(tag)) {
throw new PreferencesMapException(key);
}
}

public static String[] formatAndSplit(String src, Map<String, String> dict) throws Exception {
String res;

// Recursive replace with a max depth of 10 levels.
for (int i = 0; i < 10; i++) {
// Do a replace with dictionary
res = StringReplacer.replaceFromMapping(src, dict);
if (res.equals(src))
break;
src = res;
Expand Down