From 282d46e68d9100af0dfdcc01e7689bb63bbf8419 Mon Sep 17 00:00:00 2001 From: ice458 <85405449+ice458@users.noreply.github.com> Date: Fri, 14 Aug 2026 16:00:06 +0900 Subject: [PATCH 1/2] usbtmc: re-arm (or stall) the bulk-OUT endpoint after a USB488 TRIGGER A single USB488 TRIGGER message left the bulk-OUT endpoint un-armed, so the host's next bulk-OUT transfer timed out. The trigger itself succeeded silently, so the failure surfaced on a later, unrelated command; only a USBTMC device clear recovered it. The bundled examples/device/usbtmc reproduced this as shipped. Every other branch of the STATE_IDLE dispatch in usbtmcd_xfer_cb() leaves the endpoint in a defined state: it either transitions out of STATE_IDLE so a later tud_usbtmc_start_bus_read() can re-arm it, or it stalls and lets the CLEAR_FEATURE(ENDPOINT_HALT) handler recover it. USBTMC_MSGID_USB488_TRIGGER did neither, and because the state stayed STATE_IDLE, even an application following the contract documented in usbtmc_device.h got a silent no-op from tud_usbtmc_start_bus_read(). Transition to STATE_NAK so the re-arm can take effect, and stall the endpoint when trigger is unsupported or the application callback rejects it, matching the existing handling for messages the driver cannot process. The callback result is deliberately not wrapped in TU_VERIFY(), which would return before the stall/re-arm and reintroduce the same hang. Since the driver now re-arms after a trigger, drop tud_usbtmc_msg_trigger_cb from the list of callbacks after which the application must do so. Fixes #3821 Co-Authored-By: Claude Opus 5 --- src/class/usbtmc/usbtmc_device.c | 18 +++++++++++++++--- src/class/usbtmc/usbtmc_device.h | 1 - 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/class/usbtmc/usbtmc_device.c b/src/class/usbtmc/usbtmc_device.c index 07190d89f..e248341ac 100644 --- a/src/class/usbtmc/usbtmc_device.c +++ b/src/class/usbtmc/usbtmc_device.c @@ -497,9 +497,21 @@ bool usbtmcd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint #if (CFG_TUD_USBTMC_ENABLE_488) case USBTMC_MSGID_USB488_TRIGGER: - // Spec says we halt the EP if we didn't declare we support it. - TU_VERIFY(usbtmc_state.capabilities->bmIntfcCapabilities488.supportsTrigger); - TU_VERIFY(tud_usbtmc_msg_trigger_cb(msg)); + // Unlike the messages above, TRIGGER is complete on arrival and has no response, so nothing else + // will move us out of STATE_IDLE. Do it here, otherwise the tud_usbtmc_start_bus_read() below (and + // any call the application makes from its callback) is a no-op and the bulk-OUT endpoint is left + // un-armed, silently timing out every subsequent host transfer. + TU_VERIFY(atomicChangeState(STATE_IDLE, STATE_NAK)); + + // Spec says we halt the EP if we didn't declare we support it; do the same when the application + // rejects the trigger. The callback result must not be wrapped in TU_VERIFY() here: returning + // early would skip both the stall and the re-arm below. + if (!usbtmc_state.capabilities->bmIntfcCapabilities488.supportsTrigger || + !tud_usbtmc_msg_trigger_cb(msg)) { + usbd_edpt_stall(rhport, usbtmc_state.ep_bulk_out); + return false; + } + tud_usbtmc_start_bus_read(); break; #endif diff --git a/src/class/usbtmc/usbtmc_device.h b/src/class/usbtmc/usbtmc_device.h index 3dc700876..efda84f16 100644 --- a/src/class/usbtmc/usbtmc_device.h +++ b/src/class/usbtmc/usbtmc_device.h @@ -25,7 +25,6 @@ // * tud_usbtmc_open_cb // * tud_usbtmc_msg_data_cb // * tud_usbtmc_msgBulkIn_complete_cb -// * tud_usbtmc_msg_trigger_cb // * (successful) tud_usbtmc_check_abort_bulk_out_cb // * (successful) tud_usbtmc_check_abort_bulk_in_cb // * (successful) tud_usmtmc_bulkOut_clearFeature_cb From af81f9ef42254301c2239eed657b0adce8b466b0 Mon Sep 17 00:00:00 2001 From: ice458 <85405449+ice458@users.noreply.github.com> Date: Fri, 14 Aug 2026 16:23:38 +0900 Subject: [PATCH 2/2] usbtmc: document why the trigger re-arm result is ignored A false return from tud_usbtmc_start_bus_read() here does not mean arming failed: it means the endpoint is already armed, either because the application re-armed it from its trigger callback or because a transfer is still queued (usbd_edpt_xfer() reports failure when the endpoint is busy). Both cases end in STATE_IDLE, so the state cannot disambiguate them either, and stalling on the result would halt a healthy endpoint. Co-Authored-By: Claude Opus 5 --- src/class/usbtmc/usbtmc_device.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/class/usbtmc/usbtmc_device.c b/src/class/usbtmc/usbtmc_device.c index e248341ac..0e9978a81 100644 --- a/src/class/usbtmc/usbtmc_device.c +++ b/src/class/usbtmc/usbtmc_device.c @@ -511,6 +511,9 @@ bool usbtmcd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint usbd_edpt_stall(rhport, usbtmc_state.ep_bulk_out); return false; } + // Result deliberately ignored: false here means the endpoint is already armed - either the + // application re-armed it from its callback, or a transfer is still queued - not that arming + // failed. Stalling on it would halt a healthy endpoint. tud_usbtmc_start_bus_read(); break;