kevinmehall
commented
1 year ago As for what the API should be instead, I think the design space should be explored further, but here's one idea, expanded from one of my suggestions on rusb:
impl Device {
/// Get a Pipe for the specified endpoint
///
/// If a pipe for this endpoint already exists and has not been
/// dropped, an error is returned.
fn pipe(&self, endpoint: u8) -> Result<Pipe, ClaimPipeError>,
}
/// Buffer is essentially Vec<u8>, with an allocated capacity, and a portion that may
/// be initialized and safely accessed.
///
/// Could just *be* Vec<u8> but may be better to have a separate type for things like
/// DMA-able memory, an auto-ZLP flag, extra fields for multipacket ISO transfers, etc.
struct Buffer{ ... }
impl Pipe {
/// Submit a new transfer on the endpoint.
///
/// For an IN endpoint, the transfer size is set by the *capacity* of
/// the buffer, and the length and current contents are ignored. The
/// buffer is returned from a later call to `complete` filled with
/// the data read from the endpoint.
///
/// For an OUT endpoint, the contents of the buffer are written to
/// the endpoint.
pub fn submit(&mut self, buf: Buffer) -> Result<(), TransferError>;
/// Block waiting for the next pending transfer to complete, and return
/// its buffer or an error status.
///
/// For an IN endpoint, the returned buffer contains the data
/// read from the device.
///
/// For an OUT endpoint, the buffer is unmodified, but can be
/// reused for another transfer.
pub fn complete(&mut self, timeout: Option<Duration>) -> Result<Buffer, TransferError>;
/// Get the number of transfers that have been submitted with
/// `submit` that have not yet been returned from `complete`.
pub fn pending_transfers(&self) -> usize;
/// Get the number of transfers that have completed and are
/// ready to be returned from `complete` without blocking.
pub fn ready_transfers(&self) -> usize;
/// Cancel all pending transfers on the endpoint pipe.
/// TODO: maybe this should be on the `Device` or an object separable from the `Pipe`
/// so it can be called from another thread, and cause a blocking `complete` call to
//// immediately return.
fn cancel_all(&mut self) -> Result<(), TransferError>;
}
fn read_example(device: &Device) -> Result<(), anyhow::Error> {
let pipe = device.pipe(0x81)?;
while pipe.pending_transfers() < 8 {
pipe.submit(Buffer::with_capacity(8192));
}
loop {
let buf = pipe.complete()?;
process_data(&buf);
pipe.submit(buf);
}
}
fn write_example(device: &Device) -> Result<(), anyhow::Error> {
let pipe = device.pipe(0x02)?;
loop {
let mut buf = if pipe.pending_transfers() < 8 {
Buffer::with_capacity(8192)
} else {
pipe.complete()?
};
fill_data(&mut buf);
pipe.submit(buf);
}
}One advantage of the nonblocking submission / blocking wait design is it means that you can call into OS-specific event waiting APIs from the blocking .complete() method directly instead of needing a background thread or an integration with an IO reactor to receive events. If there's a need to integrate with the Futures ecosystem, there could be an async version of the complete method returning a Future, or maybe have the Pipe itself implement Stream.
ktemkin
Several years ago, I prototyped async support in libusb-rs, and others have been slowly getting a version of it into rusb.
While that patch initially didn't use
Futurebecause it was written before Rust had async/await, my experience with helping review other rusb async proposals is that trying to map libusb-like async callback APIs directly to returning a RustFuturerepresenting the completion of a specific transfer is not an optimal API.The main thing everyone does with libusb's async API is queuing several transfers on the same endpoint to keep the bus busy and maximize throughput. This pattern is kind of awkward to implement on top of an API where each transfer returns its own
Future, though. There's a mismatch between having multipleFutures to poll and the fact that they're supposed to complete in FIFO order. The futures APIs are more oriented towards loosely coupled independent tasks running concurrently. This usage actually looks more like aStreampolling for the next item than aFuturecompleting once, but with the extra complexity of explicitly handling both completion and submission.futures::FuturesUnorderedgets you part of the way there as a way to wait for any of multiple futures, but that requires an extra dependency and some plumbing of futures to implement this most common pattern.The other use case for an async API is integrating with an external event loop. With libusb, this is only truly possible on Linux (Windows and macOS need a background thread internal or external to libusb for event handling), and their C API for this is quite error-prone. In my opinion, there's also not a huge motivation for this with USB. Unlike with network servers, userspace USB drivers have a small and bounded number of endpoints and can easily launch a few dedicated threads to service them. It's still possible to integrate with Rust async IO reactors and executors if desired, but may not be the main use case to optimize for.
Cancellation is another thing to think about that it looks like usrs skips so far. Libusb pretends to have an API to cancel an individual transfer, but on Windows and MacOS, this actually cancels all pending transfers on the endpoint because that's what the underlying OS APIs allow (though cancelling all transfers is what you want most of the time anyway). Even where supported, the libusb-style individual cancellation has subtle pitfalls. For instance, you need to be sure to cancel OUT transfers in reverse order to avoid race conditions where some of the later writes make it to the device while previous transfers were cancelled. In Rust
Futureterms, where drop is cancellation, that means you can't just drop a collection of transfer futures to cancel all of them --FuturesUnordereddoesn't specify drop order, andVecdrops in forward order.The other issue that kept coming up with
Futurefor USB transfers is theFutureAPI's inability to deal with borrowed data soundly in completion-based IO because it's safe tomem::forgeta future without calling its destructor. Your code sidesteps this one by using only owned buffers, and while a lot of the allocation and locking usrs performs looks like it could be avoided, I think some form of owned buffers are the way to go.I'm really excited about this project because a libusb replacement in Rust is something I've considered doing and started researching multiple times. Glad to see you actually doing it, and hopefully I can help in some way.