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

Switch to syscall.Open on Darwin #36

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jc-m
Copy link

@jc-m jc-m commented Feb 9, 2018

This resolve the high cpu caused by os.OpenFile as described here: golang/go#22099

@@ -80,6 +80,22 @@ type termios struct {
c_ispeed speed_t
c_ospeed speed_t
}
type Port struct{
Copy link
Owner

Choose a reason for hiding this comment

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

Blank line before this

@@ -80,6 +80,22 @@ type termios struct {
c_ispeed speed_t
c_ospeed speed_t
}
type Port struct{
Copy link
Owner

Choose a reason for hiding this comment

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

This shouldn't be exported (i.e. capitalized).

@@ -231,7 +247,7 @@ func openInternal(options OpenOptions) (io.ReadWriteCloser, error) {
return nil, err
}

Copy link
Owner

Choose a reason for hiding this comment

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

You need to ensure that the FD isn't leaked if you return early below.

@jc-m
Copy link
Author

jc-m commented Feb 15, 2018

I'll look at your comments, but right now, it seems that there are other issues like some of the flags not being set in termios. Still trying to debug.

@jc-m
Copy link
Author

jc-m commented Feb 15, 2018

For example, the termios struct you define has a byte array of 20, which is not aligned on 64 bits. This is why the unix.termios has a padding array of 4 bytes. One the issues I see for example is that rtscts doesn't get applied to the serial port.

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.

2 participants