goburrow / serial

Go (golang) serial library for modbus
https://github.com/goburrow/modbus
MIT License
191 stars 90 forks source link

Default timeout in Windows is currently undefined behavior #5

Closed xiegeo closed 7 years ago

xiegeo commented 7 years ago

In short: Config.Timeout = 0, Linux does not time out, Windows times out at 0.5 seconds in my case.

When Config.Timeout is left as default (0), the behavior on Windows and Linux is different. On Linux, it is infinite (implemented with null *timeval for select). But on Windows it skips SetCommTimeouts which according to https://msdn.microsoft.com/en-us/library/aa450505.aspx:

An application must always set communication timeouts using the COMMTIMEOUTS structure each time it opens a communication port.

If this structure is not configured, the port uses default timeouts supplied by the driver, or timeouts from a previous communication application.

By assuming specific timeout settings when the settings are actually different, an application can have read/write operations that never complete or complete too often.

In my test, a read with Config.Timeout = 0 on Windows returns after a timeout of 0.5 seconds. It should not timeout.

nqv commented 7 years ago

Thanks for you report, will fix when time allows 👍

nqv commented 7 years ago

@xiegeo I've added a patch on windows-timeout branch. Haven't tested yet as I need to install a Windows.

Would be great if you can test the branch before I manage to obtain a copy of Windows VM :)

xiegeo commented 7 years ago

Thank you for the fast patch, but not working yet.

master branch: uses the last time out set. (if i run connect once with 10s timeout, then again with 0 timeout, it stays 10s.

windows-timeout branch: always resets timeout to very short when timeout is 0 (to fast to tell from log).

nqv commented 7 years ago

Sorry, I tried to make it like libmodbus. I force pushed to revert to current behaviour. I'm not sure how to set infinity timeout though. Would you mind checking it again @xiegeo (you might need to do git pull -f on that branch) Thank you.

xiegeo commented 7 years ago

It behaves the same as in my last post.

xiegeo commented 7 years ago

according to the docs https://msdn.microsoft.com/en-us/library/windows/desktop/aa363190(v=vs.85).aspx

A value of zero for both the ReadTotalTimeoutMultiplier and ReadTotalTimeoutConstant members indicates that total time-outs are not used for read operations.

0 should be used for all TotalTimeout for no timeout, and ReadIntervalTimeout could be something like the 3.5 char delay in Modbus RTU packet termination.

and this stackoverflow answer also suggest the same:

The behavior you want will come from:

cto.ReadIntervalTimeout = 10;
cto.ReadTotalTimeoutConstant = 0;
cto.ReadTotalTimeoutMultiplier = 0;

It blocks arbitrarily long for the first byte (total timeout is disabled by setting the latter two fields to zero, per the documentation), then reads up to the buffer size as long as data is streaming in. If there's a 10ms gap in the data, it will return with what has been received so far.

nqv commented 7 years ago

I'm a bit confused. For no timeout, should ReadIntervalTimeout be set to 0 as well to disable read timeout? I updated the branch, will test it this weekend.

xiegeo commented 7 years ago

@nqv Any updates?

I made a tester to read the last set timeout to see how other Modbus applications set it https://github.com/xiegeo/serial/blob/xiegeo-experimental/timeout_windows_test.go

nqv commented 7 years ago

@xiegeo I'm really sorry about this. I haven't managed to install an Windows VM. By the way, have you tried my last changes (with set everything to zero and will hopefully disable timeout)?

Please bear with me, I'll try with your test as soon as possible. Thank you

xiegeo commented 7 years ago

I think it does disable timeout.

But it is not the working for me for other reasons. After some head scratching debugging sessions, I think a read without timeout was also blocking my writes on Windows, but not blocking on Linux. This breaks my usage of disabled timeout.

nqv commented 7 years ago

I'm wondering if it's still blocked if you set timeout to a very big number (e.g 24 hours)? May be it's another issue, not related to timeout

xiegeo commented 7 years ago

It looks like the read just is blocking any writes no matter the timeout.

https://msdn.microsoft.com/en-us/library/ff802693.aspx#serial_topic4

if one thread were waiting for a ReadFile function to return, any other thread that issued a WriteFile function would be blocked.

xiegeo commented 7 years ago

I just fund github.com/tarm/serial, which uses Overlapped I/O. I think it is a good reference for making the windows behavior same as linux.

Edit: I tested tarm/serial. The Overlapped I/O does solve the problem with read blocking write, but it is not fully consistent across os. A read that was waiting for data would return the first byte first on windows, then the rest on the next read, instead of the full data (either the full packet or full buffer) all at once. I'll see if I can play with timeout or overlapped settings to get windows to act like on linux.