Send STOPFX3 on signal exit, not just on Ctrl-C#19
Open
mattgodbolt wants to merge 1 commit into
Open
Conversation
The capture loop sends STOPFX3 (and downclocks the ADC) only on a graceful loop exit, and that only happens on a signal the handler actually catches. Two gaps let a signal skip it and leave the FX3's GPIF engine streaming into a dead endpoint — wedging the next start (STARTFX3 timeout / RESETFX3 Io, needing a replug): 1. ctrlc is pulled in without the `termination` feature, so it catches SIGINT only. Running under `timeout(1)`, systemd, or any `kill` (all SIGTERM) bypasses the handler entirely and kills the process mid-stream with no STOPFX3. 2. Even when the signal is caught, it interrupts the blocking libusb event wait and makes rusb-async's poll() panic from inside the crate (it panic!s on any libusb_handle_events error, including EINTR / error -10), skipping the explicit STOPFX3. Fixes: - Enable ctrlc's `termination` feature (also catches SIGTERM/SIGHUP). - Add Fx3StopGuard, a Drop guard that sends STOPFX3 on every exit path including the panic unwind (the build is the default panic=unwind). - Break the capture loop gracefully if poll() returns an error while terminating, instead of treating it as a fatal transfer error. Verified on an RX888 mk2: a SIGTERM to a streaming process now shuts down cleanly with STOPFX3 sent; before, it was killed with no STOPFX3 and the next run failed to start the FX3 until a replug. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds more robust shutdown semantics for RX888 captures so the FX3 GPIF engine is stopped even when capture is interrupted by signals or rusb-async panics on EINTR.
Changes:
- Introduces a
Dropguard to always sendSTOPFX3(and ADC downclock) on scope exit/panic unwind. - Improves polling loop handling to treat some signal-related poll errors as orderly shutdown instead of fatal.
- Enables
ctrlc’sterminationfeature so SIGTERM/SIGHUP triggers the same termination path as SIGINT.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/main.rs | Adds Fx3StopGuard and updates poll loop / shutdown path to be signal- and panic-resilient. |
| Cargo.toml | Enables ctrlc termination feature so SIGTERM/SIGHUP invokes the handler and shutdown path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+203
to
+225
| /// Stops the FX3's GPIF streaming engine when dropped. | ||
| /// | ||
| /// The capture loop's normal exit sends STOPFX3 explicitly, but a signal | ||
| /// (SIGINT/SIGTERM) interrupts the blocking libusb event wait and makes | ||
| /// `rusb-async`'s `poll()` panic *from inside the crate* (it `panic!`s on any | ||
| /// `libusb_handle_events` error, including EINTR / error -10). A panic skips | ||
| /// the explicit shutdown, so without this guard an interrupted capture leaves | ||
| /// the FX3 streaming into a dead endpoint — the next run then fails to start | ||
| /// it (STARTFX3 timeout / RESETFX3 Io) and needs a physical replug. | ||
| /// | ||
| /// As a Drop guard it runs during the panic unwind too (the build uses the | ||
| /// default `panic = "unwind"`), guaranteeing STOPFX3 is sent on every exit | ||
| /// path. Errors are ignored: we're tearing down regardless. | ||
| struct Fx3StopGuard { | ||
| handle: Arc<DeviceHandle<Context>>, | ||
| } | ||
|
|
||
| impl Drop for Fx3StopGuard { | ||
| fn drop(&mut self) { | ||
| let _ = rx888_send_command(self.handle.as_ref(), FX3Command::STARTADC, 10_000_000); | ||
| let _ = rx888_send_command(self.handle.as_ref(), FX3Command::STOPFX3, 0); | ||
| } | ||
| } |
Owner
There was a problem hiding this comment.
adc has nowhere to send data to if gpif stops. i'm not sure if theres a shutdown pin that we can use here
GPIOPin::SHDWN is the right pin to use
Comment on lines
+213
to
+215
| /// As a Drop guard it runs during the panic unwind too (the build uses the | ||
| /// default `panic = "unwind"`), guaranteeing STOPFX3 is sent on every exit | ||
| /// path. Errors are ignored: we're tearing down regardless. |
Comment on lines
+418
to
+428
| let mut data = match transfer_pool.poll(timeout) { | ||
| Ok(data) => data, | ||
| // A signal (SIGINT/SIGTERM) interrupting the blocking libusb event | ||
| // wait can surface here as an error. If the handler has already set | ||
| // `terminate`, this is an orderly shutdown: break to the graceful | ||
| // STOPFX3 below rather than treating it as a fatal transfer error. | ||
| // (The signal can alternatively make poll() panic from inside | ||
| // rusb-async on EINTR — that path is covered by Fx3StopGuard.) | ||
| Err(_) if terminate.load(std::sync::atomic::Ordering::Relaxed) => break, | ||
| Err(e) => panic!("Transfer failed: {e:?}"), | ||
| }; |
Comment on lines
+220
to
+225
| impl Drop for Fx3StopGuard { | ||
| fn drop(&mut self) { | ||
| let _ = rx888_send_command(self.handle.as_ref(), FX3Command::STARTADC, 10_000_000); | ||
| let _ = rx888_send_command(self.handle.as_ref(), FX3Command::STOPFX3, 0); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The capture loop sends
STOPFX3(and downclocks the ADC) only on a graceful loop exit, and that graceful exit only happens on a signal the handler actually catches. Two gaps let a signal skip the shutdown and leave the FX3's GPIF engine streaming into a dead endpoint — which wedges the next start (STARTFX3: Timeout/RESETFX3: Io, recoverable only by a replug or USB reset):ctrlcis pulled in without theterminationfeature, so it installs a handler for SIGINT only. Running the streamer undertimeout(1), systemd, or any plainkill— all SIGTERM — bypasses the handler entirely and kills the process mid-stream with noSTOPFX3. InteractiveCtrl-Cworked; anything automated did not.Even when the signal is caught, it interrupts the blocking libusb event wait and makes
rusb-async'spoll()panic from inside the crate (itpanic!s on anylibusb_handle_eventserror, including EINTR / error-10), which also skips the explicitSTOPFX3.Fix
ctrlc'sterminationfeature so the handler also fires on SIGTERM/SIGHUP.Fx3StopGuard, aDropguard that sendsSTOPFX3on every exit path, including the panic unwind (the build uses the defaultpanic = "unwind").poll()returns an error while terminating, rather than treating it as a fatal transfer error.Testing
On an RX888 mk2: a
SIGTERMto a streaming process now shuts down cleanly withSTOPFX3sent (exit 0, or a caught panic-unwind that still runs the guard). Before this change the same signal killed the process with noSTOPFX3, and the next invocation could not start the FX3 until the device was physically replugged.🤖 Generated with Claude Code