Skip to content
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

Check for nulls when using programmer #10033

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Check for nulls when using programmer #10033

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08ea1cc
Check for nulls when using programmer
tedder Apr 11, 2020
db872fc
Fix typo in string
tedder Apr 13, 2020
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
6 changes: 6 additions & 0 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 @@ -280,12 +280,18 @@ private String waitForUploadPort(String uploadPort, List<String> before, boolean
private boolean uploadUsingProgrammer(String buildPath, String className) throws Exception {

TargetPlatform targetPlatform = BaseNoGui.getTargetPlatform();
if (targetPlatform == null) {
throw new RunnerException(tr("Gould not load platform for programmer. Ensure programmer selection is correct for the board."), false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you have any idea why targetPlatform might end up null? Is this when an invalid board is selected? I think this is still unrelated to having a programmer selected?

Copy link
Author

Choose a reason for hiding this comment

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

here's where GetTargetPlatform returns null. I think it might be when a programmer that isn't connected is selected.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think that this is related to the selected programmer, and I'm pretty sure that the IDE doesn't actually check for a connected programmer at all (it does not know how, often programmers have specific USB interfaces that the IDE does not know about).

In this case, I think the "targetPlatform" is based on the target package and platform preferences, which, along with the board preference, together indicate the selected board. So I guess the getTargetPlatform can return null when no board is selected (which might not be possible, since there is always a default board selected I think), or when the currently selected board is no longer available (because the platform was uninstalled, or the board was removed from it in an update).

So I guess the error message should talk about the board selection, rather than the programmer selection?

Have you been able to specifically reproduce this null, to verify that it is handled correctly with your PR?

Copy link
Author

Choose a reason for hiding this comment

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

I haven't set up a build environment. I was hoping this would be a start for a core maintainer to take on. If you don't think it's helpful, that's fine.

Copy link
Collaborator

Choose a reason for hiding this comment

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

AH, I see you already said you didn't compile this, sorry for missing that. I think this is helpful, but the error messages probably need a bit more tweaking to be a bit more appropriate :-)

As for a maintainer taking this on, with all the load that is already there, I would not expect that to happen quickly, but maybe some other contributor can work on top of this (I won't have time for it, probably).

}
String programmer = PreferencesData.get("programmer");
if (programmer.contains(":")) {
String[] split = programmer.split(":", 2);
targetPlatform = BaseNoGui.getCurrentTargetPlatformFromPackage(split[0]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this could again return null when an invalid package is specified in the "programmer". Or does this throw an exception instead?

Copy link
Author

Choose a reason for hiding this comment

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

As in the other comment. Trying to be defensive AND give a unique error message for future users who ask for support.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a bit confused. I think this can return null without that resulting in a proper error message (but still a NullPointerException). Maybe you misunderstood my meaning?

programmer = split[1];
}
if (programmer == null) {
throw new RunnerException(tr("Gould not load programmer. Ensure programmer selection is correct for the board."), false);
}

PreferencesMap prefs = PreferencesData.getMap();
PreferencesMap boardPreferences = BaseNoGui.getBoardPreferences();
Expand Down