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

SetReadTimeout sets incorrect time #141

Open
quite opened this issue Sep 15, 2022 · 5 comments
Open

SetReadTimeout sets incorrect time #141

quite opened this issue Sep 15, 2022 · 5 comments
Assignees
Labels
2.0 Breaking changes scheduled for v2.0 release bug

Comments

@quite
Copy link
Contributor

quite commented Sep 15, 2022

I currently (on v1.4.0) have to do:

err = conn.SetReadTimeout(2_000 / 100 * time.Millisecond)

To set timeout to 2 seconds. I.e. set milliseconds but divided by 100. Sorry, don't have time to look more into it right now, using the above workaround at the moment

@cmaglie
Copy link
Member

cmaglie commented Sep 17, 2022

Which OS?

@quite
Copy link
Contributor Author

quite commented Sep 17, 2022

Ah, this is on Linux. Only tried that so far.

@quite
Copy link
Contributor Author

quite commented Sep 22, 2022

Ok, have some more info. This seems to work as expected:

err = conn.SetReadTimeout(2_000 * time.Millisecond)
conn.Read(p)

The case where I needed to divide the ms timeout by 100 is this:

err = conn.SetReadTimeout(2_000 / 100 * time.Millisecond)
r := bufio.NewReader(conn)
r.Peek(1)

@cmaglie
Copy link
Member

cmaglie commented Nov 25, 2022

I see, I think the problem is that the Read function does not return an io.Timeout in case of timeout.

This is better explained here: #118

I'm filling up a list of issues for a go-serial 2.0 API, but I'm not going to work on it in the short term.

@cmaglie cmaglie added bug 2.0 Breaking changes scheduled for v2.0 release labels Jan 2, 2023
@cmaglie cmaglie self-assigned this Jan 2, 2023
@quite
Copy link
Contributor Author

quite commented May 6, 2024

An update on this. Using current go-serial v1.6.2 (also master), a timeout is not respected at all (!) even when using the regular Read(). #118 does fix this.

Regarding my comment #141 (comment) where Read() worked, but when Peek()ing a conn wrapped in a bufio Reader there was a need to divide the timeout by 100 to get the correct length -- I don't know if this is still the case.

So would be really nice to get #118 in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.0 Breaking changes scheduled for v2.0 release bug
Projects
None yet
Development

No branches or pull requests

2 participants