Skip to content

IEP-1765 Improve target detection UX by adding option for detailed detection script output#1459

Open
sigmaaa wants to merge 3 commits into
masterfrom
IEP-1765
Open

IEP-1765 Improve target detection UX by adding option for detailed detection script output#1459
sigmaaa wants to merge 3 commits into
masterfrom
IEP-1765

Conversation

@sigmaaa
Copy link
Copy Markdown
Collaborator

@sigmaaa sigmaaa commented May 12, 2026

Description

Added a new warning message for when boards are not detected. Additionally, added a checkbox to enable detailed script output, which contains useful debugging information—such as the MAC address—if the target is not detected.

image

Fixes # (IEP-1765)

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How has this been tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Test Configuration:

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

Dependent components impacted by this PR:

  • Edit target wizard

Checklist

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

Summary by CodeRabbit

  • New Features

    • Added "Enable detailed output" checkbox to the serial target wizard and persisted its setting.
    • New output/info group in the wizard to show probe output and status.
  • Bug Fixes

    • Wizard now shows a warning when no boards are detected.
    • Improved handling when no serial ports are available and clearer probe output modes.

Review Change Stack

@sigmaaa sigmaaa self-assigned this May 12, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 12, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 976ee78b-abb9-4ef6-bee6-04122a76c149

📥 Commits

Reviewing files that changed from the base of the PR and between dea5747 and b95fb7e.

📒 Files selected for processing (2)
  • bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/NewSerialFlashTargetWizard.java
  • bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/NewSerialFlashTargetWizardPage.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/NewSerialFlashTargetWizardPage.java

📝 Walkthrough

Walkthrough

Adds a detailed-output toggle to the New Serial Flash Target wizard: new localization keys, a persisted preference for the checkbox, an info-area refactor with checkbox and text output, warning messaging when no boards are found, and branching output rendering based on the toggle.

Changes

Serial Target Wizard Detailed Output Feature

Layer / File(s) Summary
Message keys and localization strings
bundles/com.espressif.idf.launch.serial.ui/.../Messages.java, bundles/com.espressif.idf.launch.serial.ui/.../messages.properties
New message constants and properties for the detailed-output checkbox label, no-boards-detected warning, target output group, and related labels.
Preference persistence for detailed-output
bundles/com.espressif.idf.launch.serial.ui/.../NewSerialFlashTargetWizard.java
New helper stores the detailed-output checkbox state to InstanceScope preferences; performFinish() calls the helper; BackingStoreException is logged via Logger.
UI and output rendering
bundles/com.espressif.idf.launch.serial.ui/.../NewSerialFlashTargetWizardPage.java
Refactors info area into createInfoAreaGroup(...) with a checkbox initialized from prefs, adds isOutputDetailed state and getter, sets a wizard warning when no boards detected, fixes empty-combo early return, and branches TargetPortInfo.run to render either full lines (detailed) or progress-dots + buffered parsing (standard).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • kolipakakondal
  • AndriiFilippov
  • alirana01

Poem

🐰 A tiny checkbox, softly lit,
Toggles lines or dots in a skittering fit.
When boards are quiet a warning will peep,
Output now steady, awake, or asleep,
The wizard hops on, logs safe and neat.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.38% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and clearly describes the main change: adding a checkbox option to enable detailed detection script output for improved target detection UX.
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.

✏️ 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-1765

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/NewSerialFlashTargetWizardPage.java`:
- Around line 532-544: The loop reading from bufferedReader currently calls
appendToInfoArea(".") unconditionally and again in the else branch, causing
duplicate dots; remove the unconditional appendToInfoArea(".") before the if
(isOutputDetailed) check so that appendToInfoArea(readLine +
System.lineSeparator()) runs only in detailed mode and appendToInfoArea(".")
plus chipInfo.append(readLine)... runs only in the else branch (refer to the
while ((readLine = bufferedReader.readLine()) != null) loop, isOutputDetailed,
appendToInfoArea, and chipInfo).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 18ebd60d-c9cf-4773-8b4a-111501504176

📥 Commits

Reviewing files that changed from the base of the PR and between 1b99852 and dea5747.

📒 Files selected for processing (3)
  • bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/Messages.java
  • bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/NewSerialFlashTargetWizardPage.java
  • bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/messages.properties

@AndriiFilippov
Copy link
Copy Markdown
Collaborator

@sigmaaa hi !
I think we should keep the checkbox selection saved. It feels more intuitive, and users will probably want this type of output consistently. Having to re-enable the checkbox every time would likely be inconvenient and frustrating.

Screen.Recording.2026-05-13.at.9.18.33.mov

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
{}

@sigmaaa
Copy link
Copy Markdown
Collaborator Author

sigmaaa commented May 13, 2026

Hi @AndriiFilippov, added storing checkbox status in the last commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants