Fixed RTS/CTS flow control for Windows

[krutkay]

The previous implementation of RTS/CTS flow control would hold the RTS line high and ignore the CTS line.

This corrects those two issues. When the RTS/CTS option is enabled, the serial port will now not send data to the remote device until the CTS line is low, and will lower the RTS line when the input buffer is more than three-quarters full (as per this document).

[krutkay]

CI is failing because of a Highlight.js issue, which is a subdependency of the website package.

[reconbot]

Sorry for the delay on this, I need to do the research to figure out if this is a breaking change and then test it thoroughly.

[krutkay]

Sounds good. I appreciate the effort that you put in to maintaining this library, and understand that you'd be concerned about breaking it (and other people's code).

Let me know if there's anything I can do to help with your testing!

[reconbot]

It does change behavior for people...

Enables RTS handshaking. The driver raises the RTS line when the "type-ahead" (input) buffer is less than one-half full and lowers the RTS line when the buffer is more than three-quarters full. If handshaking is enabled, it is an error for the application to adjust the line by using the EscapeCommFunction function.

So.. with this enabled we do have those functions but they would error. It's not a commonly used feature.

It looks like RTS_CONTROL_TOGGLE is also a useful mode. In practice I don't have enough background to know which one of the 3 modes is the more desirable default (existing RTS_CONTROL_ENABLE). Unless the industry has dropped two of them completely I'm inclined to make this configurable instead of a flag.

Also what does our linux and darwin bindings do? We should match behavior if this is supported on those platforms.

[krutkay]

My understanding of flow control (mostly learned from TLDP) is that each port (device) has a "request-to-send" (RTS) output pin, and a "clear-to-send" (CTS) input pin. The RTS of one device is connected to the CTS of the other device.

The device receiving data will raise its RTS line when the connection is opened (the buffer is empty), indicating that it can receive additional data. The recipient device will hold the line high until its buffers are (almost) full, at which point it will lower the line; the line stays low until the buffers have (partially) drained, when it will raise the RTS line again to indicate it can receive additional data.

The device that's sending data will monitor the CTS line: if the line is high, it is "cleared" to send data. When the CTS line goes low, it should not send more data to the receiving device (in reality, it's okay for the sender to transmit a couple additional bytes of data).

If my understanding is incorrect, then please stop reading now, because I'm about to make a fool of myself...

---

The current (Windows) implementation is essentially equal to disabling flow control and manually setting the RTS pin high. It will never actually change the flow of data.

• RTS_CONTROL_ENABLE permanently sets the RTS line to be high, so we will never tell the remote device to stop sending us data.
• The fOutxCtsFlow flag is set to false, so the CTS line is not monitored and we will never stop sending data when the remote buffers are full.

(On a side note, if we use RTS_CONTROL_DISABLE, we should never receive any data - I don't think the ENABLE and DISABLE options are useful flags to have in the library).

I believe the flow control option should be RTS_CONTROL_TOGGLE as it will lower the RTS line when the Windows input buffer is 75% full, and will hold the line low until the buffer is at least 50% empty.
Changing to RTS_TOGGLE_CONTROL (and enabling fOutxCtsFlow) changes the library behaviour, but shouldn't adversely affect user experience (assuming their hardware correctly implements flow control):

• If they're sending or receiving small amounts of data, they will never trigger conditions where flow control is necessary, so they will never be affected by these changes. The new behaviour will mirror the old behavior.

• If they're sending or receiving large amounts of data quickly, they may be affected. These users may be experiencing data loss due to buffer overruns, whether or not they've noticed. These changes should fix any data loss issues they've been having, as we will now tell the remote device to stop sending data (or will stop sending data to the remote device).

TL;DR: I believe this PR changes the Windows behaviour to be correct (and match the Linux and Darwin behaviour).

[reconbot]

This sounds like the right thing to do. (I love tldp btw!)

[thedayofcondor]

I tested @krutkay patch on my windows version of [email protected] against my code and I can confirm it works as intended - I now get consistent behaviour across Linux and Windows (AKA HW handshaking is working as expected)

[reconbot]

lets do this!

[thedayofcondor]

I am still testing - but with that patch I managed to reliably establish a connection, download the device configuration for the first time (it's a long multipart back and forth between my pc and the device) and can stream data without losses. All working 100% of the times.

It is a massive improvement.

Just, something is not 100% right yet - communication takes about 20 seconds from Windows, 200ms from Linux, like I was working with a really tiny buffer and the HW handshaking was stopping the communication all the times.

However it may be a completely unrelated issue.

[thedayofcondor]

The latest news is... it works. It is as fast as Linux today - I am not sure what the issue was, looks like a reboot fixed it 🙄.

[reconbot]

🙌

Thank you both for your persistence and patience. There doesn't seem to be a way to do a git install with a monorepo. 🤔

So I've published v8 with the beta tag. You can install serialport@beta to get these changes.