IEP-1565 ESP32P4: Debugging failure #1252
Conversation
WalkthroughThe updates include a modification to the classpath configuration by simplifying the entry for the "svd" directory, and an enhancement to the SvdPathResolver class to handle null SVD URLs by logging a message and returning an empty string early. Changes
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/SvdPathResolver.java (1)
63-73: Null-check is good; move it before the expensivejarPathresolutionYou now guard against a missing SVD, 👍.
Minor nit:jarPathis computed even whensvdUrlisnull, incurring an unnecessaryProtectionDomainlookup.-URL svdUrl = Platform.getBundle(Activator.PLUGIN_ID) - .getResource("svd/".concat(target.concat(".svd"))); //$NON-NLS-1$ //$NON-NLS-2$ -String jarPath = new File(TabSvdTarget.class.getProtectionDomain() - .getCodeSource().getLocation().toURI()).getPath(); - -if (svdUrl == null) -{ - Logger.log("svd file is missing"); //$NON-NLS-1$ - return StringUtil.EMPTY; -} +URL svdUrl = Platform.getBundle(Activator.PLUGIN_ID) + .getResource("svd/".concat(target).concat(".svd")); //$NON-NLS-1$ //$NON-NLS-2$ +if (svdUrl == null) +{ + Logger.log("svd file is missing for target: " + target); //$NON-NLS-1$ + return StringUtil.EMPTY; +} +String jarPath = new File(TabSvdTarget.class.getProtectionDomain() + .getCodeSource().getLocation().toURI()).getPath();Adds a tiny optimisation and a clearer log message.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bundles/com.espressif.idf.debug.gdbjtag.openocd/.classpath(1 hunks)bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/SvdPathResolver.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/SvdPathResolver.java (2)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/logging/Logger.java (1)
Logger(21-137)bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/StringUtil.java (1)
StringUtil(11-25)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build_macos
🔇 Additional comments (1)
bundles/com.espressif.idf.debug.gdbjtag.openocd/.classpath (1)
3-3: Trailing-slash looks fine, but double-check duplicate/legacy entriesSwitching from
path="svd"topath="svd/"is harmless and often clearer, yet Eclipse sometimes treats the two as distinct. Make sure no other<classpathentry … path="svd">survives in downstream .classpath files, otherwise both will be exported and the resource folder will be duplicated on the build path.
|
@AndriiFilippov once this is verified we can merge and close it |
kolipakakondal
left a comment
kolipakakondal
left a comment
There was a problem hiding this comment.
Hi @sigmaaa Thanks for adding a PR. Can we add the esp32p4 svd to the IDE local svd list?
Ah I see. You've already added. |
|
@AndriiFilippov please check |
|
@kolipakakondal hi ! Everything has been tested, and Denis has been informed about the results (issue still persists). After the work on this PR is completed, @alirana01 and I will focus on this PR because Denis does not have the latest version of the ESP32P4 board. logs: |
Hi @AndriiFilippov, @kolipakakondal The initial issue mentioned in the ticket — "Cannot invoke java.net.URL.getPath() because svdUrl is null" — has been resolved. The debugging issue is probably unrelated to this PR and our side because debugging for the ESP32-P4 was working previously, as shown in this issue. I don’t recall any changes on our side that could have affected ESP32-P4 debugging, so it’s likely the issue is not on our end. However, we are currently investigating. |
|
@sigmaaa hi ! "Cannot invoke java.net.URL.getPath() because svdUrl is null" — has been resolved ✅ |
Description
Added new SVD files and added a check to ensure SVDUrl is not null to avoid NPE in the future.
Fixes # (IEP-1565)
Type of change
Please delete options that are not relevant.
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 Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit