jputcu / serialport

Cross platform haskell library for using the serial port
Other
43 stars 36 forks source link

OS X fix for accessing serial ports on /dev/tty #3

Closed ashgti closed 12 years ago

ashgti commented 13 years ago

Made a small change to the way files are opened on posix systems for OS X to work properly. It should work fine on Linux and OS X. I was primarily encountering the problem on OS X. This change allowed me to properly open serial devices connected over USB. Just wanted to let you know.

jputcu commented 13 years ago

Hello,

Thank you for the fix, I will try it out and merge it. I'm also using USB serial devices on linux but didn't have any problems.

Joris

ashgti commented 13 years ago

It seems to hang on os x if you try to open a /dev/tty* device. However, if you open a /dev/cu* device it does not hang. I am not exactly sure why one works and the other does not, but adding O_NONBLOCK on OS X to the open lets you access the /dev/tty.usb ports.

ashgti commented 13 years ago

After some more digging I found out that on OS X if you open a /dev/tty port using open(2) it will hang until the DCD is set high. I am not working with a modem though and the device I am working with (an Arduino) doesnt seem to be able to set the DCD as far as I can tell. I may just switch to using the /dev/cu port, but I just thought I'd let you know that I had this issue on OS X when I used the /dev/tty port.

tommythorn commented 12 years ago

Yeah I got hit by this and it took me a while before I found the workaround. Didn't think to check if this was a bug in serialport. AFAICT, the fix still hasn't hit the release.

jputcu commented 12 years ago

I was afraid that my timing would be affected by opening the device non-blocking, tried it and my applications still worked.

bgamari commented 12 years ago

If I'm not mistaken, simply opening the port as O_NONBLOCK substantially changes the semantics of the interface. In particular, the patch severely breaks applications which rely on the receive timeout for read rate limiting. Applications that do this will now saturate the CPU in a tight loop of reads. Surely there must be a better way to fix this; blindly changes the semantics of a previously defined interface is extremely bad practice. I'm very surprised I am the first to notice this.

tommythorn commented 12 years ago

So, you are saying that because a fix might break it for someone (unknown), we leave it broken for everyone [on Mac]?

On Thu, Jul 5, 2012 at 9:32 AM, Ben Gamari < reply@reply.github.com

wrote:

If I'm not mistaken, simply opening the port as O_NONBLOCK substantially changes the semantics of the interface. In particular, the patch severely breaks applications which rely on the receive timeout for read rate limiting. Applications that do this will now saturate the CPU in a tight loop of reads. Surely there must be a better way to fix this; blindly changes the semantics of a previously defined interface is extremely bad practice. I'm very surprised I am the first to notice this.


Reply to this email directly or view it on GitHub: https://github.com/jputcu/serialport/pull/3#issuecomment-6783633

mitar commented 12 years ago

+1 tommythorn

There was no bug report? And also you can always do something smarter then just trying to read to rate limit. select and other event-driven approaches still work even if it is non-blocking. And if you want to rate limit, you can always do a simple: read, sleep, read, sleep loop.

jputcu commented 12 years ago

You are all right, @bgamari, I added a timeout parameter to my settings so that is probably not working as it should with O_NONBLOCK. @tommythorn and @mitar having a non-blocking implementation leaves room to pick your own blocking preference. The solution should stay cross-platform.

bgamari commented 12 years ago

@tommythorn, to clarify, that is not what I am saying. The breakage I am referring to is not hypothetical: all of my code using serialport now kills an entire core spinning in a read loop. This is because the change made what has historically been a blocking interface into a non-blocking interface; this is shift in the fundamental semantics of the library. To undertake this change just to workaround a quirk/brokenness in OS X is a very bad idea, especially without even bumping the major version number. By doing this you are silently breaking a lot of previously working code with no warning and little means of debugging.

Moreover, there is no good reason why a serial library should force the user into a non-blocking model. Blocking behavior is the default in UNIXes for a reason: it is intuitive, simple, and quite adequate for most applications. Of course, if you want to use a non-blocking model, the library should accommodate you but don't make the rest of the world bend to your decision (especially when it's a decision made for the mundane reason of working around OS X oddness).

Anyways, it sounds like @jputcu has come across the right solution. The library should have both blocking and non-blocking interfaces.

That being said, I'd encourage you to find a better solution to the OS X issue as you shouldn't have to settle on O_NONBLOCK for the entire life of the fd just to open a serial device. Perhaps you could set O_NONBLOCK during open and later clear it? This should be quite simple to accomplish; in C it should look something like,

int flags = fcntl(fd, F_GETFL);
fcntl(fd, F_SETFL, flags & ~O_NONBLOCK);
bgamari commented 12 years ago

This issue really ought to be reopened. The current "solution" is simply untenable: forcing users to use non-blocked mode just to support OS X (especially considering the library claims to be portable) is not acceptable. I still believe that the approach described above (opening as NONBLOCK and clearing after open) is the most reasonable way out of this mess. Has anyone found documentation describing exactly what OS X and why? To block on open sounds absolutely insane.

mitar commented 12 years ago

https://github.com/jputcu/serialport/pull/6#issuecomment-3445431

bgamari commented 12 years ago

@mitar, I'm not entirely sure what we are to glean from this comment.

Also, note that I've opened issue #13 calling for the introduction of a non-blocking interface to serialport. There is also a potential implementation in pull request #14.

jputcu commented 12 years ago

I feel I should revert the non-blocking patch. Reading on the web I found many posts saying to use "cu" devices in stead of the "tty" devices which block until DCD is asserted.

mitar commented 12 years ago

Don't revert it for Mac OS X. At least for my serial cable there is no "cu" device, only "tty".

bgamari commented 12 years ago

@jputcu, yes, the non-blocking patch does far more harm than good and should be reverted. As a stop-gap solution, you could #ifdef O_NONBLOCK for Mac OS X, although I'm not familiar enough with Cabal to know how to accomplish this. Really though, I'm very skeptical of using the O_NONBLOCK "solution" even temporarily. It's just too much of a hack.

@mitar and @tommythorn, all of the documentation I can find suggests that you really should be using the "cu" device unless you expect the device to be a true tty. If this device doesn't exist then it's likely this is a driver bug. What hardware are you using? Is this a common FTDI adapter? How do other bindings deal with this limitation in the lack of a cu device?

This ServerFault question appears to be relevant and suggests that you may not have proper drivers installed.

mitar commented 12 years ago

You can use CPP flag and do something similar to what I am doing here:

https://bitbucket.org/mitar/nxt/src/35b2bb45dc6c/lib/Robotics/NXT/BluetoothUtils.hs#cl-49

In cabal file it is also simple, if needed:

https://bitbucket.org/mitar/nxt/src/35b2bb45dc6c/NXT.cabal#cl-53

I am using this driver:

http://osx-pl2303.sourceforge.net/

Hm, there is written that cu device file exists. But I am almost sure that there was none on my system. I sadly don't have that at hand anymore.

Anyway, maybe the best thing would be to be able to configure this? Just allow to pass a flag enabling or disabling blocking and this is it?

bgamari commented 12 years ago

Simply opening the device with O_NONBLOCK is not an acceptable solution. Silently changing the semantics of an interface on the user of the library is simply not okay. User code is written to interact with the library in a specific way; if you suddenly change how the library behaves (without so much as a version bump) you are breaking that interaction. This brings about bugs and unpredictable behavior.

There are two ways by which we can resolve this issue,

In sum, adding interface-breaking hacks into commonly used library code is simply not an acceptable outcome and could only lead to further pain.

mitar commented 12 years ago

Ehm, simply opening the device with O_NONBLOCK is not an acceptable solution if caller passes an option to open it with that flag? Why?

mitar commented 12 years ago

it is not this library's responsibility to add hacks to accommodate the further hacks of users')

I think that library's responsibility is to make serial port easily useable. If we see that on Mac OS X there are some issues, we can say that they can be fixed somewhere else, or make a workaround for it. And we know how to do that workaround. It has been proven and working.

bgamari commented 12 years ago

@mitar, I am not opposed to the introduction of a non-blocking interface (such as the implementation I proposed in pull request #14). I will not, however, support a hack whereby the semantics of the library are silently changed by way of a build flag.

The choice of blocking/non-blocking operation should be orthogonal to the runtime platform. Consequently, the non-blocking interface isn't a replacement for a proper fix for this issue, an issue which originates somewhere else entirely in the stack. Those afflicted should realize that the problem lies in their use of the wrong device. No other serial port library (at least of those that I checked: ruby-serialport, pyserial, and perl's Device-SerialPort) attempts to work around this sort of abuse of /dev/tty* on OS X.

mitar commented 12 years ago

Why build flag? I was thinking of a runtime flag you pass to the serial port open call interface.

bgamari commented 12 years ago

@mitar, fair enough. I agree with you here; those who want to use the library in non-blocking mode should be able to do so. See pull request #14 and issue #13 for my attempt at introducing such an interface.