Conversation
| #[allow(unused)] | ||
| pub(crate) fn set_configuration( | ||
| &self, | ||
| configuration: u8, | ||
| ) -> impl MaybeFuture<Output = Result<(), Error>> { | ||
| // It doesn't look like libusb does this either since the model | ||
| // of how ugen works doesn't match this | ||
| Blocking::new(move || todo!("Not supported")) | ||
| } | ||
|
|
||
| pub(crate) fn reset(&self) -> impl MaybeFuture<Output = Result<(), Error>> { | ||
| // Another API that isn't as easily exposed via ugen | ||
| Blocking::new(move || todo!("Not supported")) | ||
| } |
There was a problem hiding this comment.
I didn't find either of these in ugen
| pub fn set_alt_setting( | ||
| self: Arc<Self>, | ||
| _alt_setting: u8, | ||
| ) -> impl MaybeFuture<Output = Result<(), Error>> { | ||
| // doesn't work exactly the same here | ||
| Blocking::new(move || todo!("Not implemented")) | ||
| } | ||
|
|
||
| pub fn get_alt_setting(&self) -> u8 { | ||
| self.state.lock().unwrap().alt_setting | ||
| } |
There was a problem hiding this comment.
Another case where I don't think this works the same way with ugen?
jamesmunns
left a comment
jamesmunns
left a comment
There was a problem hiding this comment.
First pass over things, I'm gunna take a closer look at transfer.rs, as there's a lot of unsafe going on there.
I'm not super familiar with ugen or nusb (platform-wise, at least), so take my comments with the appropriate grain(s) of salt.
| Ok(n) => { | ||
| use crate::transfer::SETUP_PACKET_SIZE; | ||
| // TODO(AJM): Is this: | ||
| // 1. `SETUP_PACKET_SIZE..(SETUP_PACKET_SIZE + n)`, OR |
There was a problem hiding this comment.
@labbott I could probably use help figuring out what the right thing is for this (and probably some other places where we do similar slicing)
There was a problem hiding this comment.
I have been bitten by this while attempting to work on this code. The intention is that the buffer is allocated to contain the setup packet SETUP_PACKET_SIZE plus space for the expected data in size. n is the total number of bytes actually read so SETUP_PACKET_SIZE..(SETUP_PACKET_SIZE+n) would be correct.
I also don't love this setup and would almost rather model this as two separate buffers simply to make it clearer (I suspect the single buffer model is a hold over from ye olde C style things)
|
|
||
| pub(crate) max_packet_size: usize, | ||
|
|
||
| /// A queue of pending transfers, expected to complete in order |
There was a problem hiding this comment.
It's minor, but here is a doc comment in a private struct member, but not on the other members (or the struct). In particular max_packet_size is crate-pub, and should probably be documented.
There was a problem hiding this comment.
This matches what the other platforms have but I'll clean it up
| pub(crate) fn control_in( | ||
| self: Arc<Self>, | ||
| data: ControlIn, | ||
| _timeout: Duration, |
There was a problem hiding this comment.
My guess here would be that you open the device in non-blocking mode (or put it into that mode with fcntl) and then use something like poll or select on it.
| fds.insert( | ||
| ep.address, | ||
| IllumosUsbFds { | ||
| fd: Arc::new(fd), |
There was a problem hiding this comment.
There's a lot of Arcs inside of Arcs, it seems. I don't know that it makes a lot of sense to try and peel some of the layers of that onion off; it's just something I noticed.
5 participants
No description provided.