Skip to content
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

Debugger now allows generic selection of sub-configurations #2435

Merged
merged 3 commits into from
Nov 24, 2023

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Nov 23, 2023

Please check if the PR fulfills these requirements

See how to contribute

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)
  • configuration.schema.json updated if new parameters are added.

What kind of change does this PR introduce?

It is now possible to use any sub-tree of the platform configuration to override the debugger configuration using the directive debug.additional_config=CONF_PREFIX.

This change allows a more convenient rationalization and selection of the configs to apply. For example, we could factor common parts of a configuration in the platform.txt file:

# CONFIG 1
debug-overrides.esp32.cortex-debug.custom.name=Arduino on ESP32
debug-overrides.esp32.cortex-debug.custom.request=attach
debug-overrides.esp32.cortex-debug.custom.postAttachCommands.0=set remote hardware-watchpoint-limit 2
debug-overrides.esp32.cortex-debug.custom.postAttachCommands.1=monitor reset halt
debug-overrides.esp32.cortex-debug.custom.postAttachCommands.2=monitor gdb_sync
debug-overrides.esp32.cortex-debug.custom.postAttachCommands.3=thb setup
debug-overrides.esp32.cortex-debug.custom.postAttachCommands.4=c
debug-overrides.esp32.cortex-debug.custom.overrideRestartCommands.0=monitor reset halt
debug-overrides.esp32.cortex-debug.custom.overrideRestartCommands.1=monitor gdb_sync
debug-overrides.esp32.cortex-debug.custom.overrideRestartCommands.2=thb setup
debug-overrides.esp32.cortex-debug.custom.overrideRestartCommands.3=c

# CONFIG 2
debug-overrides.esp32s2.cortex-debug.custom.name=Arduino on ESP32-S2
debug-overrides.esp32s2.cortex-debug.custom.request=attach
debug-overrides.esp32s2.cortex-debug.custom.postAttachCommands.0=set remote hardware-watchpoint-limit 2
debug-overrides.esp32s2.cortex-debug.custom.postAttachCommands.1=monitor reset halt
debug-overrides.esp32s2.cortex-debug.custom.postAttachCommands.2=monitor gdb_sync
debug-overrides.esp32s2.cortex-debug.custom.postAttachCommands.3=thb setup
debug-overrides.esp32s2.cortex-debug.custom.postAttachCommands.4=c
debug-overrides.esp32s2.cortex-debug.custom.overrideRestartCommands.0=monitor reset halt
debug-overrides.esp32s2.cortex-debug.custom.overrideRestartCommands.1=monitor gdb_sync
debug-overrides.esp32s2.cortex-debug.custom.overrideRestartCommands.2=thb setup
debug-overrides.esp32s2.cortex-debug.custom.overrideRestartCommands.3=c

and choose to add the right configuration depending on the board in the boards.txt file:

myboard.name=My Board with esp32
myboard.debug.additional_config=debug-overrides.esp32
...

anotherboard.name=My Board with esp32s2
anotherboard.debug.additional_config=debug-overrides.esp32s2
...

Another possibility is to compose the configuration using another variable present in the board configuration, for example if in the platform.txt we add:

debug.additional_config=debug-overrides.{build.mcu}

we may use the build.mcu value as a "selector" for the board-specific debug configuration that is added to the global debug configuration:

myboard.name=My Board with esp32
myboard.build.mcu=esp32
...

anotherboard.name=My Board with esp32s2
anotherboard.build.mcu=esp32s2
...

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change, and is titled accordingly?

Other information

@cmaglie cmaglie added this to the Arduino CLI v0.35.0 milestone Nov 23, 2023
@cmaglie cmaglie self-assigned this Nov 23, 2023
Copy link

codecov bot commented Nov 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b41f404) 67.28% compared to head (03d5ae5) 67.38%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2435      +/-   ##
==========================================
+ Coverage   67.28%   67.38%   +0.10%     
==========================================
  Files         210      210              
  Lines       20576    20574       -2     
==========================================
+ Hits        13844    13864      +20     
+ Misses       5593     5575      -18     
+ Partials     1139     1135       -4     
Flag Coverage Δ
unit 67.38% <100.00%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -169,12 +175,10 @@ func getDebugProperties(req *rpc.GetDebugConfigRequest, pme *packagemanager.Expl
}
}

toolchainPrefix := debugProperties.Get("toolchain.prefix")
// HOTFIX: for samd (and maybe some other platforms). We should keep this for a reasonable
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also for other cores like the esp32 one

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

Copy link
Contributor

Choose a reason for hiding this comment

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

So I'd change the comment

Copy link
Member Author

Choose a reason for hiding this comment

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

The comment says:

for samd (and maybe some other platforms)

esp32 is some other platform :-)

@cmaglie cmaglie marked this pull request as ready for review November 24, 2023 10:55
@cmaglie cmaglie added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Nov 24, 2023
@cmaglie cmaglie merged commit a4e7eb4 into arduino:master Nov 24, 2023
100 checks passed
@cmaglie cmaglie deleted the debugger_improvements branch November 24, 2023 14:48
cmaglie added a commit to cmaglie/arduino-cli that referenced this pull request Dec 20, 2023
…2435)

* debug: extend hotfix for trailing dash '-' to all toolchains

* Added additional_config selector for debug configuration

* Added docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants