fix(ui): guard ProgressWindow.destroy() against double-destroy TclError

[yashhzd]

Summary

• Adds winfo_exists() guard in ProgressWindow.destroy() to prevent TclError on double-destroy
• Adds two tests for double-destroy safety (direct and via auto-destroy at 100%)

Problem

When a flight controller connection succeeds:

1. update_progress_bar(100, 100) auto-destroys the Toplevel (line 132-133)
2. Context manager __exit__ calls destroy() again
3. FlightControllerConnectionProgress.destroy() checks if self.progress_window: — always True (Python object, not widget state)
4. ProgressWindow.destroy() → self.progress_window.destroy() on already-destroyed Toplevel → TclError

Fix

# Before (crashes)
def destroy(self) -> None:
    self.progress_window.destroy()

# After (safe)
def destroy(self) -> None:
    if self.progress_window.winfo_exists():
        self.progress_window.destroy()

Test plan

• New test_user_can_safely_destroy_progress_window_twice verifies no crash on double-destroy
• New test_user_sees_no_crash_when_progress_completes_then_destroys verifies the exact auto-destroy + manual destroy scenario
• CI passes

Fixes #1401

[Copilot]

Pull request overview

Guards ProgressWindow.destroy() against double-destroy scenarios in Tkinter to prevent TclError, and adds regression tests covering the crash path described in #1401.

Changes:

• Add a winfo_exists() check before destroying the Tkinter Toplevel.
• Add two tests to ensure calling destroy() twice (including after auto-destroy at 100%) does not raise.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
ardupilot_methodic_configurator/frontend_tkinter_progress_window.py	Adds existence guard before destroying the Tk widget to avoid TclError on double-destroy.
tests/test_frontend_tkinter_progress_window.py	Adds regression tests for double-destroy, including the “auto-destroy at 100% then manual destroy” scenario.

[Copilot]

winfo_exists() and destroy() can still raise tkinter.TclError if the underlying Tcl interpreter (or root) has already been destroyed (e.g., during app shutdown). To make destroy() fully idempotent and safe during teardown, consider wrapping the existence check + destroy call in a try/except TclError and treating that error as a no-op in this cleanup path.

Suggested change

	if self.progress_window.winfo_exists():
	self.progress_window.destroy()
	"""
	Safely destroy the progress window, ignoring TclError during interpreter teardown.
	"""
	try:
	if (
	hasattr(self, "progress_window")
	and self.progress_window is not None
	and self.progress_window.winfo_exists()
	):
	self.progress_window.destroy()
	except tk.TclError:
	# Interpreter or widget may already be destroyed; ignore in teardown.
	pass

[Copilot]

The docstring says the window was destroyed “e.g. by reaching 100%”, but the test destroys it manually. To keep the test self-descriptive, either adjust the GIVEN wording to “has already been destroyed” (without the 100% example) or change the setup to destroy via update_progress_bar(100, 100).

[Copilot]

This test currently only checks that the second destroy() call doesn’t raise. Adding a post-condition assertion (e.g., assert not progress_window.progress_window.winfo_exists()) would make the intended end state explicit and help catch regressions where destroy() might recreate/replace the widget reference.

Suggested change

	progress_window.destroy()
	progress_window.destroy()
	# Window should remain destroyed
	assert not progress_window.progress_window.winfo_exists()

[amilcarlucas]

Thanks, good catch.