search

stephendpmurphy / shost

💻 shost - GUI, CLI and static library for connecting to FTDI MPSSE capable devices
MIT License
2 stars 1 forks source link

add I²C support #2

Closed NoeelMoeskops closed 2 years ago

NoeelMoeskops commented 2 years ago

Is it possible to add I²C support? Should be possible using the libmpsse library. I used it before to interface with I²C devices.

stephendpmurphy commented 2 years ago

@NoeelMoeskops absolutely! The lib I am including as a submodule (https://github.com/stephendpmurphy/libMPSSE) has support for SPI, I2C and GPIO. I just haven't gotten to it yet. I was actively working on a project that required a SPI host, so it was completed first.

Please feel free to start work on it. From the looks of the lib API, it's going to look very similar to SPI interface.

NoeelMoeskops commented 2 years ago

I made some progress, will open a pull request once I get a proof-of-concept working. Right now the code is not really DRY (since I'm copying a lot from spi.c) and I'm using the opensource libmpsse since I'm already familiar with that (can all be changed later once we get a working prototype).

Also, might I ask why you used the proprietary libraries from FTDI instead of the opensource versions? Not that there is anything wrong with that, just wondering :).

stephendpmurphy commented 2 years ago

Wasn't even aware they had an open source lib for the MPSSE interactions. Send me a link and I will gladly get an issue made to convert to the open-source version.

NoeelMoeskops commented 2 years ago

No problem! Its libmpsse and ftdi1. To make CMake happy you need to put some cmake find module things like I did here

stephendpmurphy commented 2 years ago

Awesome. I like the idea of the binary being as "all inclusive" as possible and requiring minimal number of libraries to be installed along side it. So anything we can statically compile into the CLI, the better. That's just my opinion, let me know if you see reasons not to.

I do also have hopes to make this work on Windows from the same code-base. I didn't look too closely, but I do know that the FTDI libs do support windows. Do you know if the open-source versions do as well?

I'll be on vacation for a couple days, so please feel free to que up PRs and issues as they arise. I'll tackle them all when I come back.

Thanks for contributing back to this project with me!

NoeelMoeskops commented 2 years ago

Awesome. I like the idea of the binary being as "all inclusive" as possible and requiring minimal number of libraries to be installed along side it. So anything we can statically compile into the CLI, the better. That's just my opinion, let me know if you see reasons not to.

ftdi1 is widely available on repositories, so we can just list it as a dependency. libmpsse is not however, so including libmpsse statically is I think a good option.

I do also have hopes to make this work on Windows from the same code-base. I didn't look too closely, but I do know that the FTDI libs do support windows. Do you know if the open-source versions do as well?

Hmm, not sure. Will have to figure that out. Is that important in order to adopt them?

I'll be on vacation for a couple days, so please feel free to que up PRs and issues as they arise. I'll tackle them all when I come back.

Thanks for contributing back to this project with me!

No problem, enjoy your holiday!

stephendpmurphy commented 2 years ago

I do also have hopes to make this work on Windows from the same code-base. I didn't look too closely, but I do know that the FTDI libs do support windows. Do you know if the open-source versions do as well?

Hmm, not sure. Will have to figure that out. Is that important in order to adopt them?

I personally think so. Referencing another point you made that it feels clunky to have two different types of libs that are accomplishing the same thing in the source. However, I suppose this is at least a difference between OS installs and not having to double up on libs for a single Windows or Linux install. I think you're suggestion of adopting C++ to help with abstraction would also help since we can then push the OS differences down as low as possible in a nice compartmentalized way.

I suppose at this point, lets move to using the open-source libs and we will tackle the issue with Windows when we get there.

stephendpmurphy commented 2 years ago

Just taking a look to refresh my memory as I get to work. In reference to WIndows support using the open source libs. I think we are good. The ability to get USB devices working with WSL in windows has recently been developed and will be our work around. i.e.. We focus on making the tool only work on Linux :+1:

NoeelMoeskops commented 2 years ago

Hi @stephendpmurphy, I took a quick look and could not find any information regarding libftdi1 and Windows. So it's great that WSL exists :). Also, I have been working on the c++ integration that we talked about earlier. It is almost finished, I just need to put the SPI code in a class and test it.

stephendpmurphy commented 2 years ago

Great. I've been taking a look into argp which appears to be the best way to do the CLI option and argument parsing.

stephendpmurphy commented 2 years ago

@NoeelMoeskops I apologize, but I think I'm going to hold off on the C++ switch over for the moment. I would still like to get the CLI parsing switched over, implement I2C and transition to the open-source MPSSE and libFTDI projects.

NoeelMoeskops commented 2 years ago

@stephendpmurphy I understand, However the switch to C++ is almost finished. I also refactored the CLI part quite a lot to make it more abstract and contained in a single file. I think it would be wise if we merge my i2c branch to upstream as fast as possible to avoid any double work :). Furthermore, I will commit and push the C++ changes properly tomorrow then you can take a look.

stephendpmurphy commented 2 years ago

I agree, let's get the I2C implementation into the develop branch so we can refactor and polish both SPI and I2C at the same time. Check out my current branch. I decided on implementing argp for the CLI option and argument parsing.

It does exactly what we I previously implemented by hand and has all the helpful bits of building out the --usage and --help options. Once we get I2C into develop, I'll replace the CLI parsing. I apologize if this renders any of the CLI rework you did useless.

https://github.com/stephendpmurphy/mpsse-cli/tree/useArgpForCLIparsing

NoeelMoeskops commented 2 years ago

The main thing I did regarding CLI is move all the relevant code from spi.c and mpsse-cli.c to cli.c to keep the code clean and added support for I²C. CLI handling in the i2c branch is only done in the main() by invoking int parsecli(int argc, char *argv[], arg_t* res) which will put everything in an arg_t object. But looking at your code I see that you did something similar :). Maybe we could combine some of the logic. But I leave that to you. The relevant commit: https://github.com/NoeelMoeskops/mpsse-cli/commit/56676931cc10966ab5a783dc4c12083a2930b2ff

stephendpmurphy commented 2 years ago

Awesome. I'm gonna try and get to it today! Thanks @NoeelMoeskops