Skip to content

fix: ignore window close while a flash is in progress #17

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

Merged
nedseb merged 2 commits into from
Apr 28, 2026
Merged

fix: ignore window close while a flash is in progress #17

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a8c2f8
fix: ignore window close while a flash is in progress
nedseb Apr 28, 2026
c1ba41a
fix: address Copilot review on close-guard
nedseb Apr 28, 2026
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
13 changes: 13 additions & 0 deletions src/ui/main_window.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,19 @@ impl MainWindow {
return Task::none();
}
iced::window::Event::CloseRequested => {
// Suppress the close while a flash is in progress so
// OpenOCD (or the FUS upgrade) isn't killed mid-write.
// The opacity overlay already signals the busy state
// visually; the user can SIGTERM if they really need
// out, but a misclick on the X shouldn't brick the
// device.
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

The comment suggests using SIGTERM as an escape hatch, but this app has Windows-specific code paths and SIGTERM isn’t generally applicable there. Consider rewording to a platform-neutral “force-quit / end task” phrasing to avoid confusion for Windows users and future maintainers.

Suggested change
// visually; the user can SIGTERM if they really need
// out, but a misclick on the X shouldn't brick the
// device.
// visually; the user can force-quit / end task the
// app if they really need out, but a misclick on the
// X shouldn't brick the device.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@nedseb nedseb Apr 28, 2026

Choose a reason for hiding this comment

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

Fixed in c1ba41a — applied your suggestion. SIGTERM dropped, replaced with "force-quit / end task" which is platform-neutral.

if self.tab_daplink.is_busy() || self.tab_ws.is_busy() {
eprintln!(
"Close request ignored: a flash is in progress."
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

The close-guard uses tab_daplink.is_busy()/tab_ws.is_busy(), but TabDaplink::is_busy() currently maps to is_readonly, which is also set during file-browse dialogs (not only during flashing). In that case the close will be ignored but the log message claims a flash is in progress, which is misleading. Consider either (a) making the guard check a flash-specific flag, or (b) broadening the message/comment to “operation in progress” and ensuring the semantics are consistent across tabs.

Suggested change
// Suppress the close while a flash is in progress so
// OpenOCD (or the FUS upgrade) isn't killed mid-write.
// The opacity overlay already signals the busy state
// visually; the user can SIGTERM if they really need
// out, but a misclick on the X shouldn't brick the
// device.
if self.tab_daplink.is_busy() || self.tab_ws.is_busy() {
eprintln!(
"Close request ignored: a flash is in progress."
// Suppress the close while an operation is in progress
// so tools such as OpenOCD (or the FUS upgrade) are
// not interrupted mid-action. The opacity overlay
// already signals the busy state visually; the user
// can SIGTERM if they really need out, but a misclick
// on the X should not interrupt an active operation.
if self.tab_daplink.is_busy() || self.tab_ws.is_busy() {
eprintln!(
"Close request ignored: an operation is in progress."

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@nedseb nedseb Apr 28, 2026

Choose a reason for hiding this comment

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

Fixed in c1ba41a — same fix as your other note: log/comment now say "operation in progress" instead of "flash in progress", consistent across both tabs and not misleading for the file-browse edge case.

);
return Task::none();
}

match dirs::get_settings_dir() {
Ok(settings_dir) => {
let fields_file = settings_dir.join(SETTINGS_FILE);
Expand Down
7 changes: 7 additions & 0 deletions src/ui/tab_daplink.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ pub struct TabDaplink {
}

impl TabDaplink {
/// True while a flash sequence (or a file browse) is running. Used by
/// `MainWindow` to suppress window-close requests during a flash so the
/// OpenOCD child isn't killed mid-operation.
pub fn is_busy(&self) -> bool {
self.is_readonly
}
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

is_busy() is implemented as self.is_readonly, but in this tab is_readonly is also toggled during file-browse (Browse*) operations, not just during flashing. Since MainWindow uses is_busy() to suppress window-close specifically to avoid killing OpenOCD mid-flash, it would be safer to expose a flash-specific “in progress” flag (set on StartProcess and cleared on DoneProcess) and use that for close suppression, leaving browse dialogs unaffected.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@nedseb nedseb Apr 28, 2026

Choose a reason for hiding this comment

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

Fixed in c1ba41a — went with option (b) you mentioned: rewored the log message and comments to "operation in progress" rather than introducing a separate flash-specific flag. The brief file-browse case is rare-to-impossible to trigger anyway (rfd uses a modal dialog under GTK3 so the parent X is unreachable while it's up), but the wording is now correct in either situation.


pub fn update(&mut self, message: TabDaplinkMessage) -> Task<Message> {
match message {
TabDaplinkMessage::LogMessage(log) => self.log_widget.push(log),
Expand Down
7 changes: 7 additions & 0 deletions src/ui/tab_wireless_stack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ const ALL_STACK: [WirelessStackFile; 21] = [
];

impl TabWirelessStack {
/// True while a flash sequence is running. Used by `MainWindow` to
/// suppress window-close requests so the OpenOCD child or the FUS
/// upgrade isn't killed mid-operation.
pub fn is_busy(&self) -> bool {
self.is_readonly
}

pub fn view(&self) -> Element<Message> {
let grid_fields = grid!(
grid_row!(
Expand Down
Loading