Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More API functions #8

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,19 @@ use thiserror::Error;

use std::ffi::CStr;
use std::io;
use std::str;

use super::ffi;

#[derive(Debug, Error)]
pub enum Error {
#[error("failed to initialize the ftdi context")]
InitFailed,
Copy link
Owner

Choose a reason for hiding this comment

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

This isn't used anywhere, is it?

#[error("failed to enumerate devices to open the correct one")]
EnumerationFailed,
#[error("the specified device could not be found")]
DeviceNotFound,
#[error("failed to open the specified device")]
#[error("failed to open or close the specified device")]
AccessFailed,
#[error("the requested interface could not be claimed")]
ClaimFailed,
Expand All @@ -34,9 +37,14 @@ pub enum Error {

impl Error {
pub(crate) fn unknown(context: *mut ffi::ftdi_context) -> Self {
let message = unsafe { CStr::from_ptr(ffi::ftdi_get_error_string(context)) }
.to_str()
.expect("all error strings are expected to be ASCII");
let message = unsafe {
// Null pointer returns empty string. And we otherwise can't
// use a context without Builder::new() returning first.
let err_raw = ffi::ftdi_get_error_string(context);
// Manually checked- every error string in libftdi1 is ASCII.
str::from_utf8_unchecked(CStr::from_ptr(err_raw).to_bytes())
};

Error::Unknown {
source: LibFtdiError { message },
}
Expand All @@ -51,7 +59,13 @@ pub struct LibFtdiError {
message: &'static str,
}

#[derive(Debug, Error)]
#[error("libusb error code {code}")]
pub struct LibUsbError {
code: i32,
}

// Ideally this should be using libusb bindings, but we don't depend on any specific USB crate yet
pub(crate) fn libusb_to_io(code: i32) -> io::Error {
io::Error::new(io::ErrorKind::Other, format!("libusb error code {}", code))
io::Error::new(io::ErrorKind::Other, LibUsbError { code })
}
78 changes: 78 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use libftdi1_sys as ffi;

use std::convert::TryInto;
use std::io::{self, Read, Write};
use std::os::raw;

pub mod error;

Expand All @@ -20,6 +21,24 @@ pub enum Interface {
Any,
}

pub enum FlowControl {
Disable,
Copy link
Owner

Choose a reason for hiding this comment

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

Naming: having this one as the only verb in this list looks weird. I'd prefer something like Disabled / Off

RtsCts,
DtrDsr,
XonXoff
}

impl Into<i32> for FlowControl {
Copy link
Owner

Choose a reason for hiding this comment

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

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.

fn into(self) -> i32 {
match self {
FlowControl::Disable => 0x0,
Copy link
Owner

Choose a reason for hiding this comment

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

Yes, we should import these constants into libftdi1-sys.

FlowControl::RtsCts => (0x1 << 8),
FlowControl::DtrDsr => (0x2 << 8),
FlowControl::XonXoff => (0x4 << 8),
}
}
}

impl Into<ffi::ftdi_interface> for Interface {
fn into(self) -> ffi::ftdi_interface {
match self {
Expand Down Expand Up @@ -100,6 +119,26 @@ impl Device {
}
}

pub fn usb_close(mut self) -> Result<Builder> {
Copy link
Owner

@tanriol tanriol Apr 25, 2020

Choose a reason for hiding this comment

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

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.

let result = unsafe { ffi::ftdi_usb_close(self.context) };

match result {
0 => {
let context = std::mem::replace(&mut self.context, std::ptr::null_mut());
std::mem::forget(self);

let builder = Builder {
context
};

Ok(builder)
},
-1 => Err(Error::AccessFailed), // usb release failed
-3 => unreachable!("uninitialized context"),
_ => Err(Error::unknown(self.context))
}
}

pub fn usb_purge_buffers(&mut self) -> Result<()> {
let result = unsafe { ffi::ftdi_usb_purge_buffers(self.context) };
match result {
Expand Down Expand Up @@ -169,6 +208,45 @@ impl Device {
err => panic!("unknown get_write_chunksize retval {:?}", err),
}
}

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();
Copy link
Owner

Choose a reason for hiding this comment

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

Is the slice actually needed here? Why not just ffi::ftdi_read_pins(self.context, &mut pins)?


let result = unsafe { ffi::ftdi_read_pins(self.context, pins_ptr) };

match result {
0 => Ok(pins),
-1 => Err(Error::RequestFailed), // read pins failed
-2 => unreachable!("uninitialized context"),
_ => Err(Error::unknown(self.context)),
}
}

pub fn set_baudrate(&mut self, baudrate : u32) -> Result<()> {
Copy link
Owner

Choose a reason for hiding this comment

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

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?

// TODO: libftdi1 will multiply baud rates in bitbang mode
// by 4. This leads to UB if baudrate >= INT_MAX/4.
Copy link
Owner

Choose a reason for hiding this comment

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

I'd just add the corresponding assert here to avoid the possible UB.

let result = unsafe { ffi::ftdi_set_baudrate(self.context, baudrate as raw::c_int) };

match result {
0 => Ok(()),
-1 => Err(Error::InvalidInput("baud rate not supported")),
-2 => Err(Error::RequestFailed), // set baudrate failed
-3 => unreachable!("uninitialized context"),
_ => Err(Error::unknown(self.context))
}
}

pub fn set_flow_control(&mut self, flowctrl: FlowControl) -> Result<()> {
let result = unsafe { ffi::ftdi_setflowctrl(self.context, flowctrl.into()) };

match result {
0 => Ok(()),
-1 => Err(Error::RequestFailed), // set flow control failed
-2 => unreachable!("uninitialized context"),
_ => Err(Error::unknown(self.context))
}
}
}

impl Drop for Device {
Expand Down