Unify `ftdi-rs` and `safe-ftdi`

[cr1901]

Opening this Issue so we can hash out the details... I documented my findings in a Markdown file, reproduced here for convenience :)

Differences between ftdi-rs and safe-ftdi

Short version: We both seem to have the same ideas, but slightly different ways of going about implementing a safe wrapper.

Context struct

ftdi-rs keeps a copy of the ftdi_context; safe-ftdi stores a pointer.

ftdi-rs:

pub struct Context {
    native: ffi::ftdi_context,
}

safe-ftdi:

pub struct Context {
    context : *mut ftdic::ftdi_context,
}

Similarly, for Drop:

ftdi-rs:

impl Drop for Context {
    fn drop(&mut self) {
        unsafe { ffi::ftdi_deinit(&mut self.native) }
    }
}

safe-ftdi:

impl Drop for Context {
    fn drop(&mut self) {
        unsafe { ftdic::ftdi_free(self.context) }
    }
}

I'm not sure which one is better...

Error Handling

ftdi-rs maps everything back to an io::Result type, and each function has function-specific matching code to map back to an io::Error in the Err variant of io::Result.

safe-ftdi has a single private check_ftdi_error that's at the end of each function that maps back to its own safe_ftdi::Error in the Err variant of safe-ftdi::Result. With some error, we could probably implement From<T> for safe_ftdi::Error for various errors to use ? and avoid typing out check_ftdi_error in each function.

Present in ftdi-rs but not safe-ftdi

• Read and Write implementations! This is good, and something I should've implemented, but didn't :D!
• You follow C-GETTER in the API guidelines. I didn't get this far, but I wouldn't have before learning about this tonight.
• Better bounds-checking.

Present in safe-ftdi but not ftdi-rs

• A crate-specific error type. I think this is preferable to reusing io::Result<>, and it seems to be standard for crates to define their own error type. A friend has been helping me out with redefining what such an error type looks like.
• mercpcl re-implementation :).

Other Questions I Have

• Can ftdi_rs::Interface be a C-style enum, removing the Into conversion?

Function name mapping

libftd1-sys	ftdi-rs	safe-ftdi	Comment
ftdi_init	new
ftdi_new		new
ftdi_set_interface	set_interface
ftdi_usb_reset	usb_reset
ftdi_usb_purge_buffers	usb_purge_buffers
ftdi_set_latency_timer	set_latency_timer
ftdi_get_latency_timer	latency_timer		C-GETTER
ftdi_write_data_set_chunksize	set_write_chunksize
ftdi_write_data_get_chunksize	write_chunksize		C-GETTER
ftdi_read_data_set_chunksize	set_read_chunksize
ftdi_read_data_get_chunksize	read_chunksize		C-GETTER
ftdi_usb_open	usb_open	open
ftdi_set_baudrate		set_baudrate
ftdi_set_bitmode		set_bitmode
ftdi_read_pins		read_pins
ftdi_read_data	io::Read::read	read_data	Should be exposed along w/ trait?
ftdi_write_data	io::Write::write	write_data	Should be exposed along w/ trait?

[tanriol]

Nice to see such a thorough comparison :-)

Let's actually start with a slightly different question. For ftdi-rs this will definitely be an incompatible version bump (it's at 0.0.2, there are no versions compatible with this). Are we trying to make a good enough for now incremental improvement, or something that could in the future become the good, complete and stable API?

I'm asking this because there are some points that I don't have a plan for yet and that I'd like to be more or less correct in the "complete and stable" version. For example:

If an FTDI chip has multiple interfaces, what are the limitations on their concurrent usage? Can they be used in parallel from a single application? Multiple applications?

In particular, I'm not sure whether the library should be tracking BITMODE_SYNCFF usage - IIRC, this one can only be configured on interface A and interface B cannot be used in parallel with it at all.

Next, I'm actually unsure about Read and Write. Specifically, how should they interact with different modes? Should they be allowed in MPSSE mode, for example? Should the API ignore these questions, or track them with asserts during runtime, or have different types for different modes the chip can be in?

[cr1901]

Are we trying to make a good enough for now incremental improvement, or something that could in the future become the good, complete and stable API?

I was trying for a "good, complete and stable API", but I can certainly live with "good enough for now incremental improvement". That is why I opened this issue so we could do back and forth :P.

I think what I would like at the end of this round of changes is most (or even all) of the public libftdi1 functions exposed in this crate, using safe abstractions to call into libftdi1-sys.

If an FTDI chip has multiple interfaces, what are the limitations on their concurrent usage? Can they be used in parallel from a single application? Multiple applications?

I do not honestly remember, because I don't think I've ever tried using them concurrently LOL. My understanding is that each FTDI interface is loaded as a separate USB device on FTDI devices.¹. I've not used libusb directly before, but I'm guessing that it's not possible to share USB interfaces between applications, and using two interfaces in the same application would require using the same context struct. Which leads to the next point:

In particular, I'm not sure whether the library should be tracking BITMODE_SYNCFF usage - IIRC, this one can only be configured on interface A and interface B cannot be used in parallel with it at all.

Re: your above two comments, I have some ideas on how to handle the same application requirement safely... Context could be a singleton, in which you move out interface tokens to be moved to other threads. That way each thread using an interface can't trample on each other.

Disadvantages:

• Not sure offhand how to make the API functions generic without manually passing a token around however :(. Maybe create an Interface{A,B,C,D} type impl a FtdiContext trait, where a composite type that also implements FtdiContext is the function returned by new?

• Need some way to lock. I don't even know if libftdi1 is thread-safe.

Next, I'm actually unsure about Read and Write. Specifically, how should they interact with different modes? Should they be allowed in MPSSE mode, for example? Should the API ignore these questions, or track them with asserts during runtime, or have different types for different modes the chip can be in?

I would prefer the "different types for different modes", but I admit this might be a long term goal. I think my opinion right now is to get more safe wrappers around libftdi1-sys implemented, deploy in applications, and see if any nice (usage, API implementation) patterns appear before implementing Read and Write. Which means, yes, remove them for now in this new version.

Footnotes

1. A USB device has one or more configurations. Each configuration has one or more interfaces. USB devices can describe themselves at the device level or interface level; the latter is called a composite device.

[awygle]

Hi, safe-ftdi user here, asked by @cr1901 to comment on my use cases.

All I'd want from a version 0.1 (or even 1.0) release would be serial support. Serial mode is by far the most common use case for FTDI devices, IME. Second place would be MPSSE for JTAG, which would be nice to have but not critical. My preference would be to make a usable, safe wrapper for a subset of functionality first, before focusing on supporting every feature.

[tanriol]

@cr1901 Sorry, got dragged away by real-life. Could you please cross-check my current understanding that all modes except MPSSE can support Read/Write with the straightforward meaning of "get data received from the other side" and "send this data to the other side" respectively? Have I missed anything?

[tanriol]

@awygle, thank for your feedback :-) will try :-)

[tanriol]

I've looked through some documentation, and their mode combinations - including EEPROM-only modes and which settings affect which mode - are a bit overwhelming.

[cr1901]

My preference would be to make a usable, safe wrapper for a subset of functionality first, before focusing on supporting every feature.

I am unifying safe-ftdi and ftdi-rs tonight. I think this may be the most practical way forward then. I will at least merge the existing functions between then so ftdi-rs can completely supplant safe-ftdi.

To that end, before we make release 0.2.0, we should document the functions we have so it's easy to get started, and make sure we are following the API guidelines.

It won't be as convenient as "banging out the entire API at once", releasing 1.0.0, and passively maintaining the bindings. But at least we'll have a base to work off of.

[tanriol]

Ok, then I'm not messing with it any more than already pushed for now.

[cr1901]

@tanriol Still working on refactoring tonight. My energy level has been bad the past week or so, so I apologize for taking so long.