Skip to content

add uv_device_t as stream on windows and Linux to handle device IO#484

Closed
zhaozg wants to merge 1 commit into
libuv:v1.xfrom
zhaozg:device
Closed

add uv_device_t as stream on windows and Linux to handle device IO#484
zhaozg wants to merge 1 commit into
libuv:v1.xfrom
zhaozg:device

Conversation

@zhaozg

@zhaozg zhaozg commented Aug 18, 2015

Copy link
Copy Markdown
Contributor

reopen #379
older #19
oldest joyent/libuv#1580

TODO: rewrite simple test code.


This change is Reviewable

@creationix

Copy link
Copy Markdown
Contributor

Would this also allow creating ptty sessions with libuv or does that need another primitive? I often have need for both creating local ptty sessions and communicating over serial to embedded devices.

Comment thread src/unix/device.c Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space is missing

@rphillips

Copy link
Copy Markdown
Contributor

Does this work on OSX? Would be nice if it did.

@creationix

Copy link
Copy Markdown
Contributor

@zhaozg Do you have an example of opening a serial port and talking to it? I don't know enough about this level of C interfaces to figure it out on my own.

@zhaozg

zhaozg commented Sep 1, 2015

Copy link
Copy Markdown
Contributor Author

@rphillips I without osx box, should have no result on osx.

@creationix I have example on windows to support serial (through luv), but I think it would easy to work on linux(because we have ffi). https://github.com/zhaozg/luv/tree/device https://github.com/zhaozg/luvit/tree/extend/deps/serial

@zhaozg

zhaozg commented Sep 4, 2015

Copy link
Copy Markdown
Contributor Author

Document ioctl calls used on all platforms serialport/node-serialport#555

@zhaozg zhaozg force-pushed the device branch 2 times, most recently from 9b03774 to 214af8f Compare September 4, 2015 16:18
@dtex

dtex commented Sep 8, 2015

Copy link
Copy Markdown

@zhaozg , I would just like to make sure I understand the state of your PR. This is far out of my area of expertise so I am not qualified to look at the code and figure it out. Forgive me if I ask obvious or overly simple questions.

This PR adds support for Windows, Linux and OSX but it has not been tested on OSX. Is that correct?

In your response above you said "should have no result on osx". I'd just like to clarify what you meant by that.

You also said "I think it would easy to work on linux(because we have ffi)". Am I correct in understanding that your pull request does not depend on FFI. You are just confident it will work because it can be done via FFI as well. Is that correct?

Thank you for your time. This PR has the potential to be a really big deal for the Nodebots community.

@zhaozg

zhaozg commented Sep 9, 2015

Copy link
Copy Markdown
Contributor Author

Hi @dtex

This PR adds support for Windows, Linux and OSX but it has not been tested on OSX. Is that correct?

Yes, I do some test on windows and Linux, but not on others.

In your response above you said "should have no result on osx". I'd just like to clarify what you meant by that.

Because not test on osx, so I don't know what result when it work on osx.

You also said "I think it would easy to work on linux(because we have ffi)". Am I correct in understanding that your pull request does not depend on FFI. You are just confident it will work because it can be done via FFI as well. Is that correct?

Yes, and device only imp abstract IO management, But all kind devices have special ioctl or high level control APIs(eg. serial termios), that out of scope of this patch, use uv_fileno get raw device fd to finish high level controls. FFI will make that things easy done in script language,(just like luajit).

So sorry for my English language to confused you.

@batmaninpink

Copy link
Copy Markdown

Further improvements here: batmaninpink@9274d73

I am using this to implement a VPN client for Windows / Linux / OS X - which works fine with the above commit.

@saghul

saghul commented Sep 9, 2015

Copy link
Copy Markdown
Member

@batmaninpink Thanks for letting us know! A couple of comments: your patch was probably targeted at an older version of this patch, some of the changes no longer apply. And the uv_device_init_fd should be a uv_device_open, analogous to uv_{tcp,udp,pipe}_open.

@zhaozg

zhaozg commented Sep 9, 2015

Copy link
Copy Markdown
Contributor Author

Good msg from @batmaninpink I'll merge some code from your version manually. thanks.

@saghul

saghul commented Sep 9, 2015

Copy link
Copy Markdown
Member

@zhaozg I suggest you don't. See my comment here: #484 (comment)

@zhaozg

zhaozg commented Sep 9, 2015

Copy link
Copy Markdown
Contributor Author

@saghul I notice that, I'll add uv_device_open.

@batmaninpink

Copy link
Copy Markdown

Doh, updated here: batmaninpink@72553a2 @saghul @zhaozg

@zhaozg zhaozg force-pushed the device branch 2 times, most recently from 66ef940 to e90c6c7 Compare September 9, 2015 12:49
Comment thread docs/src/device.rst Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/just/for example/

@stale stale Bot closed this Jan 6, 2020
@vtjnash vtjnash reopened this Jul 29, 2020
@stale stale Bot removed the stale label Jul 29, 2020
@stale

stale Bot commented Aug 21, 2020

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale Bot added the stale label Aug 21, 2020
@stale stale Bot removed the stale label Aug 29, 2020
@stale stale Bot added stale and removed stale labels Sep 19, 2020
@libuv libuv deleted a comment from cjdelisle Sep 21, 2020
@libuv libuv deleted a comment from stale Bot Sep 21, 2020
@vtjnash

vtjnash commented Sep 21, 2020

Copy link
Copy Markdown
Member

I think that's a bit aggressive to delete that comment from @cjdelisle. It's a bit off topic, and seems to have a naive-but-understandable misunderstanding of the situation and relevant politics, so it's not particularly productive, but there's a separate button to mark and hide it as such.

@stale

stale Bot commented Oct 13, 2020

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale Bot added the stale label Oct 13, 2020
@stale stale Bot closed this Jun 2, 2021
@vtjnash vtjnash reopened this Jun 2, 2021
@stale stale Bot removed the stale label Jun 2, 2021
@stale

stale Bot commented Jun 26, 2021

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale

stale Bot commented Apr 16, 2022

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale Bot added the stale label Apr 16, 2022
@zhaozg

zhaozg commented Apr 30, 2022

Copy link
Copy Markdown
Contributor Author

closed, here https://github.com/zhaozg/libuv

@zhaozg zhaozg closed this Apr 30, 2022
@calvin2021y

Copy link
Copy Markdown

no uv__process_device_shutdown_req, uv_stream_init, uv_connection_init, uv_insert_pending_req, uv_want_endgame
symbol for window with your patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.