IEP-1000 Board doesn't match the target after cancel in debug/jtag tab #799
Conversation
| public class SvdPathResolver implements IDynamicVariableResolver | ||
| { | ||
|
|
||
| public String resolveValue(IDynamicVariable variable, String argument) throws CoreException |
There was a problem hiding this comment.
mostly code from the updateSvd method
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/LaunchBarListener.java
Outdated
Show resolved
Hide resolved
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/LaunchBarListener.java
Outdated
Show resolved
Hide resolved
|
Hi, @alirana01, thanks for the review. Fixed mentioned points in the latest commit |
bundles/com.espressif.idf.debug.gdbjtag.openocd/plugin.properties
Outdated
Show resolved
Hide resolved
...f.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/SvdPathResolver.java
Outdated
Show resolved
Hide resolved
...f.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/SvdPathResolver.java
Show resolved
Hide resolved
|
Hi @AndriiFilippov Could you please verify this? |
|
Hi @sigmaaa ! Tested under: OS - Windows 10 Test 1 👍 After pressing the "cancel" button in the debug configuration, there is no confusion between the target(ESP32) and board type. However, in the Launch Target, a switch occurs to a new (canceled target ESP32c3). As a result, if you try to build the project, you will encounter an error. There is nothing to worry about - you just need to manually change the target back to the one that was used before. Nevertheless, I believe this could potentially mislead the user. I think we should consider how to automatically return to the original target.
|
WalkthroughThe changes primarily focus on improving the handling of System View Description (SVD) paths and launch configuration attributes. The introduction of the Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 5
Files ignored due to filter (1)
- bundles/com.espressif.idf.debug.gdbjtag.openocd/plugin.xml
Files selected for processing (6)
- bundles/com.espressif.idf.debug.gdbjtag.openocd/plugin.properties (1 hunks)
- bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/SvdPathResolver.java (1 hunks)
- bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabDebugger.java (4 hunks)
- bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabSvdTarget.java (2 hunks)
- bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/CMakeMainTab2.java (1 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/LaunchBarListener.java (4 hunks)
Files skipped from review due to trivial changes (3)
- bundles/com.espressif.idf.debug.gdbjtag.openocd/plugin.properties
- bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabSvdTarget.java
- bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/CMakeMainTab2.java
Additional comments (Suppressed): 8
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/LaunchBarListener.java (4)
46-61: The use of lambda expressions instead of anonymous classes is a good practice as it makes the code more readable and concise. The change from
new Runnable()to() ->is an improvement.67-79: No significant changes were made in this hunk. The replacement of
""withStringUtil.EMPTYis a minor change that improves readability by making the purpose of the empty string clearer.97-125: The logic for checking if the target has changed and prompting the user for action has been modified. The condition for checking if JTAG flashing is enabled or if the launch configuration type is debug has been updated. This change seems to be in line with the PR summary, but ensure that it doesn't introduce any unexpected behavior.
180-183: The return statement in the
deleteDirectorymethod has been simplified. This change improves readability without altering the functionality.bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabDebugger.java (4)
88-89: The import statement for
LaunchBarListenerhas been added. Ensure that this class is available in the project and its methods are accessible from this context.177-178: A flag
ignoreJtagTargetChangeis being set/reset in thecreateControlmethod usingLaunchBarListener.setIgnoreJtagTargetChange(). This change seems to be part of the solution to prevent saving the target attribute of the configuration at the wrong time. However, ensure that this flag is used correctly in other parts of the code where the JTAG target change is handled.461-466: The code updating launch configuration attributes in the
widgetSelectedmethod has been removed. This change is consistent with the PR summary, which mentions removing the saving of configuration in listeners. However, make sure that these attributes are updated elsewhere as necessary, especially theTARGET_FOR_JTAGattribute, which seems important for the debugging process.1504-1510: The code updating launch configuration attributes in the
performApplymethod has been simplified by directly setting the attributes on the given configuration instead of creating a working copy. This change simplifies the logic and avoids unnecessary object creation, which can improve performance. However, ensure that this change does not introduce any issues related to concurrency or shared state, as multiple threads might access the same configuration object simultaneously.
...f.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/SvdPathResolver.java
Show resolved
Hide resolved
...f.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/SvdPathResolver.java
Show resolved
Hide resolved
...f.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/SvdPathResolver.java
Show resolved
Hide resolved
...f.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/SvdPathResolver.java
Show resolved
Hide resolved
...f.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/SvdPathResolver.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Review Status
Actionable comments generated: 5
Files selected for processing (6)
- bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/Messages.java (1 hunks)
- bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabDebugger.java (44 hunks)
- bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/messages.properties (1 hunks)
- bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/CMakeMainTab2.java (5 hunks)
- bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/Messages.java (1 hunks)
- bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/messages.properties (1 hunks)
Files skipped from review due to trivial changes (4)
- bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/Messages.java
- bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/messages.properties
- 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/messages.properties
Additional comments (Suppressed): 20
bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/CMakeMainTab2.java (4)
38-47: The new hunk introduces additional imports from the
org.eclipse.core.runtimepackage. Ensure that these are necessary for the changes made in this PR.104-110: A new field
lastAppliedConfigurationof typeILaunchConfigurationWorkingCopyhas been added. This seems to be used to store the last applied launch configuration.118-146: The
createControlmethod has been updated to include a new jobrevertTargetJobwhich is scheduled to run after 100ms when the parent composite is disposed. This job reverts the active launch target to the one stored inlastAppliedConfiguration. This change appears to be part of the fix for the bug where the board selection doesn't match the target after canceling in the debug/JTAG tab.586-592: The
lastAppliedConfigurationfield is now being set to the working copy of the launch configuration before it's saved. This allows the last applied configuration to be accessed later, such as in therevertTargetJobin thecreateControlmethod.bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabDebugger.java (16)
26-42: The new hunk has added a few more import statements. Ensure that these new imports are used in the code and are necessary.
90-96: A new import statement has been added. Ensure that this new import is used in the code and is necessary.
100-108: No significant changes have been made in this hunk, only the addition of
//$NON-NLS-1$comments which are used for internationalization and localization in Eclipse. These comments indicate that the string does not need to be translated.145-151: No significant changes have been made in this hunk, only the addition of
//$NON-NLS-1$comments which are used for internationalization and localization in Eclipse. These comments indicate that the string does not need to be translated.239-253: No significant changes have been made in this hunk, only the addition of
//$NON-NLS-1$comments which are used for internationalization and localization in Eclipse. These comments indicate that the string does not need to be translated.331-337: No significant changes have been made in this hunk, only the addition of
//$NON-NLS-1$comments which are used for internationalization and localization in Eclipse. These comments indicate that the string does not need to be translated.357-370: No significant changes have been made in this hunk, only the addition of
//$NON-NLS-1$comments which are used for internationalization and localization in Eclipse. These comments indicate that the string does not need to be translated.382-396: No significant changes have been made in this hunk, only the addition of
//$NON-NLS-1$comments which are used for internationalization and localization in Eclipse. These comments indicate that the string does not need to be translated.404-422: No significant changes have been made in this hunk, only the addition of
//$NON-NLS-1$comments which are used for internationalization and localization in Eclipse. These comments indicate that the string does not need to be translated.456-461: No significant changes have been made in this hunk, only the addition of
//$NON-NLS-1$comments which are used for internationalization and localization in Eclipse. These comments indicate that the string does not need to be translated.477-484: No significant changes have been made in this hunk, only the addition of
//$NON-NLS-1$comments which are used for internationalization and localization in Eclipse. These comments indicate that the string does not need to be translated.555-562: No significant changes have been made in this hunk, only the addition of
//$NON-NLS-1$comments which are used for internationalization and localization in Eclipse. These comments indicate that the string does not need to be translated.600-606: No significant changes have been made in this hunk, only the addition of
//$NON-NLS-1$comments which are used for internationalization and localization in Eclipse. These comments indicate that the string does not need to be translated.625-632: No significant changes have been made in this hunk, only the addition of
//$NON-NLS-1$comments which are used for internationalization and localization in Eclipse. These comments indicate that the string does not need to be translated.694-700: No significant changes have been made in this hunk, only the addition of
//$NON-NLS-1$comments which are used for internationalization and localization in Eclipse. These comments indicate that the string does not need to be translated.793-799: No significant changes have been made in this hunk, only the addition of
//$NON-NLS-1$comments which are used for internationalization and localization in Eclipse. These comments indicate that the string does not need to be translated.808
| } | ||
|
|
||
| if (!selectedItem.contentEquals(updatedSelectedTarget) && isFlashOverJtag) { | ||
| try { | ||
| ILaunchConfigurationWorkingCopy wc = launchBarManager.getActiveLaunchConfiguration() | ||
| .getWorkingCopy(); | ||
| wc.setAttribute(IDFLaunchConstants.TARGET_FOR_JTAG, selectedItem); | ||
| wc.doSave(); | ||
| } catch (CoreException e2) { | ||
| Logger.log(e2); | ||
| } | ||
| updateLaunchBar(selectedItem); | ||
| } | ||
| boardConfigsMap = parser.getBoardsConfigs(selectedItem); |
There was a problem hiding this comment.
The code for updating the launch configuration with the selected target has been removed from the if block. Instead, the updateLaunchBar method is called directly. This change simplifies the code and removes the need to handle a CoreException here.
| @Override | ||
| public void createControl(Composite parent) | ||
| { | ||
|
|
||
| if (Activator.getInstance().isDebugging()) | ||
| { | ||
| System.out.println("openocd.TabDebugger.createControl() "); | ||
| System.out.println("openocd.TabDebugger.createControl() "); //$NON-NLS-1$ | ||
| } | ||
|
|
||
| LaunchBarListener.setIgnoreJtagTargetChange(true); | ||
| parent.addDisposeListener(event -> { | ||
| String targetNameFromUI = fTargetName.getText(); | ||
| // TODO: find a better way to roll back target change in the launch bar when Cancel was pressed. | ||
| // We have to do like this because we don't have access to the cancel button | ||
| Job revertTargetJob = new Job(Messages.TabDebugger_SettingTargetJob) | ||
| { | ||
| protected IStatus run(IProgressMonitor monitor) | ||
| { | ||
| try | ||
| { | ||
| String targetName = lastAppliedConfiguration.getOriginal() | ||
| .getAttribute(IDFLaunchConstants.TARGET_FOR_JTAG, targetNameFromUI); | ||
| ILaunchTargetManager launchTargetManager = Activator.getService(ILaunchTargetManager.class); | ||
| ILaunchTarget selectedTarget = Stream.of(launchTargetManager.getLaunchTargets()) | ||
| .filter(target -> target.getId().contentEquals((targetName))).findFirst().orElseGet(() -> null); | ||
| launchBarManager.setActiveLaunchTarget(selectedTarget); | ||
| return Status.OK_STATUS; | ||
| } | ||
| catch (CoreException e) | ||
| { | ||
| Logger.log(e); | ||
| return Status.CANCEL_STATUS; | ||
| } | ||
| } | ||
| }; | ||
| revertTargetJob.schedule(100); | ||
| LaunchBarListener.setIgnoreJtagTargetChange(false); | ||
| }); | ||
|
|
||
| if (!(parent instanceof ScrolledComposite)) | ||
| { | ||
| ScrolledComposite sc = new ScrolledComposite(parent, SWT.V_SCROLL | SWT.H_SCROLL); |
There was a problem hiding this comment.
This hunk introduces a new logic to handle the cancellation of target change in the launch bar. It creates a job to revert the target change if the Cancel button was pressed. This seems like a workaround due to lack of access to the cancel button. While this might work, it's worth considering if there's a more direct way to handle this scenario.
|
|
||
| { | ||
| Label label = new Label(comp, SWT.NONE); | ||
| label.setText(Messages.getString("DebuggerTab.gdbServerTelnetPort_Label")); | ||
| label.setToolTipText(Messages.getString("DebuggerTab.gdbServerTelnetPort_ToolTipText")); | ||
| label.setText(Messages.getString("DebuggerTab.gdbServerTelnetPort_Label")); //$NON-NLS-1$ | ||
| label.setToolTipText(Messages.getString("DebuggerTab.gdbServerTelnetPort_ToolTipText")); //$NON-NLS-1$ | ||
|
|
||
| fGdbServerTelnetPort = new Text(comp, SWT.SINGLE | SWT.BORDER); | ||
| GridData gd = new GridData(); |
There was a problem hiding this comment.
Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [427-437]
The new hunk has removed some code related to updating the launch configuration when the selected item is not equal to the updated selected target. Ensure that this removal doesn't affect the functionality of the application.
| String selectedItem = fTarget.getText(); | ||
| if (!selectedItem.contentEquals(updatedSelectedTarget)) | ||
| { | ||
| try | ||
| { | ||
| ILaunchConfigurationWorkingCopy wc = launchBarManager.getActiveLaunchConfiguration() | ||
| .getWorkingCopy(); | ||
| wc.setAttribute(IDFLaunchConstants.TARGET_FOR_JTAG, selectedItem); | ||
| TabSvdTarget.updateSvd(selectedItem, wc); | ||
| wc.doSave(); | ||
| } | ||
| catch (CoreException e1) | ||
| { | ||
| Logger.log(e1); | ||
| } | ||
| updateLaunchBar(selectedItem); | ||
| } | ||
| fGdbClientExecutable.setText(IDFUtil.getXtensaToolchainExecutablePathByTarget(selectedItem)); |
There was a problem hiding this comment.
The new hunk has removed some code related to updating the launch configuration when the selected item is not equal to the updated selected target. Ensure that this removal doesn't affect the functionality of the application.
| } | ||
|
|
||
| int ret = -1; | ||
| if ("global".equals(text)) | ||
| if ("global".equals(text)) //$NON-NLS-1$ | ||
| { | ||
| ret = PreferencesUtil.createPreferenceDialogOn(parent.getShell(), GlobalMcuPage.ID, null, null) | ||
| .open(); | ||
| } | ||
| else if ("workspace".equals(text)) | ||
| else if ("workspace".equals(text)) //$NON-NLS-1$ | ||
| { | ||
| ret = PreferencesUtil.createPreferenceDialogOn(parent.getShell(), WorkspaceMcuPage.ID, null, null) | ||
| .open(); | ||
| } | ||
| else if ("project".equals(text)) | ||
| else if ("project".equals(text)) //$NON-NLS-1$ | ||
| { | ||
| assert (fConfiguration != null); | ||
| IProject project = EclipseUtils.getProjectByLaunchConfiguration(fConfiguration); |
There was a problem hiding this comment.
The new hunk has removed some code related to updating the launch configuration when the selected item is not equal to the updated selected target. Ensure that this removal doesn't affect the functionality of the application.
Hi @AndriiFilippov, @kolipakakondal , @alirana01 |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabDebugger.java (45 hunks)
- bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/CMakeMainTab2.java (6 hunks)
Files skipped from review due to trivial changes (1)
- bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabDebugger.java
Additional comments (Suppressed): 6
bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/CMakeMainTab2.java (6)
41-44: New import statements have been added for
IProgressMonitor,IStatus,Status, andJob. These classes are used for creating and managing background jobs in Eclipse. The addition of these imports suggests that new functionality related to background jobs has been added to theCMakeMainTab2class.88-88: A new constant
JOB_DELAY_MShas been introduced. This constant is used to specify the delay before a job is scheduled to run. The value is set to 100 milliseconds. This change suggests that a new job is being scheduled somewhere in the class.121-125: The
addDisposeListenermethod has been updated to include a call toscheduleRevertTargetJob(targetNameFromUI)before settingIgnoreJtagTargetChangetofalse. This change ensures that the target is reverted to its original value when the parent composite is disposed. This is a good practice as it ensures that the state of the application is consistent even when UI elements are disposed.568-573: The
doSavemethod is now called inside a try block, and anyCoreExceptionthat occurs is logged. This is a good practice as it ensures that any exceptions that occur during the save operation are caught and logged, preventing the application from crashing unexpectedly.715-718: The code for updating the launch configuration with the selected target has been removed from the if block. Instead, the
updateLaunchBarmethod is called directly. This change simplifies the code and removes the need to handle aCoreExceptionhere.860-884: A new method
scheduleRevertTargetJobhas been added. This method creates and schedules a new job that reverts the target to its original value. The job is scheduled to run after a delay specified byJOB_DELAY_MS. This is a good practice as it ensures that the target is reverted to its original value in a non-blocking manner, allowing the UI to remain responsive.
|
@sigmaaa hi ! Tested under: In general, everything is good, except for one scenario: create new project -> go to Edit Launch Config -> click "Cancel" -> the Launch Mode and Launch Target disappear.
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabDebugger.java (45} hunks)
- bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/CMakeMainTab2.java (6} hunks)
Files skipped from review due to trivial changes (1)
- bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabDebugger.java
Additional comments: 6
bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/CMakeMainTab2.java (6)
41-44: The new imports
IProgressMonitor,IStatus,Status, andJobare introduced to handle the scheduling of a job that reverts the target selection when the "Cancel" button is pressed. This is a part of the new functionality added to handle the target selection in the launch bar.88-88: The constant
JOB_DELAY_MSis introduced to specify the delay before the job that reverts the target selection is scheduled. This delay allows other operations to complete before the job is run.121-125: The
addDisposeListenermethod is modified to schedule a job that reverts the target selection when the parent composite is disposed. This is done to ensure that the target selection is reverted when the "Cancel" button is pressed. ThescheduleRevertTargetJobmethod is called with the current target name as an argument.568-573: The
doSavemethod is now called within the try block, which means that anyCoreExceptionthrown by this method will be caught and logged. This is a good practice as it ensures that exceptions are properly handled and do not cause the program to crash.713-718: The code for updating the launch configuration with the selected target has been removed from the if block. Instead, the
updateLaunchBarmethod is called directly. This change simplifies the code and removes the need to handle aCoreExceptionhere.860-884: The
scheduleRevertTargetJobmethod is introduced to schedule a job that reverts the target selection. The job checks if there is an active launch configuration and if so, it retrieves the target name from it. It then gets the launch target manager service and finds the launch target with the retrieved target name. Finally, it sets the active launch target to the found target. If anyCoreExceptionis thrown during this process, it is caught and logged, and the job is cancelled.
|
Hi @AndriiFilippov, thanks for noticing. Fixed |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabDebugger.java (45 hunks)
- bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/CMakeMainTab2.java (6 hunks)
Additional comments: 15
bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabDebugger.java (9)
29-39: New imports have been added. Ensure that these new dependencies are properly managed and do not conflict with existing ones.
103-106: New constants have been defined. Ensure that these constants are used appropriately throughout the code.
178-189: The
createControl()method has been modified to setLaunchBarListenerto ignore JTAG target changes and schedule the revert target job on dispose. Ensure that this change does not affect the functionality of the application.259-291: A new method
scheduleRevertTargetJob()has been added. This method schedules a job for reverting the target change in the launch bar when the Cancel button is pressed. Ensure that this method is called appropriately and that it functions as expected.506-511: The new hunk has removed some code related to updating the launch configuration when the selected item is not equal to the updated selected target. Ensure that this removal doesn't affect the functionality of the application.
1025-1027: The
updateGdbServerActualPath()method has been added. This method updates the GDB server actual path. Ensure that this method is called appropriately and that it functions as expected.1092-1094: The
initializeFrom()method has been modified to set thefConfigurationfield and print debug statements. Ensure that this change does not affect the functionality of the application.1340-1358: The
isValid()method has been modified to check for empty fields and set error messages. This is a good practice as it helps in validating the input and providing feedback to the user.1552-1554: The
performApply()method has been modified to save the JTAG flash voltage, target, and board attributes. Ensure that these attributes are used appropriately throughout the code.bundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/CMakeMainTab2.java (6)
41-44: New imports have been added to handle asynchronous jobs and monitor their progress. This is likely related to the new
scheduleRevertTargetJob()method.88-88: A new constant
JOB_DELAY_MShas been added. This constant is used to schedule the delay for therevertTargetJob.121-124: The
createControl()method now schedules a job to revert the active launch target when the parent composite is disposed. This ensures that the target is reverted back to its original state when the user exits the tab.567-572: The saving of flash arguments has been removed from the
performApply()method. This change simplifies the code and removes the need to handle aCoreExceptionhere.714-717: The
widgetSelected()method has been updated to callupdateLaunchBar()and update the board configurations when the selected target changes. This ensures that the launch bar and board configurations are always in sync with the selected target.859-886: A new method
scheduleRevertTargetJob()has been added. This method schedules a job to revert the active launch target. The job is scheduled with a delay specified byJOB_DELAY_MS. If the active launch configuration is null or an exception occurs, the job is cancelled. Otherwise, the active launch target is set to the selected target and the job completes successfully.
|
Hi @sigmaaa ! please, check this scenario: create project -> go to "New Launch Configuration" -> select new config: (Run + ESP-IDF Application) -> click "Next" -> click "Cancel" -> the Launch Mode and Launch Target disappear. |
Description
The initial bug can be reproduced this way: select board in the debug/jtag tab -> click cancel -> select the previous target in the launch bar -> open the debug tab again -> board doesn't match the target. This happens because we had to save the target attribute of the configuration not only when we finish the dialog, but in the moment when the target is changed. We had to do this because we want the SVD path to always be according to the selected target. However, it led us to the problem when we couldn't actually add a custom SVD path, because it will be always reinitialized after the configuration is reopened. Moreover, it saves the wrong value for the SVD path, if the SVD path is not opened before the finish dialog.
To address this issue, I made the following changes: I removed the saving configuration in the listeners (which also ensures proper operation cancellation), and I introduced a dynamic variable for the SVD path. This variable will be replaced during the debug phase to resolve the problem
Fixes # (IEP-1000)
Fixes #(IEP-1004)
Type of change
Please delete options that are not relevant.
How has this been tested?
Test 1:
Test 2:
values should be correct
Test 3:
Test Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit
New Features:
Refactor:
CMakeMainTab2class by removing unnecessary code blocks and introducing more efficient methods.TabDebuggerclass with updates to UI elements, tooltips, and methods, enhancing the user interface's intuitiveness.Note: The remaining changes do not have a direct impact on end-users and are categorized as "Refactor" or "Chores."