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

Update usb.c to address Bug 1886 ("sr: usb: Failed to get libusb file descriptors.") on Windows #235

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

COMBudda
Copy link

Add Windows handling to usb.c.

Per https://libusb.sourceforge.io/api-1.0/group__libusb__poll.html

libusb_get_pollfds()
As file descriptors are a Unix-specific concept, this function is not available on Windows and will always return NULL.

Add Windows handling to usb.c.

Per https://libusb.sourceforge.io/api-1.0/group__libusb__poll.html

libusb_get_pollfds()
As file descriptors are a Unix-specific concept, this function is not available on Windows and will always return NULL.
@ExplodingWaffle
Copy link

Thank you! This fix worked for me (with Kingst LA1010). I imagine this is causing issues with everything on Windows, so it would be nice to see merged, building from source is a lot of effort.

@toonvault
Copy link

Same here... neither the Saleae Clone nor the Kingst LA5016 did work with pulseview on windows, with the very same error
(usb: Failed to get libusb file descriptors.)

I am surprised that it was rather difficult to find more with this error, tried a few nightly builds I found on my drives from a few month ago.. same and on various workstations/laptops.

Building pulseview myself with this fix (MXE) solved the problem.

Thanks for this elegant fix!

Copy link

@lesshonor lesshonor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch solved my problem. Built with MXE using your average WSL2 Ubuntu 22.04 install.

If you intend to do the same, note that:

  • In addition to all the listed sigrok-util reqs and the MXE reqs, you will also need to install at least doxygen and asciidoctor.
  • mxe_fixes.patch and libusb1_upgrade.patch don't appear to be necessary anymore

@mahoekst
Copy link

Would be nice if this fix merged to main. I am currently stuck on my Windows machine to use the HiLetgo USB Logic Analyzer

@l33tm4st3r
Copy link

Would be nice if this fix merged to main. I am currently stuck on my Windows machine to use the HiLetgo USB Logic Analyzer

Me too...

@4rz0
Copy link

4rz0 commented Feb 15, 2024

Looking forward to have this merged into a release as well.

@CraigHutchinson
Copy link

Anybody got a windows binary? Maybe attach on a release in the fork etc?

@mahoekst
Copy link

Would love to find one too. Meanwhile I started using https://www.saleae.com/downloads/ which works with my USB logic analyzer as well.

@sricheson
Copy link

Would be nice to have this merged into main.

@multiplemonomials
Copy link

multiplemonomials commented Jun 16, 2024

Hmm, I ran into this issue as well in my work on mbed-ce/mbed-os, and I've done some investigation of the Sigrok source code. While I am admittedly a newcomer to glib, it looks like the code expects to work the following way:

  1. Sigrok calls libusb_get_pollfds() to get a list of USB-related file descriptors which, when they have events, mean stuff has happened with USB. It also sets up callbacks so that it can add and remove file descriptors from glib as they change inside libusb.
  2. Sigrok packs these file descriptors into a GSource implementation. This basically exposes them to glib so that it can call the source's callback whenever any of these file descriptors has an event. When glib calls the callback, sigrok forwards this to the hardware's callback, which in turn calls libusb's nonblocking update function.
  3. Sigrok runs the glib main loop. Each iteration, glib polls all of the file descriptors from libusb, and if any of them have events, it calls the source callback.

In current master branch, on windows, this breaks because libusb_get_pollfds() is not implemented and always fails. Until recently, sigrok was building against an ancient libusb fork ("event abstraction") which had special handling for this, but that fork was not maintained and does not seem to work reliably on recent versions of Windows (not surprising because it was last updated in 2015 IIRC). However, sigrok switched to build against the latest libusb, which fixes the compatibility issues, but does not actually work because of the missing polling support.

I tested the code in this PR, and as far as I can tell, it doesn't actually really fix the issue. When I compiled PulseView based on this Sigrok using MSYS2, I can only ever capture for a few tenths of a second at a time. After that, the hardware just stops reporting data. And that kinda makes sense, because all this PR does is remove the part that gets the pollable FDs from libusb. So, instead of passing FDs to monitor into glib, we don't pass any FDs and just the base timeout gets used. I would think this means that the USB stack gets polled very rarely; not often enough to keep an acquisition running correctly for very long.

In order to fix this issue, I think deeper changes to the logic with sigrok and libusb would be required.

One approach would be to replicate a "pollable fd" for glib using a thread. The thread would run in an infinite loop calling libusb_handle_events() (which blocks until at least one event happens, handles the event(s), and then returns), and then signaling a semaphore. This semaphore would work as a "pollable fd" and could be passed to glib. The remaining glib logic would be basically unchanged, as this semaphore would indicate when there are USB events available, just like the pollable FDs on Linux.

However, there might be an even simpler way to do this. If we look at the libusb source code, calling the nonblocking handle events function basically does already do a poll (here on posix, here on windows). Why are we going through so many extra steps so that glib can do this poll operation internally, when we could just be letting libusb take care of it?

What I'm thinking is, use a glib "idle source" to simply call into libusb every iteration of the main loop. That will do exactly the same thing we were doing before by polling the file descriptors/handles that libusb is is using internally and taking action based on that. We just won't have to mess around with providing the file descriptors to glib which cannot be done in a platform independent way. Plus, it will make the code way simpler, as we won't need a custom GSource and we won't need to care about reading file descriptors from libusb.

I have done testing using the above approach and I'm making progress: I am able to capture traces of arbitrary duration using my FX2LAFW logic analyzer! However, it breaks the ability to stop the logic analyzer, due to some sort of bad interaction with the other idle task in use. Still trying to figure out exactly why...

@multiplemonomials
Copy link

OK give this one a shot: #242

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.