Tokio v1.0.0 support. #33
Conversation
|
Are you looking for someone to help out with your PR? I would love to get upstream using tokio 1. |
|
I think with tokio's replacement of |
|
I've talked with the Tokio folks on discord, and there is literally no way to hook an asynchronous serial port via a windows The only solution to provide Windows support in this version of Tokio would be to run the windows port on a standard thread, and then tie it to the async resource using something like a DuplexStream: https://docs.rs/tokio/1.0.1/tokio/io/struct.DuplexStream.html Long term, we need to look into how Tokio implements the Windows iodriver to see if the comm port handle could be plugin via a similar wrapper like |
|
BTW, I've raised an issue on Tokio so that we can coordinate with them to see if there is a path forward: |
With the move to tokio 1.0, it looks likely that tokio-serial will drop support for Windows until tokio adds a Windows equivalent of `AsyncFd`. (See berkowski/tokio-serial#33) Let's stop running CI on Windows in response. Not removing cfg platform gates out of the code for now in case we are able to restore Windows as target later on.
With the move to tokio 1.0, it looks likely that tokio-serial will drop support for Windows until tokio adds a Windows equivalent of `AsyncFd`. (See berkowski/tokio-serial#33) Let's stop running CI on Windows in response. Not removing cfg platform gates out of the code for now in case we are able to restore Windows as target later on.
| return Ok(buf.advance(read)); | ||
| }) { | ||
| Ok(result) => return Poll::Ready(result), | ||
| Err(_would_block) => return Poll::Pending, |
There was a problem hiding this comment.
This breaks as the read() can return would block in which case you go pending without the async fd actually registered with the runtime; The original version with the loop is correct as read() returns would_block then the poll_read_ready() call should return Pending and everything should work nicely :).
fwiw i hit this problem in practise while playing with this branch and a fdti usb dongle in linux
There was a problem hiding this comment.
I'm confused about this: if the read returns EWOULDBLOCK doesn't that imply a bug somewhere else? We can't arrive at line 227 without being signaled by tokio that there are in fact some bytes to read. I haven't been using this branch but I have some code that is morally equiv that I have been testing with ftdi dongles (snap!) and I haven't seen this condition triggered (yet).
There was a problem hiding this comment.
It is possible to get a false positive from poll_read_ready and get the would block error, but the snippet is still incorrect — you have to loop around and call poll_read_ready again if that happens.
There was a problem hiding this comment.
I see what you are saying. Thanks for making that connection. I'd conflated the clearing of the tokio-side readiness state (which is handled by try_io) with the need to poll again to get registered. I also completely misread the sample code for AsyncFd. I guess I'd just gotten (un)lucky with my testing :)
There was a problem hiding this comment.
@mnbbrown Please look at PR: mnbbrown#1
We got bug with "hanging" usart and adding loops fixed the problem.
There was a problem hiding this comment.
I can confirm I have been seeing this bug too (when working on tokio-modbus's update to tokio-1.0) and rfael's patch fixed it.
|
As the discussion on the tokio side for windows support is still ongoing; Is there a chance this could move foward without windows support as a step for e.g. tokio-serial 5.0 ? |
|
I'll finally have time to work on this, but first up is getting berkowski/mio-serial#23 resolved. |
|
Hey. I am currently looking into the Tokio situation on windows. Would named pipes alone resolve the windows support, or do you need some sort of |
|
Any updates on this @berkowski or @Darksonn? Getting serial working again with Tokio on Windows would be something my firm would be willing to financially sponsor. If this would help move this along in any way, please reach out to me: adam@stepfunc.io |
|
Named pipes are coming along nicely in tokio-rs/tokio#3760, but I haven't heard anything from @berkowski in several months, so it's unclear what is going to be happening on the tokio-serial side. |
|
Named pipes just got released https://github.com/tokio-rs/tokio/releases/tag/tokio-1.7.0 🎊 |
|
I've implemented the missing Windows part in https://github.com/enlyze/tokio-serial/tree/v4.4.0_serialstream_multiplatform This works well for me under Windows and Linux. |
|
beta support for tokio 1.X available on crates.io with release 5.4.0-beta1. |
Opening a PR incase it's helpful. New to rust so not sure if it's the best implementation.
Draft because:
Addresses #31 and #32