Tweak windows COMMTIMEOUTS settings #13

DanielJoyce · 2023-08-25T18:14:32Z

Matches the normal developer expectations around POSIX style timeouts

See ticket #7 for discussion

the normal developer experience which is mostly POSIX-style these days See ticket de-vri-es#7 for discussion

DanielJoyce · 2023-08-25T18:17:57Z

Basically, if there is data to read, it returns immediately. If there is no data, it waits until the configured timeout has passed. If there is data to write, it writes immediately. If the timeout passes when writing, it returns immediately.

This matches the expected behavior which for many developers is POSIX style. The more advanced settings could be exposed as additional config items in the windows config.

DanielJoyce · 2023-08-25T18:33:22Z

Please note, I have no way of testing this. I am just applying the changes I've seen in other serial port packages to this one.

de-vri-es · 2023-08-28T07:58:07Z

Thanks! On a first glance and from the docs, it looks good. I'll do some tests on a Windows machine too. I just need to get my hands one one first :p

de-vri-es · 2023-09-13T09:28:32Z

src/sys/windows/mod.rs

@@ -8,6 +8,8 @@ use winapi::shared::minwindef::{BOOL, HKEY};
 use winapi::shared::winerror;
 use winapi::um::{commapi, fileapi, handleapi, ioapiset, minwinbase, synchapi, winbase, winnt, winreg};

+const DEFAULT_TIMEOUT: u32 = 10;


As a default timeout, I think 10ms is too short. Normally read/writes can block for much longer before timing out. Maybe even a few seconds would be good.

The value of 10 ms was the timeout between receiving character data, where it could be sensible.

This time out, at least in POSIX land is how we wait for data to be available, or block on the serial port waiting to write some data. Usually its basically 0 for serial ports. Usually you try and read/write data immediately without a timeout, and the system tells you how many bytes you actually read/wrote.

I know windows has its own weird windows-isms, so I don't know.

Theoretically I guess windows might provide support for filling a larger buffer within a timeout window timeframe, but the purpose of this PR was to try and approximate the simpler POSIX style serial port handling, and we could then expose a windows-only api.

Again, should dig around in the PySerial sources for windows. Hmm

I think a lot of people doing serial on windows get confused because many other packages on windows ( pyserial, arduino ) basically configure the windows serial port handling to be more like POSIX.

I'm also in favor of emulating POSIX behaviour, but I still think 10ms is very short. On Linux, the default timeout for a virtual terminal seems to be infinite (or at least longer than my patience).

Note that the timeout only triggers if no data is available. If data is available, it will still return without waiting for the timeout. That's why I think 10ms is too short as a default timeout.

de-vri-es · 2023-09-13T09:29:12Z

src/sys/windows/mod.rs

+			timeouts.WriteTotalTimeoutMultiplier = std::u32::MAX;
+			timeouts.WriteTotalTimeoutConstant = DEFAULT_TIMEOUT;


Suggested change

timeouts.WriteTotalTimeoutMultiplier = std::u32::MAX;

timeouts.WriteTotalTimeoutConstant = DEFAULT_TIMEOUT;

timeouts.WriteTotalTimeoutMultiplier = 0;

timeouts.WriteTotalTimeoutConstant = DEFAULT_TIMEOUT;

DWORDMAX is not a special value for the write timeouts.

This from PySerial win32 seems to show some other behavior

# Set Windows timeout values # timeouts is a tuple with the following items: # (ReadIntervalTimeout,ReadTotalTimeoutMultiplier, # ReadTotalTimeoutConstant,WriteTotalTimeoutMultiplier, # WriteTotalTimeoutConstant) timeouts = win32.COMMTIMEOUTS() if self._timeout is None: pass # default of all zeros is OK elif self._timeout == 0: timeouts.ReadIntervalTimeout = win32.MAXDWORD else: timeouts.ReadTotalTimeoutConstant = max(int(self._timeout * 1000), 1) if self._timeout != 0 and self._inter_byte_timeout is not None: timeouts.ReadIntervalTimeout = max(int(self._inter_byte_timeout * 1000), 1) if self._write_timeout is None: pass elif self._write_timeout == 0: timeouts.WriteTotalTimeoutConstant = win32.MAXDWORD else: timeouts.WriteTotalTimeoutConstant = max(int(self._write_timeout * 1000), 1) win32.SetCommTimeouts(self._port_handle, ctypes.byref(timeouts))

elif self._write_timeout == 0: timeouts.WriteTotalTimeoutConstant = win32.MAXDWORD

Thats an attempt to make the timeout as long as possible. It's not a special value.

Also, I'm suggesting to change the WriteTotalTimeoutInterval to 0, not WriteTotalTimeoutConstant.

de-vri-es · 2023-09-13T09:29:38Z

src/sys/windows/mod.rs

@@ -85,7 +96,7 @@ impl SerialPort {
 		unsafe {
 			let mut timeouts = std::mem::zeroed();
 			check_bool(commapi::GetCommTimeouts(self.file.as_raw_handle(), &mut timeouts))?;
-			timeouts.WriteTotalTimeoutMultiplier = 0;
+			timeouts.WriteTotalTimeoutMultiplier = std::u32::MAX;


Suggested change

timeouts.WriteTotalTimeoutMultiplier = std::u32::MAX;

timeouts.WriteTotalTimeoutMultiplier = 0;

DanielJoyce · 2023-09-26T22:22:42Z

Yeah, there is about zero info on the write side on what the write timeout should be.

DanielJoyce · 2023-09-29T21:11:01Z

Adding this for notes

https://github.com/pyserial/pyserial/blob/31fa4807d73ed4eb9891a88a15817b439c4eea2d/serial/serialwin32.py

Will dig in here and see how they do it since a lot of windows users coming from python to rust are getting tripped up and it seems PySerial sets the timeout behavior to be more posix style.

jerrywrice · 2023-10-02T20:37:28Z

You may already be familiar with this, but the key info on the Windows DCB timeout settings and associated behavior is found at link https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntddser/ns-ntddser-_serial_timeouts

In my experience as an IoT/instrument software developer, the following excerpt is what I use in practice most often =>

If both ReadIntervalTimeout and ReadTotalTimeoutMultiplier are set to MAXULONG, and ReadTotalTimeoutConstant is set to a value greater than zero and less than MAXULONG, a read request behaves as follows:

If there are any bytes in the serial port's input buffer, the read request completes immediately with the bytes that are in the buffer and returns the STATUS_SUCCESS status code.

If there are no bytes in the input buffer, the serial port waits until a byte arrives, and then immediately completes the read request with the one byte of data and returns the STATUS_SUCCESS status code.

If no bytes arrive within the time specified by ReadTotalTimeoutConstant, the read request times out, sets the Information field of the I/O status block to zero, and returns the STATUS_TIMEOUT status code.**

de-vri-es · 2023-10-06T20:17:37Z

Closing this in favor of #15: that one sets a 3 second default timeout and it sets WriteTotalTimeoutMultiplier to 0 instead of u32::MAX.

It also sets the same timeout by default for the Unix implementation.

Thanks for all the discussion and suggestions!

Tweak windows COMMTIMEOUTS settings to match

82bf38f

the normal developer experience which is mostly POSIX-style these days See ticket de-vri-es#7 for discussion

de-vri-es self-requested a review August 28, 2023 08:02

de-vri-es self-assigned this Aug 28, 2023

de-vri-es reviewed Sep 13, 2023

View reviewed changes

de-vri-es closed this Oct 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tweak windows COMMTIMEOUTS settings #13

Tweak windows COMMTIMEOUTS settings #13

DanielJoyce commented Aug 25, 2023

DanielJoyce commented Aug 25, 2023

DanielJoyce commented Aug 25, 2023

de-vri-es commented Aug 28, 2023

de-vri-es Sep 13, 2023

DanielJoyce Sep 29, 2023

DanielJoyce Sep 29, 2023

DanielJoyce Sep 29, 2023

de-vri-es Sep 30, 2023

de-vri-es Sep 13, 2023

DanielJoyce Sep 29, 2023 •

edited

Loading

DanielJoyce Sep 29, 2023

de-vri-es Sep 30, 2023

de-vri-es Sep 13, 2023

DanielJoyce commented Sep 26, 2023

DanielJoyce commented Sep 29, 2023

jerrywrice commented Oct 2, 2023

de-vri-es commented Oct 6, 2023 •

edited

Loading

		timeouts.WriteTotalTimeoutMultiplier = std::u32::MAX;
		timeouts.WriteTotalTimeoutConstant = DEFAULT_TIMEOUT;

	timeouts.WriteTotalTimeoutMultiplier = std::u32::MAX;
	timeouts.WriteTotalTimeoutMultiplier = 0;

Tweak windows COMMTIMEOUTS settings #13

Tweak windows COMMTIMEOUTS settings #13

Conversation

DanielJoyce commented Aug 25, 2023

DanielJoyce commented Aug 25, 2023

DanielJoyce commented Aug 25, 2023

de-vri-es commented Aug 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielJoyce Sep 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielJoyce commented Sep 26, 2023

DanielJoyce commented Sep 29, 2023

jerrywrice commented Oct 2, 2023

de-vri-es commented Oct 6, 2023 • edited Loading

DanielJoyce Sep 29, 2023 •

edited

Loading

de-vri-es commented Oct 6, 2023 •

edited

Loading