#60 Setting timeouts is too disruptive

[Peter Creath]

This is more of an enhancement request than anything else.

Currently, setTimeout internally results in a full _reconfigurePort, which is quite inefficient in the middle of a read loop when you're trying to manage timeouts precisely. The full reconfiguration can also (occasionally) cause data in the buffer to be dropped, when all that is desired is control over the read/write timeout.

Proposed solution:

1) Move the platform-specific timeout configuration into a separate function that is called by setTimeout and _reconfigurePort. This way, a call to setTimeout will no longer call the full _reconfigurePort.

and

2) (optional, if the above is done) Add an optional parameter to read() and write() that allows the specified timeout to be used for that read/write. For platforms that separate timeouts from the read/write calls, this will result in a call to the function in (1), and for others it will simply override the internal _timeout value when issuing the low-level read/write.

I can provide a patch if desired.

[Chris Liechti]

I've looked at this but it seems that for example the inter char timeout on posix requires a tcsetattr call, other platforms have their own specialties. I'm not sure if all platforms support changing timeouts without dropping data or disturbing flow control.

Longer read timeouts are always possible by calling read multiple times, optionally checking time().

In general I think that timeouts should be to handle errors and not be part of the normal protocol. The accuracy of timeouts across platforms and on desktop operating systems varies and usually they should be in the range of seconds.

Therefore i'm -1 on that one.

[Peter Creath]

Hmm. That's an interesting point about inter-char timeouts.

The problem with building longer reads by calling read multiple times is that you end up busy-polling the port. Here's the annoying case:

1) You want to read for up to 4 seconds. So you said a timeout of, say, 1 second so that you call read no more than 4 times.
2) You start receiving data at 3.5 seconds.

Now what do you do? If you change the read timeout to 0.5 seconds you might lose data, and if you don't change it, you go past 4.0 seconds. On the other hand, if you start getting more data, you end up having to update the timeout after every read() (or use a very very small timeout), which is extremely inefficient.

How about an optional read-timeout parameter to read()? That would let a client make sure that read() won't block long past the desired time, without having to reconfigure the port repeatedly.

[Peter Creath]

It looks like inter-char timeouts are handled entirely separately from read/write timeouts, so they shouldn't affect the original solutions I proposed.

But ultimately, all I want to do is control the read timeout so that reads don't block indefinitely. From a quick look, it appears that this is possible across all the supported platforms:

serialcli.py:
self._port_handle.ReadTimeout = int(self._timeout*1000)

serialjava.py:
self.sPort.enableReceiveTimeout(self._timeout*1000)

serialposix.py:
ready,_,_ = select.select([self.fd],[],[], self._timeout)

serialwin32.py:
win32file.SetCommTimeouts(self.hComPort, timeouts)

(sermsdos.py doesn't use timeouts)

None of these require massive changes to the port configuration -- they just change the behavior of the read operation.

(For that matter, it looks like this would work for writes as well.)

[Peter Creath]

I'm boosting the priority of this, because with the advent of non-standard (and higher) baud rates, it's not longer just an enhancement request. The current method of setting timeouts _is_ causing garbled data.

In particular, the use of a full _reconfigurePort after every setTimeout is causing data to get dropped. As previously, I recommend splitting out the timeout-setting code from _reconfigurePort into a per-platform _setPortTimeout that gets called by both _reconfigurePort and setTimeout. That way setTimeout won't be causing quite so much chaos every time the timeout is updated.

I tested the concept by skipping the _reconfigurePort in setTimeout (in serialposix), and all the data errors went away.

Let me know if you'd like me to submit a patch with the full fix.