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.

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

Added port list functionality and increased range of baud rates available via serial_posix.go #37

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

Conversation

cormacc
Copy link

@cormacc cormacc commented Dec 14, 2015

Added GetPortsList method to all implementations, shamelessly lifted
from the gobug.st/serial package and subject to the conditions of its
BSD-like license (which has been reproduced in source as per terms of said
license). Experienced stability issues with gobug.st ages back, which is
why I switched and stole.

Updated serial_posix to support full list of termios baud rates (as per
existing serial_linux implementation).

Moved posixTimeoutValues method from top level serial.go to new
serial_x.go source file, as only required for linux and posix
implementations.

A major disclaimer -- not retested on windows and OSX since some refactoring and merging upstream changes over the last day or two, as moved office and only have Linux machines to hand.

from the gobug.st/serial package and subject to the conditions of its
license (which has been reproduced in source as per terms of said
license)

Updated serial_posix to support full list of termios baud rates (as per
existing serial_linux implementation).

Moved posixTimeoutValues method from top level serial.go to new
serial_x.go source file, as only required for linux and posix
implementations.
"unsafe"
"strings"
"unsafe"
log "github.com/Sirupsen/logrus"
Copy link
Owner

Choose a reason for hiding this comment

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

Please don't include the extra dependancy.

@tarm
Copy link
Owner

tarm commented Dec 16, 2015

Thanks for the explanation.

From your description, I think it could be OK to include that functionality.

Just as OpenPort() has single common implementation with a godoc comment with platform specific openPort()'s, I think it makes sense to have that pattern for ListPorts(). The comment for ListPorts should be very explicit about it being precise on windows, but only a heuristic convenience function on unix platforms.

Also please split out the baud rate changes into a separate PR.

Added ListPorts method in serial.go that delegates to platform
implementation called listPorts. Associated godoc explicitly states that posix
implementation is just a convenient heuristic. Renamed platform-specific
implementations in serial_posix.go and serial_windows.go from
GetPortsList to listPorts for consistency.

Reverted baud-rate additions in serial_posix.go. May be included a
future PR.

Corrected erroneous exporting of function in serial_windows.go

Tidied up posix implementation of listPorts.

Externalised portname filter definition to posix_<platform>.go
@cormacc
Copy link
Author

cormacc commented Dec 17, 2015

Think that last commit addresses most of the concerns to date?

Specialised the port name regex in posix_darwin, posix_linux, posix_other:

  • "_darwin" version matches anything beginning with "cu." or "tty." as per http://pbxbook.com/other/mac-tty.html
  • "_linux" version matches any group of one or more digits beginning with "ttyS", "ttyUSB", "ttyACM" or "ttyAMA"
  • "_other" catchall version is overly inclusive -- matches anything beginning with "tty"

Multiple files for a single regex filter seems a bit excessive, but would lend itself to the per-platform definition of a baud-rate map discussed.

@tarm tarm mentioned this pull request Jan 31, 2016
@larsgk
Copy link

larsgk commented Aug 31, 2016

Maybe this could be improved a bit - to include VID, PID and PnP description string. Also, covering darwin, linux and windows.

I know, node-serialport does it quite well.. maybe some inspiration could be found there:

https://github.com/EmergingTechnologyAdvisors/node-serialport/blob/master/src/serialport_unix.cpp
https://github.com/EmergingTechnologyAdvisors/node-serialport/blob/master/src/serialport_win.cpp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants