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

Refactoring of ArduinoSerial class and serial port access for clarity #179

Closed
wants to merge 12 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@kigster
Copy link
Contributor

kigster commented Jul 12, 2014

I was trying to follow existing code ideas and lines, but made a bunch of tidying and cleanup refactors. Here is a short list:

  • Adding explicit warning about needing to manually create /var/lock on Mac OS-X
  • Reformatted all of my current and previous changes (including last PR) according to codeformat.xml file
  • Switched back to ERROR the case when port is simply not found otherwise the flow is really confusing
  • extracted java waitFor(int delay) method that's reused across ArduinoSerial class
  • extracted hardcoded board names into String constants for ease of mainteneance
  • added some defensive code around preventing Null Pointer Exceptions I was getting
  • renamed methods and variables according to Java's published naming standard
  • rewrote method java waitForComPortsToReappear() to hopefully improve readability, and made it shorter
  • introduced new optional variable A.UPLOAD.FORCE_NO_WAIT_FOR_UPLOAD_PORT if set to "true" will skip waiting for ports even for Arduino Esplora, etc.

Sorry for such a big PR! I was in the car and couldn't stop :)

kigster added some commits Jul 12, 2014

Adding explicit instructions about /var/lock
 * this cost me at least 2 hours of wrestling 
   until I stumbled upon this thread:
   #129

Adding a check for for MacOS and /var/lock being
accessible so that nobody has to waste anymore time :) 
Once you run the command, subsequent upload succeeds.
Switch back to ERROR, otherwise how to know?
 * Since the change was made so long ago I am hoping we can go 
   back to having this useful descriptive error.  It's really 
   nice to be reminded when the port setting may have remained
   from a previous project/setup.
Refactoring & tidying of much of the serial por
 * If port not found, switching back to error.
   Since the change was made so long ago I am hoping we can go
   back to having this useful descriptive error.  It's really
   nice to be reminded when the port setting may have remained
   from a previous project/setup.

 * Also refactor and extract methods in the ArduinoSerial class for
   clarity.

 * Also renamed variables and method names to be consistent with
   Java standards.
Add A.UPLOAD.FORCE_NO_WAIT_FOR_UPLOAD_PORT var
 - setting this to true allows fully skipping waiting for 
   comm ports to disappear/appear and thus save a few seconds.
Refactoring & tidying of much of the serial por
 * If port not found, switching back to error.
   Since the change was made so long ago I am hoping we can go
   back to having this useful descriptive error.  It's really
   nice to be reminded when the port setting may have remained
   from a previous project/setup.

 * Also refactor and extract methods in the ArduinoSerial class for
   clarity.

 * Also renamed variables and method names to be consistent with
   Java standards.
Add A.UPLOAD.FORCE_NO_WAIT_FOR_UPLOAD_PORT var
 - setting this to true allows fully skipping waiting for 
   comm ports to disappear/appear and thus save a few seconds.
@jantje

This comment has been minimized.

Copy link
Member

jantje commented Jul 12, 2014

I give 100% priority to #67 right now so I will not accept these changes. Sorry for that.

@jantje jantje closed this Jul 12, 2014

@kigster

This comment has been minimized.

Copy link
Contributor

kigster commented Jul 12, 2014

That's fine, I'll use my fork until jssc is ready.

@jantje

This comment has been minimized.

Copy link
Member

jantje commented Jul 12, 2014

that is a great advantage of open source :-)

@kigster

This comment has been minimized.

Copy link
Contributor

kigster commented Aug 3, 2014

@jantje Curious if you'd be interested in merging this, since #67 is not moving forward?

@jantje

This comment has been minimized.

Copy link
Member

jantje commented Aug 3, 2014

I hope to have some time free in the comming 2 weeks to look at the jssc switch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment