-
Notifications
You must be signed in to change notification settings - Fork 133
First part of autodetecting new com port #883
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.
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
Conversation
| // the port list ignoring any numbers on the end. If this specific scenario doesn't occur, | ||
| // then it will have no effect. | ||
| String oldComPort = getUploadPort(); | ||
| if (!"".equals(oldComPort)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!"".equals
will not catch null for oldComport
Instead of nesting, return fast e.g.
if(oldComport == null || oldComport.trim().isEmpty()){
return;}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As to java 8 doc this is fine (and I hope it is as I rely on this behaviour as well in many places)
https://docs.oracle.com/javase/8/docs/api/java/lang/String.html#equals-java.lang.Object-
The result is true if and only if the argument is not null and is a String object that represents the same sequence of characters as this object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String text = null;
System.out.println("".equals(text));
text = "";
System.out.println("".equals(text));
result
false
true
| if (!"".equals(oldComPort)) { | ||
| String[] comPorts = SerialManager.listComPorts(); | ||
| List<String> ports = Arrays.asList(comPorts); | ||
| if (!ports.contains(oldComPort)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same situation
if(ports.contains(oldComPort)){return;}
| List<String> ports = Arrays.asList(comPorts); | ||
| if (!ports.contains(oldComPort)) { | ||
| for (String port: ports) { | ||
| if (port.replaceAll("\\d+$", "").equals(oldComPort.replaceAll("\\d+$", ""))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this do? Zero or more digits until end of line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is... I need to test on Linux, but in the one case I've tested, the serial port seems to be /dev/cu.usbmodem14611, /dev/cu.usbmodem14111, /dev/cu.usbmodem14411, etc.
I've only tested on my Uno, I have a couple of other boards I'll test with too to validate the behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a mac some USB serial adapters look like this: /dev/cu.usbserial-A50285BI. Not sure if they change when moved to a different usb port or if the value change depending on what order they are plugged in.....
wimjongman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see comments
| // since the port value that we have has to have been set, and it has to match a value from | ||
| // the port list ignoring any numbers on the end. If this specific scenario doesn't occur, | ||
| // then it will have no effect. | ||
| String oldComPort = getUploadPort(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the name oldComport is not correct and confusing.
In Sloeber we usually use currentComPort or curComPort
| // the port list ignoring any numbers on the end. If this specific scenario doesn't occur, | ||
| // then it will have no effect. | ||
| String oldComPort = getUploadPort(); | ||
| if (!"".equals(oldComPort)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed in Slack I feel there should be a test for a key here. Do not uses space as that is a key for something else.
Just for concept review at the moment. It's a bit hackish, because we have a template at project preference save time but all the values get populated in the template at save time. But I think the concept is reasonable. It should be safe from interfering with other types of ports (i.e. remote uploads, etc, where you would not have a port value that would resemble a port retrieved from the serial port list).
I was planning to make it an optional thing, which I have not yet implemented. Also, it does not reset the proper serial monitor yet.
Would appreciate feedback as to whether this approach makes sense to you.