nodejs / hardware

Hardware Working Group
42 stars 16 forks source link

Expose ioctl in LIBUV #10

Closed dtex closed 1 year ago

dtex commented 9 years ago

If we could set rate, bit and parity for serial communications we would no longer need a compile step for Node-Serialport installations.

According to https://github.com/libuv/libuv/issues/333 this is not possible but we'd like to get a handle and why and explore alternatives.

fivdi commented 9 years ago

ioctl is a bit of a God function. It's second parameter is more or less a device dependent blob that makes a ton of things possible, so not wanting to expose it in libuv is understandable. Also, it's a Linux/Unix function, so what should be used on Windows?

Exposing ioctl may be asking for too much. Perhaps it would be better to propose a concrete set of functions related to serial ports in libuv, for example, functions that allow the following properties to be set for a file descriptor:

nebrius commented 9 years ago

I know we kinda sorta rejected this earlier, but maybe we should revisit integrating the node-serialport apis into node core?

fivdi commented 9 years ago

If we look at the functionality provided by node today compared to 2009 when node-serialport was originally implemented, perhaps a lot of the functionality that's currently implemented in node-serialport is now available in node.

For example, could something like the following JavaScript code be used to create two streams, one stream for serial port receive and one for serial port send?

  rxstream = fs.createReadStream('/dev/ttyAMA0', {
    highWaterMark: <some number>,
    encoding: <some encoding>,
    flags: <some flags>
  });
  txstream = fs.createWriteStream('/dev/ttyAMA0', {
    highWaterMark: <some number>,
    encoding: <some encoding>,
    flags: <some flags>
  });

  rxstream.once('open', function (rxfd) {
    // rxfd available here, so call new libuv functions to set baudrate, parity, etc...
  });
  txstream.once('open', function (txfd) {

  });

These two streams could then be turned into a duplex stream with something like duplexify. At the end of the day, there's going to be more to it that this of course, but would this not be a large portion of what node-serialport is today?

nebrius commented 9 years ago

@fivdi that approach has been researched in the past by @voodootikigod, and the tl;dr is that it just doesn't provide enough of what we need.

fivdi commented 9 years ago

Can you tell me what it doesn't provide or link to an explanation of what it doesn't provide?

nebrius commented 9 years ago

ioctl

fivdi commented 9 years ago

The sample code above doesn't provide ioctl, but it's not supposed to. Instead, it call calls currently non-existent functions from libuv in the rxstream open event handler:

  rxstream.once('open', function (rxfd) {
    // rxfd available here, so call new libuv functions to set baudrate, parity, etc...
  });

These new libuv functions would be used to set the baudrate, parity, character size, etc, ...

nebrius commented 9 years ago

Ah I'm sorry, I see what you're getting at now, I missed the "new methods to set baudrate, parity, etc" bit. People are always saying "why don't you just use TTY", so it's a bit of a sore spot.

voodootikigod commented 9 years ago

Brian and Bryan,

Let me come up with the list of what we need added (sorry for delay was kinda tied up with JSConf).

Chris Williams

@voodootikigod http://twitter.com/voodootikigod | GitHub http://github.com/voodootikigod

The things I make that you should check out: SaferAging http://www.saferaging.com/ | JSConf http://jsconf.com/ | RobotsConf http://robotsconf.com/ | RobotsWeekly http://robotsweekly.com/

Help me end the negativity on the internet, share this http://jsconf.eu/2011/an_end_to_negativity.html.

On Mon, Jun 1, 2015 at 5:28 PM, Bryan Hughes notifications@github.com wrote:

Ah I'm sorry, I see what you're getting at now, I missed the "new methods to set baudrate, parity, etc" bit. People are always saying "why don't you just use TTY", so it's a bit of a sore spot.

— Reply to this email directly or view it on GitHub https://github.com/nodejs/hardware/issues/10#issuecomment-107717852.

dtex commented 9 years ago

This came up at NodeConf today and @mikeal mentioned that @saghul may already have it on his radar. Tagging both to make sure no work is being duplicated.

saghul commented 9 years ago

This is the PR you can monitor. Help in reviewing is more than welcome! https://github.com/libuv/libuv/pull/379 On Jun 13, 2015 12:51 AM, "Donovan Buck" notifications@github.com wrote:

This came up at NodeConf today and @mikeal https://github.com/mikeal mentioned that @saghul https://github.com/saghul may already have it on his radar. Tagging both to make sure no work is being duplicated.

— Reply to this email directly or view it on GitHub https://github.com/nodejs/hardware/issues/10#issuecomment-111635568.

dtex commented 9 years ago

This is tightly related to issue #2

nebrius commented 8 years ago

Small update from the libuv PR listed above. That PR was closed and a new one opened at https://github.com/libuv/libuv/pull/484

dtex commented 8 years ago

Hey gang, a new milestone has been added to libuv for the forthcoming 1.8.0 release and it does not include our favorite PR. I've added a comment requesting its inclusion but if y'all can think of additional, compelling arguments in favor of it, I don't think that would hurt to add them.

fivdi commented 8 years ago

@dtex part of this comment from the corresponding PR is:

1- uv_device_ioctl needs to go

uv_device_t is inherently platform specific, and neither the Windows or Unix implementations add anything over the native functions. Since we now have uv_fileno, users can easily use it to get the fd or HANDLE and use native APIs to do whatever is needed.

My interpretation of this is that the uv_device_ioctl function needs to be removed from the PR but it's still there. Perhaps I'm wrong though.

dtex commented 8 years ago

That may be part of it but @saghul said he's got some new notes to hand off to the requester and that it's unlikely this will land in 1.8.0

saghul commented 8 years ago

Yes, my current plan is to slim the idea even more, and make it even more flexible (specially on the Unix side). Will update the libuv issue soon.

grahamehorner commented 6 years ago

ping any update ?

saghul commented 6 years ago

Not really. Alas this has stalled.

mk-pmb commented 4 years ago

@dtex

If we could set rate, bit and parity for serial communications we would no longer need a compile step for Node-Serialport installations.

Which OSs can we target? If it's only linux, it would probably be more secure to set said params with a child_process running stty, as that would expose a lot less of the more dangerous stuff. Also since it identifies devices by filename, permissions can be more fine-grained than superuser yes/no.

@nebrius

People are always saying "why don't you just use TTY", so it's a bit of a sore spot.

Not sure whether you meant to specifically include stty; however, when people suggest stuff often, you'll get a better chance by making kind of an FAQ about what suggestions you've already tested and what they're lacking.

@grahamehorner

Would you mind sharing what you plan to use ioctl for?

greggwon commented 3 years ago

Interaction with i2c and other GPIO pins has similar issues. It's really important to manage all of these blocking moments in one way that is uniformly consistent. epoll(2) and other blocking requests for the next event should be possible to use because applications have done this for years.