SWTException: Widget is disposed in NewSerialFlashTargetWizardPage during background chip detection

[sigmaaa]

Description

Created a method appendToInfoArea that checks whether the widget is disposed before using it.

Fixes # (IEP-1757)

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

How has this been tested?

Edit the launch target and select the port. While information is being appended to the widget, close the dialog or click Apply. Then check the log — there should be no SWT “widget is disposed” exception.

Test Configuration:

• ESP-IDF Version:
• OS (Windows,Linux and macOS):

Dependent components impacted by this PR:

• Launch target dialog

Checklist

• PR Self Reviewed
• Applied Code formatting
• Added Documentation
• Added Unit Test
• Verified on all platforms - Windows,Linux and macOS

Summary by CodeRabbit

• Refactor
  • Improved internal code organization for UI update operations, enhancing code maintainability and reliability.

[coderabbitai]

📝 Walkthrough

Walkthrough

A reusable appendToInfoArea method is introduced to encapsulate thread-safe UI updates using display.asyncExec and widget disposal checks. The TargetPortInfo.run() method is refactored to consistently use this helper for all info area updates, replacing inline async blocks.

Changes

Cohort / File(s)	Summary
Thread-Safe UI Update Refactoring bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/NewSerialFlashTargetWizardPage.java	Extracts a reusable appendToInfoArea helper method for synchronized widget updates. Refactors TargetPortInfo.run() to use this helper for initial status messages, incremental progress indicators, conditional chip-type messages, and trailing separators, replacing previously inline async execution blocks.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

• AndriiFilippov
• kolipakakondal
• alirana01

Poem

🐰 A helper hopped into view,
Threading safely, old code new,
Async calls now tucked away,
Widget updates, the cleaner way! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title directly addresses the bug being fixed: an SWTException during background chip detection in the specific class being modified.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch IEP-1757

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/NewSerialFlashTargetWizardPage.java (1)

493-527: Please add SWTBot regression coverage for this wizard flow.

This is a UI/workflow change in async port detection; add or extend a SWTBot test (Given-When-Then) that covers port selection plus page disposal while detection is running.

Based on learnings: Add or extend SWTBot tests in com.espressif.idf.ui.test following Given-When-Then structure when changing UI/workflows/wizards.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/NewSerialFlashTargetWizardPage.java`
around lines 493 - 527, Add a SWTBot regression test in the
com.espressif.idf.ui.test suite that exercises the async port-detection wizard
flow in NewSerialFlashTargetWizardPage: simulate triggering
espToolCommands.chipInformation for a serial port, verify appendToInfoArea
progress output and that extractChipFromChipInfoOutput result is handled
(TargetPortFoundMessage/TargetPortNotFoundMessage), and include a
Given-When-Then case that disposes the wizard page while detection is running to
assert no UI hang or exceptions; use SWTBot to open the wizard, select a port,
start detection, dispose the page mid-detection, and assert expected UI state
and absence of errors.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/NewSerialFlashTargetWizardPage.java`:
- Around line 493-527: Add a SWTBot regression test in the
com.espressif.idf.ui.test suite that exercises the async port-detection wizard
flow in NewSerialFlashTargetWizardPage: simulate triggering
espToolCommands.chipInformation for a serial port, verify appendToInfoArea
progress output and that extractChipFromChipInfoOutput result is handled
(TargetPortFoundMessage/TargetPortNotFoundMessage), and include a
Given-When-Then case that disposes the wizard page while detection is running to
assert no UI hang or exceptions; use SWTBot to open the wizard, select a port,
start detection, dispose the page mid-detection, and assert expected UI state
and absence of errors.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5819884b-359e-4814-aff2-da42a618b284

📥 Commits

Reviewing files that changed from the base of the PR and between 24024e5 and 031d02b.

📒 Files selected for processing (1)

• bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/NewSerialFlashTargetWizardPage.java

[kolipakakondal]

LGTM

[kolipakakondal]

Was there any functionality issue without this while making changes to the launch target and selecting the port?

[sigmaaa]

Was there any functionality issue without this while making changes to the launch target and selecting the port?

Hi Kondal, thanks for the review. I don’t think there are any functional issues with this (at least none that I’m aware of). However, the exception makes the logs harder to read, so I decided to fix it.