Set Windows timeouts to enforce non-blocking read

[larsch]

Setting ReadIntervalTimeout and ReadTotalTimeoutMultiplier to MAXDWORD in the COMMTIMEOUTS structure on Windows makes Windows behave as normally expected; a read call to the serial port will return immediately if there is any data available, or if a single byte arrives in the buffer. If no data arrives, it will timeout after the timeout specified in ReadTotalTimeoutConstant.

This fixes issues where reading from a serial port would take longer than desired when less data arrives that there is room for in the buffer passed to the read function.

See also: https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-commtimeouts

[eldruin]

(Sorry I am still unfamiliar with this crate and just joined the org)
Is this the read function you are referring to?
The problem with returning early before filling the buffer would be that users now need to know how many bits were actually read. The method I linked returns the number of bytes read, though so it should not be very problematic.
However, this is a significant behavioral change and should be prominently documented.

[jerrywrice]

This is an issue I also need resolved. I've recently joined this forum, but I'm unclear whom I should be coordinating with to offer my services/support in order to get 'on boarded' so I can be effective. I've many years of Windows development experience (c/c++), which based on comments from others seems to be in short supply for this crate. On the other hand, I'm just starting to become capable with Rust. Can you (larsch) offer any suggestions as to whom I can connect with to get oriented? I'm unclear who makes the decisions, what is being planned, what significant (historical) requirements must be observed, etc...). Jerry.

[eldruin]

@jerrywrice probably the best way to get oriented is in the chat over at: https://matrix.to/#/#serialport-rs:matrix.org

[jerrywrice]

I've also devoted effort to examining this - and have successfully tested with the same (or a very similar) patch. The patch I've tested also eliminates the Windows infinite block when the read timeout is set to 0. It works quite well. There is also an associated patch I want to discuss at the Aug. 17th support team meeting, involving the read() and how the aforementioned patch waits for the full timeout period before returning (unless all requested data arrives early). This additional patch, which I'm actively developing, potentially allows the Windows read() to more closely match the behavior of the Linux read(), and returns immediately when any data arrives - not waiting for the full timeout period to transpire. I might have some results by Aug 17th on this read() Windows patch. I think bundling these two patches together in the same release makes sense, since they involve run-time behavior change (under Windows), and I believe are more easily documented and understood by the crate's user developers, if released at the same time. Obviously requires some discussion.

[sirhcel]

Thank you very much for your effort @jerrywrice! I think I found the proposed patch now in this PR and I'm looking forward to discuss it in #121.

[agrif]

Any idea when this would be merged? This problem makes serial ports unusable (or at least unusably awkward) on windows, and it looks like every rust serial crate is dragging their feet a bit on it.

Switching to this branch changed my program from "why is this data transfer only 50 bytes per second" to working exactly the same as it does on mac and linux, without using any cursed workarounds like 1-byte BufReaders.

[vadixidav]

@sirhcel Sorry to be a bother, but any progress on this? Can this be merged as-is? I have been maintaining patches to serialport and mio-serial (see berkowski/mio-serial#38, which is waiting on this PR as well) for a few years now. There are a few other repositories where attempts have been made to fix this issue, but the PRs have been rejected (mostly related to setting the character limit to 1 at some level of the protocol stack), and it seems everything leads back here.

[eldruin]

@sirhcel have you thought about this one?

[sirhcel]

Yes @eldruin. According to the Windows documentation referenced in the description, this is the right thing to do here.

I managed to set up test_timeout.rs for this a while ago and ti got already integrated with #184. So I can finally show that with a timeout of Duration::ZERO reading data hangs (for several minutes where it should timeout in practice in several milliseconds):

>set SERIALPORT_TEST_PORT_1=COMx
>set SERIALPORT_TEST_PORT_2=COMy

>cargo test
[...]
     Running tests\test_timeout.rs (target\debug\deps\test_timeout-6d1f20037bc3d55c.exe)

running 6 tests
test test_timeout_greater_zero::case_1 ... ok
test test_timeout_greater_zero::case_2 ... ok
test test_timeout_greater_zero::case_3 ... ok
test test_timeout_zero::case_1 ...

And with the rebased fix, the test times out with Duration::ZERO:

>cargo test
[...]
     Running tests\test_timeout.rs (target\debug\deps\test_timeout-6d1f20037bc3d55c.exe)

running 6 tests
test test_timeout_greater_zero::case_1 ... ok
test test_timeout_greater_zero::case_2 ... ok
test test_timeout_greater_zero::case_3 ... ok
test test_timeout_zero::case_1 ... ok
test test_timeout_zero::case_2 ... ok
test test_timeout_zero::case_3 ... ok

test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.22s
[...]

Thank you very much for this well prepared PR @larsch! I'm sorry that it took so long to convince myself, that it is good to go.

I will prepare a new release soon.