-
Notifications
You must be signed in to change notification settings - Fork 133
IEP-1772 Show meaningful error message instead of invalid command name "program_esp_bins" #1481
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -57,13 +57,15 @@ | |||||||||||||||||||||||||||
| import org.eclipse.debug.core.ILaunchConfiguration; | ||||||||||||||||||||||||||||
| import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy; | ||||||||||||||||||||||||||||
| import org.eclipse.debug.core.ILaunchManager; | ||||||||||||||||||||||||||||
| import org.eclipse.debug.core.IStatusHandler; | ||||||||||||||||||||||||||||
| import org.eclipse.debug.core.model.ISourceLocator; | ||||||||||||||||||||||||||||
| import org.eclipse.embedcdt.core.StringUtils; | ||||||||||||||||||||||||||||
| import org.eclipse.embedcdt.debug.gdbjtag.core.DebugUtils; | ||||||||||||||||||||||||||||
| import org.eclipse.embedcdt.debug.gdbjtag.core.dsf.AbstractGnuMcuLaunchConfigurationDelegate; | ||||||||||||||||||||||||||||
| import org.eclipse.embedcdt.debug.gdbjtag.core.dsf.GnuMcuServerServicesLaunchSequence; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| import com.espressif.idf.core.logging.Logger; | ||||||||||||||||||||||||||||
| import com.espressif.idf.core.variable.JtagVariableResolver; | ||||||||||||||||||||||||||||
| import com.espressif.idf.debug.gdbjtag.openocd.Activator; | ||||||||||||||||||||||||||||
| import com.espressif.idf.debug.gdbjtag.openocd.Configuration; | ||||||||||||||||||||||||||||
| import com.espressif.idf.debug.gdbjtag.openocd.ConfigurationAttributes; | ||||||||||||||||||||||||||||
|
|
@@ -80,6 +82,12 @@ public class LaunchConfigurationDelegate extends AbstractGnuMcuLaunchConfigurati | |||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| private static final ThreadLocal<LaunchOptions> pendingLaunchOptions = new ThreadLocal<>(); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||
| * Status code used to route the "no board selected" failure through {@code BoardNotSelectedStatusHandler}. It must | ||||||||||||||||||||||||||||
| * match the {@code code} of the corresponding {@code statusHandler} extension in plugin.xml. | ||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||
| public static final int BOARD_NOT_SELECTED_STATUS_CODE = 6001; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| ILaunchConfiguration fConfig = null; | ||||||||||||||||||||||||||||
| @SuppressWarnings("unused") | ||||||||||||||||||||||||||||
| private boolean fIsNonStopSession = false; | ||||||||||||||||||||||||||||
|
|
@@ -606,10 +614,54 @@ protected IPath checkBinaryDetails(final ILaunchConfiguration config) throws Cor | |||||||||||||||||||||||||||
| if (configOptions.isEmpty()) | ||||||||||||||||||||||||||||
| throw new CoreException( | ||||||||||||||||||||||||||||
| new Status(IStatus.ERROR, Activator.PLUGIN_ID, "Missing mandatory configuration.")); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // Abort early with a clear, actionable message when no board is selected for the active target. Otherwise | ||||||||||||||||||||||||||||
| // OpenOCD would start without a board configuration and fail later with the cryptic | ||||||||||||||||||||||||||||
| // "invalid command name \"program_esp_bins\"" error. | ||||||||||||||||||||||||||||
| if (!isBoardConfigured(config)) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| IStatus status = new Status(IStatus.OK, Activator.PLUGIN_ID, BOARD_NOT_SELECTED_STATUS_CODE, | ||||||||||||||||||||||||||||
| "No board is selected for the debug target.", null); //$NON-NLS-1$ | ||||||||||||||||||||||||||||
| IStatusHandler handler = DebugPlugin.getDefault().getStatusHandler(status); | ||||||||||||||||||||||||||||
| if (handler != null) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| handler.handleStatus(status, null); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| throw new DebugException(Status.OK_STATUS); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| return super.checkBinaryDetails(config); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||
| * Determines whether a board configuration is available for the given launch configuration. A board is considered | ||||||||||||||||||||||||||||
| * configured when it is resolvable from the active launch target, or when the (possibly manually edited) resolved | ||||||||||||||||||||||||||||
| * Config options already reference a board configuration file. Without either, OpenOCD would start without the | ||||||||||||||||||||||||||||
| * board-specific commands (e.g. {@code program_esp_bins}) and the debug session would fail. | ||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||
| * @param config the debug launch configuration | ||||||||||||||||||||||||||||
| * @return {@code true} if a board configuration is available, {@code false} otherwise | ||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||
| private boolean isBoardConfigured(ILaunchConfiguration config) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| if (JtagVariableResolver.isBoardConfigResolvable()) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // Fall back to inspecting the resolved Config options for a manually configured board file. | ||||||||||||||||||||||||||||
| try | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| String resolvedOptions = Configuration.resolveAll(Configuration.getGdbServerOtherConfig(config), config); | ||||||||||||||||||||||||||||
| return resolvedOptions != null && resolvedOptions.contains("board/"); //$NON-NLS-1$ | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
Comment on lines
+652
to
+657
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Accept Windows paths in manual board-config detection. Both fallbacks require the literal
Proposed fix- return resolvedOptions != null && resolvedOptions.contains("board/");
+ return resolvedOptions != null
+ && resolvedOptions.replace('\\', '/').contains("board/");- return resolved != null && resolved.contains("board/");
+ return resolved != null && resolved.replace('\\', '/').contains("board/");📝 Committable suggestion
Suggested change
📍 Affects 2 files
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
| catch (CoreException e) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| Activator.log(e); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||
| protected Sequence getServicesSequence(DsfSession session, ILaunch launch, IProgressMonitor progressMonitor) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| /******************************************************************************* | ||
| * Copyright 2026 Espressif Systems (Shanghai) PTE LTD. All rights reserved. | ||
| * Use is subject to license terms. | ||
| *******************************************************************************/ | ||
| package com.espressif.idf.debug.gdbjtag.openocd.ui; | ||
|
|
||
| import org.eclipse.core.runtime.CoreException; | ||
| import org.eclipse.core.runtime.IStatus; | ||
| import org.eclipse.debug.core.IStatusHandler; | ||
| import org.eclipse.jface.dialogs.MessageDialog; | ||
| import org.eclipse.launchbar.core.ILaunchBarManager; | ||
| import org.eclipse.launchbar.core.target.ILaunchTarget; | ||
| import org.eclipse.launchbar.ui.target.ILaunchTargetUIManager; | ||
| import org.eclipse.swt.widgets.Display; | ||
|
|
||
| import com.espressif.idf.core.logging.Logger; | ||
| import com.espressif.idf.debug.gdbjtag.openocd.Activator; | ||
|
|
||
| /** | ||
| * Shows a clear, actionable dialog when a debug session is started without a board selected for the active launch | ||
| * target. Confirming the dialog opens the launch target editor so the user can select a board. | ||
| */ | ||
| public class BoardNotSelectedStatusHandler implements IStatusHandler | ||
| { | ||
| @Override | ||
| public Object handleStatus(IStatus status, Object source) throws CoreException | ||
| { | ||
| Display.getDefault().asyncExec(() -> { | ||
| boolean isYes = MessageDialog.openConfirm(Display.getDefault().getActiveShell(), | ||
| Messages.BoardNotSelectedDialog_title, Messages.BoardNotSelectedDialog_message); | ||
| if (isYes) | ||
| { | ||
| editActiveLaunchTarget(); | ||
| } | ||
| }); | ||
| return null; | ||
| } | ||
|
|
||
| private void editActiveLaunchTarget() | ||
| { | ||
| try | ||
| { | ||
| ILaunchBarManager launchBarManager = Activator.getService(ILaunchBarManager.class); | ||
| ILaunchTargetUIManager targetUIManager = Activator.getService(ILaunchTargetUIManager.class); | ||
| if (launchBarManager == null || targetUIManager == null) | ||
| { | ||
| return; | ||
| } | ||
| ILaunchTarget target = launchBarManager.getActiveLaunchTarget(); | ||
| if (target != null) | ||
| { | ||
| targetUIManager.editLaunchTarget(target); | ||
| } | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| Logger.log(e); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate board selection for attach launches too.
checkBinaryDetails()is called only underif (!attach)(Lines 339-340). An attach launch with “Start OpenOCD locally” enabled therefore bypasses this guard and can still start OpenOCD without a board configuration. Extract this check and run it whenever the server is started, before the attach branch.🤖 Prompt for AI Agents