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

Add more ftdi wrappers #2

Closed
wants to merge 4 commits into from
Closed

Add more ftdi wrappers #2

wants to merge 4 commits into from

Conversation

geomatsi
Copy link
Contributor

This PR adds the following new Rust wrappers for libftdi functions:

  • usb_close
  • set_bitmode and BitMode enum
  • set_flow_control and FlowControl enum

Besides, minor cleanup patch is queued: coding style by rust fmt.

Regards,
Sergey

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
@tanriol
Copy link
Owner

tanriol commented Dec 27, 2018

Hello @geomatsi,

Thank you for contributing. Your wrappers look ok if we're wrapping the API at this level. However, I'd probably prefer to look into rewriting this API into a more Rust-like one, or at least checking whether the set_bitmode modes are actually safe to use with the provided functionality.

I'll try to refresh this area and review the API additions more thoroughly in a few days.

@geomatsi
Copy link
Contributor Author

Hi @tanriol ,
Ok, just ping me if I need to fix my patches. If you plan to rewrite the whole approach to using libftdi API in ftdi-rs, then let me know as well. I am interested in using it, so I will find time to help you with this update.

Regards,
Sergey

@geomatsi
Copy link
Contributor Author

geomatsi commented Mar 6, 2019

Hi @tanriol ,

Friendly ping. Any updates from your side ?

Regards,
Sergey

@cr1901 cr1901 mentioned this pull request Apr 25, 2020
@tanriol
Copy link
Owner

tanriol commented Apr 25, 2020

Do you have any specific reason to need usb_close? We're working on an API rework now and considering whether the new API (designed in terms of Builder/Device, not Context) needs a way to close the device and go from the Device back into the Builder stage, or whether just dropping the old device and creating a new one with a new context would be perfectly fine.

@geomatsi
Copy link
Contributor Author

geomatsi commented Apr 25, 2020

Well, I don't have any specific reason other than proper shutdown of USB connection after purging all the buffers. Consider the case when you run multiple tests, creating and destroying FTDI context. So I would like to clean up previous test state before running the next one. Technically I used usb_close in Drop implementation of ftdi-embedded-hal.

@tanriol
Copy link
Owner

tanriol commented Apr 25, 2020

I'd expect ftdi_free to take care of correctly closing everything... but that might be not the case, yes. Sounds like you'd be fine with ftdi_usb_close being called unconditionally from the Device's Drop implementation.

@geomatsi
Copy link
Contributor Author

Ok. I am going to close this PR since there is a larger rework is ongoing.

BTW, did you think about the following possibility: make one more release with older API and minor updates (v0.0.3) and bump release for the new API (e.g. v0.1.0) ?

@tanriol
Copy link
Owner

tanriol commented Apr 25, 2020

As far as I remember, for Cargo 0.0.2 and 0.0.3 are incompatible anyway :-(

@geomatsi geomatsi closed this Apr 25, 2020
@cr1901
Copy link
Contributor

cr1901 commented May 8, 2020

@geomatsi I wasn't aware this PR was closed.

My open PRs #7, #8, and #9 were meant to directly incorporate this PR, PR #1, and safe_ftdi into ftdi-rs before releasing a new incompatible version on crates.io. They are subject to change of course, but when they are committed, this PR will have been incorporated into those changes.

Do you have plans to try the new version when it's released?

@geomatsi
Copy link
Contributor Author

geomatsi commented May 8, 2020

Hi @cr1901

After brief discussion with @tanriol , I've got an impression that you are working on a larger rework: see our discussion above. So I closed my PR assuming it won't be needed. I plan to adapt my ftdi-embedded-hal to new ftdi-rs API later on.

So let me know if you still need this closed PR. I will re-open if needed.

Regards,
Sergey

@geomatsi geomatsi mentioned this pull request Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants