bugst / go-serial

A cross-platform serial library for go-lang.
BSD 3-Clause "New" or "Revised" License
617 stars 188 forks source link

SetReadTimeout sets incorrect time #141

Open quite opened 1 year ago

quite commented 1 year ago

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 commented 1 year ago

Which OS?

quite commented 1 year ago

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

quite commented 1 year ago

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 commented 1 year ago

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: https://github.com/bugst/go-serial/pull/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.

quite commented 1 month ago

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(). https://github.com/bugst/go-serial/pull/118 does fix this.

Regarding my comment https://github.com/bugst/go-serial/issues/141#issuecomment-1255162877 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 https://github.com/bugst/go-serial/pull/118 in.