More API functions #8
Conversation
|
|
||
| pub fn read_pins(&mut self) -> Result<u8> { | ||
| let mut pins : u8 = 0; | ||
| let pins_ptr = std::slice::from_mut(&mut pins).as_mut_ptr(); |
There was a problem hiding this comment.
Is the slice actually needed here? Why not just ffi::ftdi_read_pins(self.context, &mut pins)?
|
|
||
| pub fn set_baudrate(&mut self, baudrate : u32) -> Result<()> { | ||
| // TODO: libftdi1 will multiply baud rates in bitbang mode | ||
| // by 4. This leads to UB if baudrate >= INT_MAX/4. |
There was a problem hiding this comment.
I'd just add the corresponding assert here to avoid the possible UB.
| @@ -21,6 +21,24 @@ pub enum Interface { | |||
| Any, | |||
| } | |||
|
|
|||
| pub enum FlowControl { | |||
| Disable, | |||
There was a problem hiding this comment.
Naming: having this one as the only verb in this list looks weird. I'd prefer something like Disabled / Off
| impl Into<i32> for FlowControl { | ||
| fn into(self) -> i32 { | ||
| match self { | ||
| FlowControl::Disable => 0x0, |
There was a problem hiding this comment.
Yes, we should import these constants into libftdi1-sys.
| XonXoff | ||
| } | ||
|
|
||
| impl Into<i32> for FlowControl { |
There was a problem hiding this comment.
I'd prefer to have this as a private method for now. The specific codes used by FTDI for this should not be part of our API.
| @@ -119,6 +119,26 @@ impl Device { | |||
| } | |||
| } | |||
|
|
|||
| pub fn usb_close(mut self) -> Result<Builder> { | |||
There was a problem hiding this comment.
Do we have an actual use case for this, or do you do this just as it was in #2? I'd expect that allocating memory for the context is cheaper than actually opening the device, so the optimization is minimal. In other words, I'd probably prefer to drop this one.
|
|
||
| use super::ffi; | ||
|
|
||
| #[derive(Debug, Error)] | ||
| pub enum Error { | ||
| #[error("failed to initialize the ftdi context")] | ||
| InitFailed, |
There was a problem hiding this comment.
This isn't used anywhere, is it?
| } | ||
| } | ||
|
|
||
| pub fn set_baudrate(&mut self, baudrate : u32) -> Result<()> { |
There was a problem hiding this comment.
All our API needs docs, but this function probably the most for now. Does it include the "but the actual baud rate is 16 times higher" property in bitbang, assuming we implement it sooner or later (and do we want it to)? What accuracy does it guarantee? Can we somehow get back the actual baud rate configured? What happens to the baud rate what we switch modes once we support that?
This PR adds the following functions, along with remarks:
read_pins(fromsafe-ftdi): Nothing special here; this is the unbuffered version ofRead::read(&mut self, buf: &mut [u8])wherebufis a slice of length 1.set_baudrate(from Add set_baudrate, set_bitmode, and disable_bitbang #1 andsafe-ftdi): Nothing special here; the UB comment probably isn't a problem since in practice there's no such thing as a baud rate ofINT_MAX/4or above :).set_flow_control(from Add more ftdi wrappers #2): Perhaps we should consider importing the defines intolibftdi1-sys?usb_close(from Add more ftdi wrappers #2): This allows us to go back to aBuilderwithoutdropping theContext.The last two functions I intend to implement are:
ftdi_set_bitmodeanddisable_bitbang, which are duals of each other. This is worth a PR in and of itself, but once it's done, #1, #2 can be closed andsafe-ftdican be deprecated! :)