IEP-1756 Deprecated! use 'adapter usb location <location>' command instead of OPENOCD_USB_ADAPTER_LOCATION variable#1450
Merged
kolipakakondal merged 2 commits intomasterfrom May 6, 2026
Merged
Conversation
📝 WalkthroughWalkthroughA new OpenOcdVersionManager utility centralizes OpenOCD version retrieval/caching and parsing; Configuration and GdbServerBackend now consult it to decide runtime command/environment adjustments (e.g., adapter USB location) instead of spawning inline version-check processes. Changes
Sequence Diagram(s)sequenceDiagram
participant Config as Configuration
participant Backend as GdbServerBackend
participant Manager as OpenOcdVersionManager
participant OpenOCD as OpenOCD executable
Config->>Backend: prepare launch args
Backend->>Manager: getVersion(openocdPath)
Manager->>OpenOCD: run "openocd --version" (stderr->stdout, 2s timeout)
OpenOCD-->>Manager: version output
Manager-->>Backend: OpenOcdVersion (cached)
Backend->>Config: capability flags (isAtLeast / isBuildDateAtLeast)
Config->>Backend: finalize args/env (inject -c adapter usb location or OPENOCD_USB_LOCATION as needed)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/Configuration.java (1)
349-357: Refactor opportunity: avoid duplicating version/build-date thresholds.Consider moving these thresholds into
OpenOcdVersionManageras a named capability API so both call sites stay consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/Configuration.java` around lines 349 - 357, Extract the duplicated version/build-date thresholds into named capability methods on OpenOcdVersionManager and call those from Configuration: replace the inline checks in supportsAdapterUsbCommand(String executablePath) and useModernPortSyntax(String executablePath) with calls like OpenOcdVersionManager.supportsAdapterUsbCommand(executablePath) and OpenOcdVersionManager.useModernPortSyntax(executablePath) (or similarly named methods) so the version logic (e.g., isBuildDateAtLeast(0,12,20260424) and isAtLeast(0,12)) lives only in OpenOcdVersionManager and both call sites delegate to those new capability APIs for consistency.bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/dsf/GdbServerBackend.java (1)
247-250: Consider centralizing the adapter-support threshold in one place.This literal threshold is duplicated across classes. Expose a single capability helper (or shared constant) to prevent future divergence.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/dsf/GdbServerBackend.java` around lines 247 - 250, The method supportsAdapterUsbCommand currently embeds a hard-coded threshold (0,12,20260424); centralize this by introducing a single shared capability helper or constant (e.g., OpenOcdCapabilities.ADAPTER_USB_COMMAND_THRESHOLD or a helper method OpenOcdVersionManager.supportsAdapterUsbCommand(executablePath)) and replace the literal check in supportsAdapterUsbCommand with a call/reference to that shared symbol so all classes use the same source of truth; update other duplicates to use the same new constant/helper and keep the version comparison logic inside OpenOcdVersionManager (or the new helper) to avoid duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/dsf/GdbServerBackend.java`:
- Around line 230-234: The code currently sets
IDFLaunchConstants.OPENOCD_USB_LOCATION when
supportsAdapterUsbCommand(commandLineArray[0]) returns true, which is reversed;
change the condition so OPENOCD_USB_LOCATION is set only when openocdLoc != null
&& commandLineArray != null && commandLineArray.length > 0 &&
!supportsAdapterUsbCommand(commandLineArray[0]) (i.e., for legacy OpenOCD that
does NOT support adapter usb location), keeping the same envMap.put call and
using the existing variables openocdLoc, commandLineArray and
supportsAdapterUsbCommand to locate the fix.
---
Nitpick comments:
In
`@bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/Configuration.java`:
- Around line 349-357: Extract the duplicated version/build-date thresholds into
named capability methods on OpenOcdVersionManager and call those from
Configuration: replace the inline checks in supportsAdapterUsbCommand(String
executablePath) and useModernPortSyntax(String executablePath) with calls like
OpenOcdVersionManager.supportsAdapterUsbCommand(executablePath) and
OpenOcdVersionManager.useModernPortSyntax(executablePath) (or similarly named
methods) so the version logic (e.g., isBuildDateAtLeast(0,12,20260424) and
isAtLeast(0,12)) lives only in OpenOcdVersionManager and both call sites
delegate to those new capability APIs for consistency.
In
`@bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/dsf/GdbServerBackend.java`:
- Around line 247-250: The method supportsAdapterUsbCommand currently embeds a
hard-coded threshold (0,12,20260424); centralize this by introducing a single
shared capability helper or constant (e.g.,
OpenOcdCapabilities.ADAPTER_USB_COMMAND_THRESHOLD or a helper method
OpenOcdVersionManager.supportsAdapterUsbCommand(executablePath)) and replace the
literal check in supportsAdapterUsbCommand with a call/reference to that shared
symbol so all classes use the same source of truth; update other duplicates to
use the same new constant/helper and keep the version comparison logic inside
OpenOcdVersionManager (or the new helper) to avoid duplication.
🪄 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: 3753ca32-b686-43dd-9c13-0ab4161939e4
📒 Files selected for processing (4)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/OpenOcdVersionManager.javabundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/Configuration.javabundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/dsf/GdbServerBackend.javatests/com.espressif.idf.core.test/src/com/espressif/idf/core/util/test/OpenOcdVersionManagerTest.java
There was a problem hiding this comment.
🧹 Nitpick comments (1)
bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/dsf/GdbServerBackend.java (1)
247-250: ⚡ Quick winDeduplicate the adapter USB support threshold to avoid drift.
This method hardcodes
0, 12, 20260424, and the same threshold exists inbundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/Configuration.java:349-352. Please centralize this in one shared API/constant (ideally inOpenOcdVersionManager) so command-line injection and env-var fallback can’t diverge later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/dsf/GdbServerBackend.java` around lines 247 - 250, The hardcoded version threshold (0, 12, 20260424) must be centralized to avoid drift: add a single public constant or helper in OpenOcdVersionManager (e.g. ADAPTER_USB_SUPPORT_MAJOR/MINOR/BUILDDATE or a method isAdapterUsbSupported(executablePath)) and update GdbServerBackend.supportsAdapterUsbCommand and the check in Configuration to call that new constant/method instead of repeating the triple (0,12,20260424); ensure the behavior still uses OpenOcdVersionManager.getVersion(...).isBuildDateAtLeast(...) under the new centralized symbol.
🤖 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.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/dsf/GdbServerBackend.java`:
- Around line 247-250: The hardcoded version threshold (0, 12, 20260424) must be
centralized to avoid drift: add a single public constant or helper in
OpenOcdVersionManager (e.g. ADAPTER_USB_SUPPORT_MAJOR/MINOR/BUILDDATE or a
method isAdapterUsbSupported(executablePath)) and update
GdbServerBackend.supportsAdapterUsbCommand and the check in Configuration to
call that new constant/method instead of repeating the triple (0,12,20260424);
ensure the behavior still uses
OpenOcdVersionManager.getVersion(...).isBuildDateAtLeast(...) under the new
centralized symbol.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 685a4e7f-204b-463a-9785-2d010c38a29e
📒 Files selected for processing (1)
bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/dsf/GdbServerBackend.java
AndriiFilippov
approved these changes
May 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Created OpenOcdVersionManager to optimize the usage of the openocd --version command. It is now used to determine whether the adapted USB location command should be used instead of the OPENOCD_USB_ADAPTER_LOCATION variable.
Fixes # (IEP-1756)
Type of change
Please delete options that are not relevant.
How has this been tested?
Download the latest openocd https://github.com/espressif/openocd-esp32/releases/tag/v0.12.0-esp32-20260424 and test it. There should no
Deprecated! use 'adapter usb location <location>' command instead of OPENOCD_USB_ADAPTER_LOCATION variablein the logsTest Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Tests