Check for nulls when using programmer #10033
Conversation
Try to test and gracefully fail rather than throwing a NullPointerException. See arduino#10032 for an example of this exception being thrown.
| @@ -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); | |||
| } | |||
| String programmer = PreferencesData.get("programmer"); | |||
| if (programmer.contains(":")) { | |||
| String[] split = programmer.split(":", 2); | |||
| targetPlatform = BaseNoGui.getCurrentTargetPlatformFromPackage(split[0]); | |||
There was a problem hiding this comment.
I think this could again return null when an invalid package is specified in the "programmer". Or does this throw an exception instead?
There was a problem hiding this comment.
As in the other comment. Trying to be defensive AND give a unique error message for future users who ask for support.
There was a problem hiding this comment.
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?
| @@ -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); | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
here's where GetTargetPlatform returns null. I think it might be when a programmer that isn't connected is selected.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
|
The best fix for this is changing This change will trigger compile errors on every place where |
|
|
Try to test and gracefully fail rather than throwing a NullPointerException.
See #10032 for an example of this exception being thrown.
All Submissions:
Changes to Core Features:
I have not compiled or run tests or linted this. I'm hoping this is a change that someone can shepherd into an upcoming release.