From 9e1cb5c0032120ca2fcbf5452afa86a27a7b1805 Mon Sep 17 00:00:00 2001 From: Martin Ling Date: Fri, 18 Mar 2022 00:43:24 +0000 Subject: [PATCH 01/10] Don't exit transfer thread if an error occurs. In the case of a libusb error, we still need the transfer thread running, in order to handle outstanding cancellations and to signal the condition variable when that is done. --- host/libhackrf/src/hackrf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/host/libhackrf/src/hackrf.c b/host/libhackrf/src/hackrf.c index c0062f4bb..6671bc372 100644 --- a/host/libhackrf/src/hackrf.c +++ b/host/libhackrf/src/hackrf.c @@ -1766,7 +1766,7 @@ static void LIBUSB_CALL hackrf_libusb_transfer_callback(struct libusb_transfer* LIBUSB_TRANSFER_ERROR, LIBUSB_TRANSFER_TIMED_OUT LIBUSB_TRANSFER_STALL, LIBUSB_TRANSFER_OVERFLOW .... */ - request_exit(device); /* Fatal error stop transfer */ + transfer_finished(device, usb_transfer); device->streaming = false; } } From 6720e56fc028a84d2d3ae1a9a88057f12d016c18 Mon Sep 17 00:00:00 2001 From: Martin Ling Date: Fri, 18 Mar 2022 01:02:33 +0000 Subject: [PATCH 02/10] Clear streaming flag if we didn't resubmit a transfer. If result < 0 here, libusb_submit_transfer returned an error, so we need to shut down. If !resubmit, then cancel_transfers() was already called by one of the stop or close functions, so streaming is already false. --- host/libhackrf/src/hackrf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/host/libhackrf/src/hackrf.c b/host/libhackrf/src/hackrf.c index 6671bc372..528b278e2 100644 --- a/host/libhackrf/src/hackrf.c +++ b/host/libhackrf/src/hackrf.c @@ -1754,6 +1754,7 @@ static void LIBUSB_CALL hackrf_libusb_transfer_callback(struct libusb_transfer* if (!resubmit || result < 0) { transfer_finished(device, usb_transfer); + device->streaming = false; } } else { transfer_finished(device, usb_transfer); From 125bf9f7bb14c1f944d60cb6205095ac56e6939e Mon Sep 17 00:00:00 2001 From: Martin Ling Date: Fri, 18 Mar 2022 00:56:25 +0000 Subject: [PATCH 03/10] Don't call callback or submit new transfers once streaming stops. This stops the RX callback from being called again with further data once it has returned nonzero, or after a transfer had an error status. --- host/libhackrf/src/hackrf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/host/libhackrf/src/hackrf.c b/host/libhackrf/src/hackrf.c index 528b278e2..23d21e88c 100644 --- a/host/libhackrf/src/hackrf.c +++ b/host/libhackrf/src/hackrf.c @@ -1737,7 +1737,7 @@ static void LIBUSB_CALL hackrf_libusb_transfer_callback(struct libusb_transfer* .tx_ctx = device->tx_ctx }; - if( device->callback(&transfer) == 0 ) + if (device->streaming && device->callback(&transfer) == 0) { // Take lock to make sure that we don't restart a // transfer whilst cancel_transfers() is in the middle From 6bd9cb05538304f3c43d1349befa6a47bfd657e9 Mon Sep 17 00:00:00 2001 From: Martin Ling Date: Fri, 18 Mar 2022 01:09:06 +0000 Subject: [PATCH 04/10] Clear streaming flag if a transfer was cancelled. If a transfer was cancelled, we are on our way to shutdown. If hackrf_stop_tx() or hackrf_stop_rx() were called, they will already have cleared this flag, but it is not cleared in hackrf_close(), and for consistency with other paths it makes sense to clear it here. --- host/libhackrf/src/hackrf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/host/libhackrf/src/hackrf.c b/host/libhackrf/src/hackrf.c index 23d21e88c..8430b31da 100644 --- a/host/libhackrf/src/hackrf.c +++ b/host/libhackrf/src/hackrf.c @@ -1762,6 +1762,7 @@ static void LIBUSB_CALL hackrf_libusb_transfer_callback(struct libusb_transfer* } } else if(usb_transfer->status == LIBUSB_TRANSFER_CANCELLED) { transfer_finished(device, usb_transfer); + device->streaming = false; } else { /* Other cases LIBUSB_TRANSFER_NO_DEVICE LIBUSB_TRANSFER_ERROR, LIBUSB_TRANSFER_TIMED_OUT From 54e00de16752ab41a60c4f739e4720056e09f162 Mon Sep 17 00:00:00 2001 From: Martin Ling Date: Fri, 18 Mar 2022 01:12:29 +0000 Subject: [PATCH 05/10] Clear streaming flag in transfer_finished(). Since we always do these together, move it into the function. --- host/libhackrf/src/hackrf.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/host/libhackrf/src/hackrf.c b/host/libhackrf/src/hackrf.c index 8430b31da..8a5337dd0 100644 --- a/host/libhackrf/src/hackrf.c +++ b/host/libhackrf/src/hackrf.c @@ -1704,6 +1704,9 @@ static void transfer_finished(struct hackrf_device* device, struct libusb_transf int i; bool all_finished = true; + // If a transfer finished for any reason, we're shutting down. + device->streaming = false; + for (i = 0; i < TRANSFER_COUNT; i++) { if (device->transfers[i] == finished_transfer) { device->transfer_finished[i] = true; @@ -1754,22 +1757,18 @@ static void LIBUSB_CALL hackrf_libusb_transfer_callback(struct libusb_transfer* if (!resubmit || result < 0) { transfer_finished(device, usb_transfer); - device->streaming = false; } } else { transfer_finished(device, usb_transfer); - device->streaming = false; } } else if(usb_transfer->status == LIBUSB_TRANSFER_CANCELLED) { transfer_finished(device, usb_transfer); - device->streaming = false; } else { /* Other cases LIBUSB_TRANSFER_NO_DEVICE LIBUSB_TRANSFER_ERROR, LIBUSB_TRANSFER_TIMED_OUT LIBUSB_TRANSFER_STALL, LIBUSB_TRANSFER_OVERFLOW .... */ transfer_finished(device, usb_transfer); - device->streaming = false; } } From c74c7423915d6c9a9858af39de6ce6e0768c2106 Mon Sep 17 00:00:00 2001 From: Martin Ling Date: Fri, 18 Mar 2022 01:15:54 +0000 Subject: [PATCH 06/10] Simplify hackrf_libusb_transfer_callback. There are now only two possible outcomes to this function: either we successfully resubmitted a transfer, or the transfer is finished and we end up calling transfer_finished(). So we can go ahead and simplify it accordingly. --- host/libhackrf/src/hackrf.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/host/libhackrf/src/hackrf.c b/host/libhackrf/src/hackrf.c index 8a5337dd0..e028285a7 100644 --- a/host/libhackrf/src/hackrf.c +++ b/host/libhackrf/src/hackrf.c @@ -1755,21 +1755,14 @@ static void LIBUSB_CALL hackrf_libusb_transfer_callback(struct libusb_transfer* // cancelled or restarted, not both. pthread_mutex_unlock(&device->transfer_lock); - if (!resubmit || result < 0) { - transfer_finished(device, usb_transfer); - } - } else { - transfer_finished(device, usb_transfer); + if (resubmit && result == LIBUSB_SUCCESS) + return; } - } else if(usb_transfer->status == LIBUSB_TRANSFER_CANCELLED) { - transfer_finished(device, usb_transfer); - } else { - /* Other cases LIBUSB_TRANSFER_NO_DEVICE - LIBUSB_TRANSFER_ERROR, LIBUSB_TRANSFER_TIMED_OUT - LIBUSB_TRANSFER_STALL, LIBUSB_TRANSFER_OVERFLOW .... - */ - transfer_finished(device, usb_transfer); } + + // Unless we resubmitted this transfer and returned above, + // it's now finished. + transfer_finished(device, usb_transfer); } static int kill_transfer_thread(hackrf_device* device) From 960d8015a4ecd9e113d25e3d5b030701a74933db Mon Sep 17 00:00:00 2001 From: Martin Ling Date: Fri, 18 Mar 2022 01:28:26 +0000 Subject: [PATCH 07/10] Clear streaming flag in cancel_transfers(). Moving this into cancel_transfers() avoids duplicating it in the two stop functions. --- host/libhackrf/src/hackrf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/host/libhackrf/src/hackrf.c b/host/libhackrf/src/hackrf.c index e028285a7..555e74a22 100644 --- a/host/libhackrf/src/hackrf.c +++ b/host/libhackrf/src/hackrf.c @@ -218,6 +218,9 @@ static int cancel_transfers(hackrf_device* device) uint32_t transfer_index; int i; + // If we're cancelling transfers for any reason, we're shutting down. + device->streaming = false; + if(transfers_check_setup(device) == true) { // Take lock while cancelling transfers. This blocks the @@ -1922,7 +1925,6 @@ int ADDCALL hackrf_stop_rx(hackrf_device* device) { int result; - device->streaming = false; result = cancel_transfers(device); if (result != HACKRF_SUCCESS) { @@ -1971,7 +1973,6 @@ static int hackrf_stop_tx_cmd(hackrf_device* device) int ADDCALL hackrf_stop_tx(hackrf_device* device) { int result; - device->streaming = false; result = cancel_transfers(device); if (result != HACKRF_SUCCESS) { From 5afd31e21c47c346664e12e811314cca5d63a8c4 Mon Sep 17 00:00:00 2001 From: Martin Ling Date: Fri, 18 Mar 2022 01:40:38 +0000 Subject: [PATCH 08/10] Set streaming flag in prepare_setup_transfers(). Avoids conditionally duplicating this across three other places. --- host/libhackrf/src/hackrf.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/host/libhackrf/src/hackrf.c b/host/libhackrf/src/hackrf.c index 555e74a22..89f86a9f0 100644 --- a/host/libhackrf/src/hackrf.c +++ b/host/libhackrf/src/hackrf.c @@ -1831,6 +1831,8 @@ static int prepare_setup_transfers(hackrf_device* device, return result; } + device->streaming = true; + return HACKRF_SUCCESS; } @@ -1894,9 +1896,6 @@ int ADDCALL hackrf_start_rx(hackrf_device* device, hackrf_sample_block_cb_fn cal { result = prepare_setup_transfers(device, endpoint_address, callback); } - if (result == HACKRF_SUCCESS) { - device->streaming = true; - } return result; } @@ -1944,9 +1943,6 @@ int ADDCALL hackrf_start_tx(hackrf_device* device, hackrf_sample_block_cb_fn cal device->tx_ctx = tx_ctx; result = prepare_setup_transfers(device, endpoint_address, callback); } - if (result == HACKRF_SUCCESS) { - device->streaming = true; - } return result; } @@ -2678,9 +2674,6 @@ int ADDCALL hackrf_start_rx_sweep(hackrf_device* device, hackrf_sample_block_cb_ device->rx_ctx = rx_ctx; result = prepare_setup_transfers(device, endpoint_address, callback); } - if (result == HACKRF_SUCCESS) { - device->streaming = true; - } return result; } From c4789df44cf143304fa00bcef8ad000cc2ff13f3 Mon Sep 17 00:00:00 2001 From: Martin Ling Date: Fri, 18 Mar 2022 01:46:17 +0000 Subject: [PATCH 09/10] Set streaming flag in prepare_transfers(). This simplifies prepare_setup_transfers(), which was just setting the flag if prepare_transfers() returned success, and passing on its return value. --- host/libhackrf/src/hackrf.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/host/libhackrf/src/hackrf.c b/host/libhackrf/src/hackrf.c index 89f86a9f0..0868c22b0 100644 --- a/host/libhackrf/src/hackrf.c +++ b/host/libhackrf/src/hackrf.c @@ -351,6 +351,7 @@ static int prepare_transfers( } } device->transfers_setup = true; + device->streaming = true; return HACKRF_SUCCESS; } else { // This shouldn't happen. @@ -1813,27 +1814,16 @@ static int prepare_setup_transfers(hackrf_device* device, const uint8_t endpoint_address, hackrf_sample_block_cb_fn callback) { - int result; - if( device->transfers_setup == true ) { return HACKRF_ERROR_BUSY; } device->callback = callback; - result = prepare_transfers( + return prepare_transfers( device, endpoint_address, hackrf_libusb_transfer_callback ); - - if( result != HACKRF_SUCCESS ) - { - return result; - } - - device->streaming = true; - - return HACKRF_SUCCESS; } static int create_transfer_thread(hackrf_device* device) From 503cd3316c0779498ccf11b4ca8ba7c742d1de17 Mon Sep 17 00:00:00 2001 From: Martin Ling Date: Fri, 18 Mar 2022 01:53:00 +0000 Subject: [PATCH 10/10] Remove request_exit() function. This just set the do_exit flag, and was now only called in one place. --- host/libhackrf/src/hackrf.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/host/libhackrf/src/hackrf.c b/host/libhackrf/src/hackrf.c index 0868c22b0..e6f015637 100644 --- a/host/libhackrf/src/hackrf.c +++ b/host/libhackrf/src/hackrf.c @@ -181,11 +181,6 @@ static int create_transfer_thread(hackrf_device* device); static libusb_context* g_libusb_context = NULL; int last_libusb_error = LIBUSB_SUCCESS; -static void request_exit(hackrf_device* device) -{ - device->do_exit = true; -} - /* * Check if the transfers are setup and owned by libusb. * @@ -1781,10 +1776,10 @@ static int kill_transfer_thread(hackrf_device* device) * thread has handled all completion callbacks. */ cancel_transfers(device); - /* - * Now call request_exit() to halt the main loop. - */ - request_exit(device); + + // Set flag to tell the thread to exit. + device->do_exit = true; + /* * Interrupt the event handling thread instead of * waiting for timeout.