Skip to content
New issue

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

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

Already on GitHub? # to your account

Read and Write Traits not implemented #60

Open
tolgadot opened this issue Sep 5, 2022 · 11 comments
Open

Read and Write Traits not implemented #60

tolgadot opened this issue Sep 5, 2022 · 11 comments
Labels
enhancement New feature or request

Comments

@tolgadot
Copy link
Contributor

tolgadot commented Sep 5, 2022

Implementing these traits would take no effort as far I can see (since there are already functions called read and write which is all that is needed) but allow the ftdi struct to be directly used in useful structures like BufReader or BufWriter etc.

Is there a specific reason why this is not implemented?

@newAM
Copy link
Member

newAM commented Sep 5, 2022

Is there a specific reason why this is not implemented?

It has been a while since I wrote this, I think the reason was that the libft2xx error type does not cleanly convert into std::io::Error.

Happy to accept a pull-request if you want to implement it!

@newAM
Copy link
Member

newAM commented Sep 5, 2022

I would still like to keep the existing methods though, the libft2xx errror codes are useful when asking for support from the vendor.

@newAM newAM added the enhancement New feature or request label Sep 5, 2022
@tolgadot
Copy link
Contributor Author

tolgadot commented Sep 6, 2022

I see, that makes sense. For now in my application I am working with

struct Wrapper {
    ftdi: Ftdi,
}

impl std::io::Read for Wrapper {
    fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
        self.ftdi
            .read(buf)
            .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))
    }
}

as a quick work-around.

What do you think about a solution where From<std::io::Error> is properly implemented for FtStatus where we try to match the error types as much as possible, with the un-matchable ones propagated like above? I am not very experienced with library error handling much either unfortunately.

@tolgadot
Copy link
Contributor Author

tolgadot commented Sep 6, 2022

Also turns out I was wrong, for Write one also needs to provide a flush function. Do you know how this is supposed to behave in this case?

@newAM
Copy link
Member

newAM commented Sep 6, 2022

Also turns out I was wrong, for Write one also needs to provide a flush function. Do you know how this is supposed to behave in this case?

The libftd2xx documentation does not mention how write behaves internally, if I recall correctly.
If write flushes itself then std::io::Write::flush can be empty, if write does not flush itself then it probably cannot be adapted to std::io::Write.

@newAM
Copy link
Member

newAM commented Sep 6, 2022

Just to make sure you have seen it, there is another crate that uses the open source libftdi driver which has std::io traits out of the box: https://crates.io/crates/ftdi
I would only recommend libftd2xx instead of libftdi if you need to target Windows platforms.

@tolgadot
Copy link
Contributor Author

tolgadot commented Sep 15, 2022

Yes I need to support Windows as well as Linux unfortunately :)

(by the way @newAM do you know if it is possible to statically compile this library from Linux for a Windows target? cargo build --target x86_64-pc-windows-msvc --features static fails because could not find native static library ftd2xx)

@tolgadot
Copy link
Contributor Author

In my wrapper type I have indeed left the flush empty:

    fn flush(&mut self) -> std::io::Result<()> {
        Ok(())
    }

This has been working without an issue so far.

@newAM
Copy link
Member

newAM commented Sep 17, 2022

do you know if it is possible to statically compile this library from Linux for a Windows target?

I think this is a bug, I took a look and cross-compiling for Windows from Linux uses \ path separators when it should probably use /.

Working on it here: ftdi-rs/libftd2xx-ffi#57

@newAM
Copy link
Member

newAM commented Sep 17, 2022

Yup, that was the problem (for that specific error), good find! Will get a release of libftd2xx-ffi out shortly.

@newAM
Copy link
Member

newAM commented Sep 17, 2022

For linking I found this guide from bevy: https://bevy-cheatbook.github.io/setup/cross/linux-windows.html

Worked for me after that.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants