Skip to content

Switch to syscall.Open on Darwin#36

Open
jc-m wants to merge 1 commit into
jacobsa:masterfrom
jc-m:issue-35
Open

Switch to syscall.Open on Darwin#36
jc-m wants to merge 1 commit into
jacobsa:masterfrom
jc-m:issue-35

Conversation

@jc-m

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

Copy link
Copy Markdown

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

Comment thread serial/open_darwin.go
c_ispeed speed_t
c_ospeed speed_t
}
type Port struct{

Copy link
Copy Markdown
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

Comment thread serial/open_darwin.go
c_ispeed speed_t
c_ospeed speed_t
}
type Port struct{

Copy link
Copy Markdown
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).

Comment thread serial/open_darwin.go
@@ -231,7 +247,7 @@ func openInternal(options OpenOptions) (io.ReadWriteCloser, error) {
return nil, err
}

Copy link
Copy Markdown
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

jc-m commented Feb 15, 2018

Copy link
Copy Markdown
Author

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

jc-m commented Feb 15, 2018

Copy link
Copy Markdown
Author

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