Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proper Flow Control #203

Closed
giseburt opened this issue Aug 13, 2013 · 28 comments
Closed

Proper Flow Control #203

giseburt opened this issue Aug 13, 2013 · 28 comments
Labels
feature-request Feature or Enhancement

Comments

@giseburt
Copy link
Contributor

I'm planning on adding more complete flow control options, since we're needing them in the TinyG node module, but I'm trying to figure out a clean way to handle the options.

The problem is that the current code assumes "flowControl" is true or false, and this is a bad assumption. There are two overall popular types, and one has two flavors, making three options to turn on or off.

There is "software" flow control -- which is actually handled in the software of the device with the serial port on it, so this would be in the FTDI or ATMega16U4 on recent Arduinos, and is irrelevant in the Leonardo or Due where the USB protocol handles this. (The current firmware for the Arduinos do not support these options, and some of them abuse the hardware RTS line to reset for bootloading.) The flavor of software flow control I am referring to is called XON/XOFF. Actually, the XON portion and the XOFF portion are separate options that may be be used independently or combined.

Then there is "hardware" flow control. This uses two additional lines along side of RX and TX. These are sometimes labelled as RTS/CTS. They do the same thing as XON/XOFF, except they do it outside of the character stream. Again, this is handled by the device (or driver, in embedded linux) that directly talks over a serial port, and only requires setting a flag in the driver. This is the type the {flowControl: true} turns on now, except for on windows.

There are two ways I can see to go forward without breaking backward compatibility:

  1. Alter the flowControl value to have true mean RTS/CTS flow control on, and false to mean all flow control off. Then also offer number values with constants that can be bitwise-OR'd together. Something like: {flowControl:SerialPort.flowControlXON|SerialPort.flowControlXOFF} and then SerialPort.flowControlRTSCTS would map to 1 and SerialPort.flowControlDisabled would map to 0. This way allows the old code to work, and new code could be more verbose.
  2. Add completely different options. Like this: {flowControlXON: true, flowControlXOFF: true}. I'm less of a fan of this method, but in some ways it's cleaner.

In any case, I'll happily implement these (along with documenting them) and make a pull request. I'm just hoping for stylistic feedback first.

@voodootikigod
Copy link
Collaborator

I feel like for the target audience (JS developers) verbosity and simplicity is a good thing, I would possibly argue that we avoid exposing bitwise operations in favor of explicit option sets. Thoughts?

@giseburt
Copy link
Contributor Author

Yeah, I suppose it is very un-Javascript to use bitwise operators. :-)

Since there are three values that could be added in any combination, we could either make it an array or a subobject.

Array: {flowControl:["XON","XOFF"]} or {flowControl:["RTSCTS"]} (I could also add stty-style options in this case. {flowControl:["ixon","ixoff"]} or {flowControl:["crtscts"]}.)

Subobject: {flowControl:{xon:true, xoff:false, rtscts:false}} I'm not as fond of this one. It is more verbose, but not it a way I feel is necessary.

I personally vote array style. Actually, seeing #202 reminds me that there should be options to set RTS and DTR on connect. (This would be for the Arduino reset functionality. It's compulsory on OS X and Linux, but not so much on Windows.)

(PS: Why did my comment close this issue?)

@giseburt giseburt reopened this Aug 13, 2013
@voodootikigod
Copy link
Collaborator

You probably hit Close & Comment not just comment (green).

Ping me on IM voodootikigod I am actually pickign this area apart and revieiwng how ruby-serial and pyserial do it, figure borrow from the established.

@voodootikigod
Copy link
Collaborator

@giseburt I believe at the termios.h level I can just set it as iXON or iXOFF so the tty v. stty point is moot (I think, would love a second glance) http://pubs.opengroup.org/onlinepubs/007908799/xsh/termios.h.html

@voodootikigod
Copy link
Collaborator

Other options will be to have this:

{ xon: true, //overrides xonxoff xoff: true, //overrides xonxoff xonxoff: true, flowcontrol: true, // same as its now rtscts: true, // duplicate of flowcontrol, but better long term dsrdtr: true //if set, use else use rtscts/flowcontrol indicator. }

Thoughts?

@giseburt
Copy link
Contributor Author

By stty I just meant the user land stty command.
http://linux.die.net/man/1/stty

It uses the strings 'ixon', 'ixoff', and 'crtscts.' I figured those could
be added as aliases. It's not important.

On the windows side there are actually more options:
http://msdn.microsoft.com/en-us/library/bb202767.aspx

Yes, on unix it would be a matter of adding the IXON and IXOFF options.
Generally, you won't want both CRTSCTS and IXON|IXOFF.

I suppose there's also IXANY, but that's a separate option. (That would
cause problems with TinyG, for example.) I mention it because it could be
useful to others, and we're already there mike changes...

On Tuesday, August 13, 2013, Chris Williams wrote:

@giseburt https://github.com/giseburt I believe at the termios.h level
I can just set it as iXON or iXOFF so the tty v. stty point is moot (I
think, would love a second glance)
http://pubs.opengroup.org/onlinepubs/007908799/xsh/termios.h.html


Reply to this email directly or view it on GitHubhttps://github.com//issues/203#issuecomment-22572522
.

@giseburt
Copy link
Contributor Author

@voodootikigod I somehow didn't see that last message from you, sorry.

Yeah, that works. I would also vote to deprecate the (undocumented) flowcontrol option, since it's redundant and vague what form of flow control it actually enables.

@D1plo1d
Copy link

D1plo1d commented Aug 13, 2013

So I'm not familiar with rtscts/flowcontrol but I am familiar with DTR. Would setting dsrdtr: true then reset the arduino on opening the serial port? Ie. would it flash DTR on for 200ms and then off again?

Also how would I reset the arduino/toggle the DTR bit presently?

giseburt added a commit to giseburt/node-serialport that referenced this issue Aug 19, 2013
Flow control options haven't been added to windows yet.
@D1plo1d
Copy link

D1plo1d commented Sep 15, 2013

@giseburt how would I toggle DTR using your patch?

@steffenmllr
Copy link

@giseburt same question here, I need to setRTS to 0 on connect - is that possible with node-serialport?

@voodootikigod
Copy link
Collaborator

Release 1.2.1 has all these changes. and the fix for close().

@steffenmllr
Copy link

Thanks @voodootikigod. On the propobility of sounding stupid, but how do I pass in the RTS(0) on connect?

var serialPort = new com.SerialPort("XXXXXXX", {
    baudrate: XXXXX,
    rtscts: 0
});

@giseburt
Copy link
Contributor Author

giseburt commented Oct 2, 2013

Oops! The setRTS() and setDTR() didn't make it into my pull request.

@steffenmllr, do you need RTS to never go high during connect, or to be switched to low on connect? On unixy OSs, I don't think we can prevent it from going high briefly, because the OS toggles it automatically.

I'll but those in another pull request. Probably late this evening.

@steffenmllr
Copy link

I need it to be switched to setRTS(0) on connect. I'm trying to port existing python code to node-serial

    def connectionMade(self):
        LOG.info("Reader connectionMade")
        self.transport.setRTS(0) # http://pyserial.sourceforge.net/pyserial_api.html#serial.Serial.setRTS
        self.inReset = True
        ....

@Humancell
Copy link

I'm confused about the current status of SetDTR ... is it working, or removed? I'm looking for an example on how to use it.

We've got a USB Serial device that uses DTR as a rest pin ... so I need to be able to explicitly toggle DTR high and low.

Is it possible to use SetDTR after opening the port to do this?

@D1plo1d
Copy link

D1plo1d commented Oct 20, 2013

@Humancell I've been trying to find out the same thing. In the meantime I have a work around / massive hack: I wrote some C code that toggles DTR that I call from node as a child process after I open the serialport using node-serialport.

My horrible hack work-around: http://git.io/lLU0Ig

@JayBeavers
Copy link
Collaborator

Where did we land with this? Is there more work pending?

I'll throw out this design for the C# serial port API for this. They went with a four value enum:
http://msdn.microsoft.com/en-us/library/system.io.ports.handshake.aspx

Here is Google's take on the matter:
http://developer.chrome.com/apps/serial.html#type-ControlSignalOptions

@camsoupa
Copy link

{
xon: true, //overrides xonxoff
xoff: true, //overrides xonxoff
xonxoff: true,
flowcontrol: true, // same as its now
rtscts: true, // duplicate of flowcontrol, but better long term
dsrdtr: true //if set, use else use rtscts/flowcontrol indicator.
}

I think this approach is fine. Very interested in dsrdtr option.

@stationweb
Copy link

I'm very interested too.
I need to change dtr and rts after the serial is opened for an initialization process.

@bcomnes
Copy link

bcomnes commented Mar 11, 2014

On a side note, it would be super rad to have manual control over DRT and RTS. The issue for that would be #75. @giseburt it sounded like maybe you were interested in see in this?

Unfortunately, I don't know C so this is probably best for someone else to handle.

@danbim
Copy link

danbim commented Mar 19, 2014

Just to resurrect this issue...

I also need to set DTR and RTS manually for initialization. My device doesn't do anything without it.

@deandob
Copy link

deandob commented Aug 10, 2014

Hi, any progress? I also need DTR manual control.

@bhaveshgohel
Copy link

Hi guys, Is this issue solved? or Any workaround for right now?

@bcomnes
Copy link

bcomnes commented Sep 9, 2014

Not solved. It needs someone to futz with some C to get it implemented. A workaround is using windows (if you need DTR set to high).

@bhaveshgohel
Copy link

@bcomnes : Yes I am using windows, Can you provide some insight to use DTR? some code will be helpful.

@bcomnes
Copy link

bcomnes commented Sep 15, 2014

@bhaveshgohel Sorry, I was vague. In my particular application, I need the DTR pin to be set to high. For some reason, I have found windows to always set this pin to high by default with this library. On OSX/Unix DTR is set to low always. I'm not a serial expert so I can't explain why this is but I used Cool Term to isolate my issues down to the state of the DTR pin. There are also hardware hacks depending on your particular need. Also your needs may be totally different than my simplistic need but thats what I know ;)

@bhaveshgohel
Copy link

@bcomnes Thanks. :)
It will be very helpful if someone come up with this solution to manual control DTR or CDC or RTS or something similar. This feature is highly needed. 😕

@voodootikigod
Copy link
Collaborator

Closing this to consolidate ticket details for only adding support for manual control DTR or CDC or RTS ticket #384.

@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Feature or Enhancement
Development

No branches or pull requests