Skip to content

Run USB as an idle task instead of a custom FD-based GSource #242

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Run USB as an idle task instead of a custom FD-based GSource #242

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
939b7cf
Use idle for USB
multiplemonomials Jun 15, 2024
34ad068
Fix missing destroy callback causing device to get "stuck on", switch…
multiplemonomials Jun 16, 2024
98e899a
A few cleanups
multiplemonomials Jul 7, 2024
f4f0b32
Add note about callback
multiplemonomials Jul 7, 2024
4d91bce
Restore removal behavior
multiplemonomials Jul 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/backend.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,17 @@ SR_API GSList *sr_buildinfo_libs_get(void)
glib_binary_age, glib_interface_age));
l = g_slist_append(l, m);

#ifdef CONF_ZLIB_VERSION
Copy link
Author

@multiplemonomials multiplemonomials Jul 7, 2024

Choose a reason for hiding this comment

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

These missing ifdefs caused me some issues when building the code, they really should be here

m = g_slist_append(NULL, g_strdup("zlib"));
m = g_slist_append(m, g_strdup_printf("%s", CONF_ZLIB_VERSION));
l = g_slist_append(l, m);
#endif
Copy link
Contributor

@biot biot Nov 24, 2025

Choose a reason for hiding this comment

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

If this solved a problem for you that's good, but I would recommend sticking this in a separate patch, not conflated with some serious core USB code.


#ifdef CONF_LIBZIP_VERSION
m = g_slist_append(NULL, g_strdup("libzip"));
m = g_slist_append(m, g_strdup_printf("%s", CONF_LIBZIP_VERSION));
l = g_slist_append(l, m);
#endif
Copy link
Contributor

@biot biot Nov 24, 2025

Choose a reason for hiding this comment

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

In fact libzip is a required dependency, libsigrok will not build without it. So there is no need for this #ifdef.

Copy link
Author

@multiplemonomials multiplemonomials Nov 27, 2025

Choose a reason for hiding this comment

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

OK will do. Actually funny story, I am so dependent on CLion for my dev work that, in order to develop this patch, I reimplemented about half the libsigrok build system in CMake so that I could open the project in CLion. So that's how I found this, but I am happy to revert it.

btw, if you guys are interested in a CMake build system option, I could see about polishing up that code some more and contributing it (though I am not 100% sure I still have it anymore)

Copy link

@Krakonos Krakonos Nov 28, 2025

Choose a reason for hiding this comment

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

@multiplemonomials I'm speaking for myself here, but I'd appreciate the move to cmake. IDE support being one of the reasons, although the autotools setup currently seems to work fine. I suggest you hit the mailing list and we can have a discussion there?


m = g_slist_append(NULL, g_strdup("minilzo"));
m = g_slist_append(m, g_strdup_printf("%s", lzo_version_string()));
Expand Down
20 changes: 1 addition & 19 deletions src/hardware/dreamsourcelab-dslogic/protocol.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -874,22 +874,6 @@ static void LIBUSB_CALL receive_transfer(struct libusb_transfer *transfer)
resubmit_transfer(transfer);
}

static int receive_data(int fd, int revents, void *cb_data)
Copy link
Author

@multiplemonomials multiplemonomials Jul 7, 2024

Choose a reason for hiding this comment

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

With this change, drivers no longer need to implement callbacks if all they are doing is calling libusb_handle_events_timeout(). I tried to update a few of them, but frankly IDK if I have the time & energy to update all of them.

Copy link
Contributor

@biot biot Nov 24, 2025

Choose a reason for hiding this comment

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

Even if your USB source changes get merged, you should probably update the drivers in a separate patch. This just makes the patchset larger and harder to review.

{
struct timeval tv;
struct drv_context *drvc;

(void)fd;
(void)revents;

drvc = (struct drv_context *)cb_data;

tv.tv_sec = tv.tv_usec = 0;
libusb_handle_events_timeout(drvc->sr_ctx->libusb_ctx, &tv);

return TRUE;
}

static size_t to_bytes_per_ms(const struct sr_dev_inst *sdi)
{
const struct dev_context *const devc = sdi->priv;
Expand Down Expand Up @@ -1028,8 +1012,6 @@ static void LIBUSB_CALL trigger_receive(struct libusb_transfer *transfer)

SR_PRIV int dslogic_acquisition_start(const struct sr_dev_inst *sdi)
{
const unsigned int timeout = get_timeout(sdi);

struct sr_dev_driver *di;
struct drv_context *drvc;
struct dev_context *devc;
Expand All @@ -1048,7 +1030,7 @@ SR_PRIV int dslogic_acquisition_start(const struct sr_dev_inst *sdi)
devc->empty_transfer_count = 0;
devc->acq_aborted = FALSE;

usb_source_add(sdi->session, devc->ctx, timeout, receive_data, drvc);
usb_source_add(sdi->session, devc->ctx, 0, NULL, NULL);

if ((ret = command_stop_acquisition(sdi)) != SR_OK)
return ret;
Expand Down
21 changes: 2 additions & 19 deletions src/hardware/fx2lafw/protocol.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,22 +612,6 @@ static unsigned int get_timeout(struct dev_context *devc)
return timeout + timeout / 4; /* Leave a headroom of 25% percent. */
}

static int receive_data(int fd, int revents, void *cb_data)
{
struct timeval tv;
struct drv_context *drvc;

(void)fd;
(void)revents;

drvc = (struct drv_context *)cb_data;

tv.tv_sec = tv.tv_usec = 0;
libusb_handle_events_timeout(drvc->sr_ctx->libusb_ctx, &tv);

return TRUE;
}

static int start_transfers(const struct sr_dev_inst *sdi)
{
struct dev_context *devc;
Expand Down Expand Up @@ -714,7 +698,7 @@ SR_PRIV int fx2lafw_start_acquisition(const struct sr_dev_inst *sdi)
struct sr_dev_driver *di;
struct drv_context *drvc;
struct dev_context *devc;
int timeout, ret;
int ret;
size_t size;

di = sdi->driver;
Expand All @@ -732,8 +716,7 @@ SR_PRIV int fx2lafw_start_acquisition(const struct sr_dev_inst *sdi)
return SR_ERR;
}

timeout = get_timeout(devc);
usb_source_add(sdi->session, devc->ctx, timeout, receive_data, drvc);
usb_source_add(sdi->session, devc->ctx, 0, NULL, NULL);

size = get_buffer_size(devc);
/* Prepare for analog sampling. */
Expand Down
3 changes: 1 addition & 2 deletions src/hardware/hantek-4032l/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,8 +591,7 @@ static int dev_acquisition_start(const struct sr_dev_inst *sdi)
cmd_pkt->trigger[0].data_range_max = range_value;
}

usb_source_add(sdi->session, drvc->sr_ctx, 1000,
h4032l_receive_data, sdi->driver->context);
usb_source_add(sdi->session, drvc->sr_ctx, 0, NULL, NULL);

/* Start capturing. */
return h4032l_start(sdi);
Expand Down
16 changes: 0 additions & 16 deletions src/hardware/hantek-4032l/protocol.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,22 +139,6 @@ static void send_data(struct sr_dev_inst *sdi,
devc->sent_samples += sample_count;
}

SR_PRIV int h4032l_receive_data(int fd, int revents, void *cb_data)
{
struct timeval tv;
struct drv_context *drvc;

(void)fd;
(void)revents;

drvc = (struct drv_context *)cb_data;

tv.tv_sec = tv.tv_usec = 0;
libusb_handle_events_timeout(drvc->sr_ctx->libusb_ctx, &tv);

return TRUE;
}

void LIBUSB_CALL h4032l_data_transfer_callback(struct libusb_transfer *transfer)
{
struct sr_dev_inst *const sdi = transfer->user_data;
Expand Down
5 changes: 0 additions & 5 deletions src/hardware/hantek-dso/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,6 @@ static void LIBUSB_CALL receive_transfer(struct libusb_transfer *transfer)
static int handle_event(int fd, int revents, void *cb_data)
{
const struct sr_dev_inst *sdi;
struct timeval tv;
struct sr_dev_driver *di;
struct dev_context *devc;
struct drv_context *drvc;
Expand Down Expand Up @@ -829,10 +828,6 @@ static int handle_event(int fd, int revents, void *cb_data)
return TRUE;
}

/* Always handle pending libusb events. */
tv.tv_sec = tv.tv_usec = 0;
libusb_handle_events_timeout(drvc->sr_ctx->libusb_ctx, &tv);

/* TODO: ugh */
if (devc->dev_state == NEW_CAPTURE) {
if (dso_capture_start(sdi) != SR_OK)
Expand Down
Loading