OpenLightingProject / ola

The Open Lighting Architecture - The Travel Adaptor for the Lighting Industry
https://www.openlighting.org/ola/
Other
647 stars 205 forks source link

libftdi API update #1012

Closed yoe closed 6 years ago

yoe commented 8 years ago

Hi,

The Debian maintainer of libftdi filed a bug against ola. I tried the simple fix ("s/libftdi-dev/libftdi1-dev/" over debian/control), but that results in no FTDI plugin being compiled.

Someone will need to look at the changes that were made in the new FTDI library and update ola accordingly. Mean time, I'll have to still compile against the old library.

peternewman commented 6 years ago

@Keeper-of-the-Keys mentioned this on IRC today. Trusty (Ubuntu 14.04) is in LTS until April 2019 and only has libfdti-dev: https://packages.ubuntu.com/search?keywords=libftdi&searchon=names&suite=trusty&section=all

So we should probably try and support both versions until then at least.

peternewman commented 6 years ago

I've made an initial start here: https://github.com/OpenLightingProject/ola/compare/0.10...peternewman:0.10-libftdi1

It doesn't compile yet: https://travis-ci.org/peternewman/ola/jobs/349129800

arielmol commented 6 years ago

I was building in yocto and got to the problem where I need to make it work so i found there is a libftdi-compat. So, I tried to install it but it still gets to the point it asks for libusb-config and it should use pkg-config. I think just updating to use pkg-config might fix it.

yoe commented 6 years ago

That won't solve the issue, it will only postpone it. The only way to really fix it is to update the use of the library to the new version.

Keeper-of-the-Keys commented 6 years ago

I hope to submit the pull request by the end of the month.

2018-04-25 9:52 GMT+03:00 Wouter Verhelst notifications@github.com:

That won't solve the issue, it will only postpone it. The only way to really fix it is to update the use of the library to the new version.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OpenLightingProject/ola/issues/1012#issuecomment-384179284, or mute the thread https://github.com/notifications/unsubscribe-auth/ABbkVv5QlE_Og_ZfPFNa2JkCMqgqvwjjks5tsB0sgaJpZM4HDV1w .

peternewman commented 6 years ago

@desert and @Keeper-of-the-Keys as mentioned above, I'd started hacking in some basic support, but it doesn't currently compile (so you may want to base it off that @Keeper-of-the-Keys ): https://github.com/OpenLightingProject/ola/compare/0.10...peternewman:0.10-libftdi1

@desert I'm confused by the libusb-config/pkg-config, given we already use pkg-config for the FTDI stuff.

arielmol commented 6 years ago

Seeing the error seems the API just changed. ../../plugins/ftdidmx/FtdiWidget.cpp:139:28: error: cannot initialize a variable of type 'struct usb_device *' with an lvalue of type 'struct libusb_device *' struct usb_device *dev = current_device->dev

peternewman commented 6 years ago

Yep, it just needs a pile of ifdefs, like started here: https://github.com/OpenLightingProject/ola/compare/0.10...peternewman:0.10-libftdi1#diff-de8c68bf126abbf38585da366b582faf

I did the initial stuff remotely just with Travis, but this seemed a point where stopping and doing it properly would make more sense and I've not had time yet.

Test content for GitHub below, please ignore. https://github.com/OpenLightingProject/ola/compare/0.10...peternewman:0.10-libftdi1#diff-de8c68bf126abbf38585da366b582faf 0.10...peternewman:0.10-libftdi1diff-de8c68bf126abbf38585da366b582faf https://github.com/OpenLightingProject/ola/compare/0.10...peternewman:0.10-libftdi1#foobar http://foo.com/bar#baz

peternewman commented 6 years ago

@Keeper-of-the-Keys said he'd fixed it on IRC.

Looking at your fork ( https://github.com/OpenLightingProject/ola/compare/master...Keeper-of-the-Keys:libfti1 ), the only real difference I see compared to mine is https://github.com/OpenLightingProject/ola/compare/master...Keeper-of-the-Keys:libfti1#diff-de8c68bf126abbf38585da366b582fafR139

+#if HAVE_LIBFTDI1
+   struct libusb_device *dev = current_device->dev;
+#else
         struct usb_device *dev = current_device->dev;
+#endif //HAVE_LIBFTDI1

Was this all you needed to change on mine? And the changes in olad/Makefile.mk to remove including the libs there?

Can you ideally just pull in my changes in git, rather than copy/pasting them, to keep the link of who did them etc. Although I'd also done it against 0.10 branch, which means we can get it out a bit sooner potentially.

Or do you want to open a Pull Request and we can progress from there?

arielmol commented 6 years ago

I have succesfully built ola with @Keeper-of-the-Keys libftdi1 branch. I used:

./configure --disable-all-plugins --disable-e133 --disable-python-libs --disable-examples --disable-http --enable-ftdidmx

I have not really tested any ftdi device. Will do that and inform.

Relevant run output:

olad/PluginManager.cpp:200: Started E1.31 (sACN)
olad/PluginManager.cpp:195: Trying to start USB
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 2:5 @0x55eac68731f0 [add]
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 2:4 @0x55eac686d070 [add]
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 2:3 @0x55eac686b620 [add]
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 2:2 @0x55eac6873150 [add]
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 2:1 @0x55eac6871cb0 [add]
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 6:1 @0x55eac6869110 [add]
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 5:11 @0x55eac6867510 [add]
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 5:10 @0x55eac686f890 [add]
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 5:9 @0x55eac6865960 [add]
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 5:2 @0x55eac6871ef0 [add]
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 5:1 @0x55eac686f9f0 [add]
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 1:2 @0x55eac686e760 [add]
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 1:1 @0x55eac686a7f0 [add]
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 4:1 @0x55eac686cee0 [add]
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 3:2 @0x55eac6867d50 [add]
libs/usb/HotplugAgent.cpp:166: USB hotplug event: 3:1 @0x55eac6869320 [add]
libs/usb/LibUsbThread.cpp:50: -- Starting libusb thread
common/thread/Thread.cpp:194: Thread , policy SCHED_OTHER, priority 0
libs/usb/LibUsbThread.cpp:35: ----libusb event thread is running
olad/PluginManager.cpp:200: Started USB
olad/PluginManager.cpp:195: Trying to start FTDI USB DMX
plugins/ftdidmx/FtdiWidget.cpp:124: Found 0 FTDI devices with PID: 24577.
plugins/ftdidmx/FtdiWidget.cpp:124: Found 0 FTDI devices with PID: 24593.
olad/PluginManager.cpp:200: Started FTDI USB DMX
Keeper-of-the-Keys commented 6 years ago

@peternewman - I originally did this against the 0.10.5 tag but then rebased to master so unless my understanding of git is wrong it should be possible to roll back to the commit before the rebase if that is better.

If you'd rather I'll see about pulling your branch first and then building with that.

@desert - I have also successfully built this on my Ubuntu 18.04 laptop, it properly detects my FTDI boards I didn't have DMX equipment to connect to it but hope to test with a few LEDs tonight. I don't have old Ubuntu systems anymore so can't build on them.

I only have 2 4 channel FTDI boards I would like to add a 2 channel and 1 channel so that I may test them together and separately.

peternewman commented 6 years ago

Thanks @Keeper-of-the-Keys .

I think I'd probably like the Travis testing stuff in the PR (ideally as well as credit for the bits I'd done, although it's fairly minor in this case). So yes, ideally if you can pull my branch and add the additional bits to finish the job, or I can pull the relevant changes you'd done into my branch and you/@desert could test against that? Whichever is easier.

Then we can try and get 0.10.7 out fairly soon with this fix in, as it seems fairly minor.

arielmol commented 6 years ago

Bring it on, I can test native x86 and yocto builds.

arielmol commented 6 years ago

I had some trouble setting up yocto cross compilations for foreign branches, a branch into the ola project would be easier.

Keeper-of-the-Keys commented 6 years ago

@peternewman I pushed a small change an hour ago that I hope will fix the travis issue, but travis isn't running as far as I can tell due to: "GitHub payload is missing a merge commit (mergeable_state: "unknown", merged: false) "

peternewman commented 6 years ago

@Keeper-of-the-Keys I suspect the Travis error was down to the fact the GitHub PR was in conflict. I used the web UI to resolve conflicts, but you can also do it on the command line. It's now attempting builds again.

peternewman commented 6 years ago

Closed by #1422 . See also for historic reference #394 .

@desert feel free to test please (the 0.10 branch)! @yoe this will be in 0.10.7 , probably releasing soon.

arielmol commented 6 years ago

Awesome, tomorrow will start testing.