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

[libftdi-compat] Add new port #6843

Merged
merged 8 commits into from
Jun 22, 2019
Merged

[libftdi-compat] Add new port #6843

merged 8 commits into from
Jun 22, 2019

Conversation

seanyen
Copy link
Contributor

@seanyen seanyen commented Jun 10, 2019

libFTDI is an open source library to talk to FTDI chips. This port is for v0.20.

@seanyen seanyen changed the title [libftdi-compat] v0.20 Windows port [libftdi-compat] Add new port Jun 10, 2019
@ras0219-msft
Copy link
Contributor

Hi @seanyen, thanks for making this PR!

I made quite a few changes:

  1. 0.20 was very outdated, so I used the latest (1.4).
  2. I renamed the port to just libftdi, since that's what everyone calls it
  3. It looks like since 0.20, some changes were made that broke Windows. I disabled the additional functionality that doesn't work and was able to build successfully.
  4. We discourage use of CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS since it causes a lot of problems. I replaced it with an explicit .def file.
  5. I patched the build to work for static or shared (not sure if that was an issue in the 0.20 version)
  6. I switched the dependency to just libusb, which is the library that 1.4 is looking for (instead of the libusb-win32 compat shim).

I'm not sure whether there were breaking changes in 1.4 that prevented you from using it, or if it was just because you had trouble getting it to build on windows. If you need 0.20 specifically and we can't make 1.4 work, let me know and we'll figure out what the right way forward is.

@seanyen
Copy link
Contributor Author

seanyen commented Jun 13, 2019

Thank @ras0219-msft for iterating my changes!

I specifically made the v0.20 port because the modules I am trying to integrate with is still consuming the old version (http://wiki.ros.org/kobuki_ftdi). And yes, like you guessed, there are breaking changes since v1 so I am not able to use the newer versions.

JFYI, here are some naming examples from other systems for libftdi packages:

Can we do the similar thing to keep two ports (0.20 and 1.4)?

@seanyen
Copy link
Contributor Author

seanyen commented Jun 17, 2019

@ras0219-msft just a friendly reminder. Any suggestions to keep both versions?

@ras0219-msft
Copy link
Contributor

ras0219-msft commented Jun 22, 2019

Thanks for the ping!

As noted in the commit message, I decided to use libftdi as 0.20 and libftdi1 as 1.4 -- this is based on the official source tarball names (libftdi-0.20.tar.gz vs libftdi1-1.4.tar.bz2) and also the naming scheme used by Debian derivatives.

One note is that these two DLLs have overlapping symbols, so they really shouldn't be installed side-by-side.

Edit: Another note -- I replaced your FindXYZ-style integration script with a config-style integration script. This solves some issues with debug mode that your script had, however it does mean you need to use modern cmake (target-based) and link against libftdi::ftdi.

@ras0219-msft ras0219-msft merged commit d1b4e88 into microsoft:master Jun 22, 2019
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.

3 participants