Skip to content

Use debug "configurations" instead or "recipes" #1033

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

Merged
merged 11 commits into from
Nov 3, 2020

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Oct 16, 2020

Added feature: show debug configuration info with arduino-cli debug -I ...
Added feature: debug programmer selection with arduino-cli debug -P ...

⚠️ Breaking change: this commit changes the way debug support must be provided by platforms.

Previously it was required:

  • To provide a debug command line recipe in platform.txt like tools.reciped-id.debug.pattern=..... that will start a gdb session for the selected board.
  • to add a debug.tool definition in the boards.txt to recall that recipe, for example myboard.debug.tool=recipe-id

Now:

  • only the configuration needs to be supplied, the arduino-cli or the GUI tool will figure out how to call and setup the gdb session. An example of configuration is the following:
debug.executable={build.path}/{build.project_name}.elf
debug.toolchain=gcc
debug.toolchain.path={runtime.tools.arm-none-eabi-gcc-7-2017q4.path}/bin/
debug.toolchain.prefix=arm-none-eabi-
debug.server=openocd
debug.server.openocd.path={runtime.tools.openocd-0.10.0-arduino7.path}/bin/
debug.server.openocd.scripts_dir={runtime.tools.openocd-0.10.0-arduino7.path}/share/openocd/scripts/
debug.server.openocd.script={runtime.platform.path}/variants/{build.variant}/{build.openocdscript}

the debug.server.XXXX subkeys are optional and also "free text", this means that the configuration may be extended as needed by the specific server.
For now only openocd is supported. Anyway, if this change works, any other kind of server may be fairly easily added.

The debug.xxx=yyy definitions above may be supplied and overlayed in the usual ways:

  • on platform.txt: definition here will be shared through all boards in the platform
  • on boards.txt as part of a board definition: they will override the global platform definitions
  • on programmers.txt: they will override the boards and global platform definitions if the programmer is selected

/cc @kittaakos

@matthijskooijman
Copy link
Collaborator

Hm, this seems to cause quite a bit of command options hardcoding in arduino-cli, is this really what we want? Some hardcoding might be appropriate (since arduino-cli already assumes it's talking to gdb), but I wonder if there should be some balance (basic recipe in the platform, maybe some add-on options hardcoded in arduino-cli? Or maybe no hardcoding at all, since a platform might have something that is gdb-compatible (talks the same protocol, but has different commandline options), or a different version of gdb that needs slightly different options?

Can you comment on what this change gains?

Looking at the proposal itself, why configure debug.toolchain.path and debug.toolchain.prefix=arm-none-eabi- and not just the full path to gdb directly? It seems the gdb is the only tool used here? If future toolchains need multiple commands, they can just be specified explicitly?

The debug.xxx=yyy definitions above may be supplied and overlayed in the usual ways

This seems like a good improvement, that the debug config can be specified by the board (for builtin debuggers) or by programmer tools.

@cmaglie
Copy link
Member Author

cmaglie commented Oct 16, 2020

Can you comment on what this change gains?

This comes from our IDE-integration experience:

  • wrt IDE: it's more difficult to attach an already initiated gdb session than providing a configuration and letting the IDE figure out how to run it. This is true even if we have complete control over the stdin/out of the GDB client, many debugging GUI did not even have support for attaching to an externally started "streaming" GDB client.
  • the gbd_pipe seems to not be very well supported, we had some troubles especially when the debugger is running and you want to "stop" it: the most reliable and tested way seems to be to run the debug server via TCP. The gdb_pipe really looks like the "ugly-brother" in this case.
  • fitting all the config options in a single recipe turns out to be fragile, there is a nested quoting required in the gdb' target extended-remote | .... because it's a whole command line fitted in a single parameter: this requires very special treatment on the debug recipe (for example this hack to run it correctly on windows). Yes this is possible but not nice...

So after an internal discussion, and since this feature is still very in alpha, we decided to try this other approach. BTW it may not be the best possible, we are still iterating on this one...

Looking at the proposal itself, why configure debug.toolchain.path and debug.toolchain.prefix=arm-none-eabi- and not just the full path to gdb directly? It seems the gdb is the only tool used here? If future toolchains need multiple commands, they can just be specified explicitly?

I know that gdb and objdump are used (and maybe also others). Since they are part of the same toolchain I prefer to define toolchainPrefix once for all, so we don't need to add more option if we later find that another tool is required.

@kittaakos
Copy link
Contributor

% ./arduino-cli debug -I -p /dev/cu.usbmodem14602 -b arduino:samd:arduino_zero_edbg ~/Documents/Arduino/Blink --format json
{
  "executable": "/Users/akos.kitta/Documents/Arduino/Blink/build/arduino.samd.arduino_zero_edbg/Blink.ino.elf",
  "toolchain": "gcc",
  "toolchain_path": "/Users/akos.kitta/Library/Arduino15/packages/arduino/tools/arm-none-eabi-gcc/7-2017q4/bin/",
  "toolchain_prefix": "arm-none-eabi-",
  "server": "openocd",
  "server_path": "/Users/akos.kitta/Library/Arduino15/packages/arduino/tools/openocd/0.10.0-arduino7/bin/",
  "server_configuration": {
    "path": "/Users/akos.kitta/Library/Arduino15/packages/arduino/tools/openocd/0.10.0-arduino7/bin/",
    "script": "/Users/akos.kitta/Library/Arduino15/packages/arduino/hardware/samd/1.8.9/variants/arduino_zero/openocd_scripts/arduino_zero.cfg",
    "scripts_dir": "/Users/akos.kitta/Library/Arduino15/packages/arduino/tools/openocd/0.10.0-arduino7/share/openocd/scripts/"
  }
}

For consumers, it would be more convenient, if the server_path would point to the executable and not to the container bin directory.

Instead of /Users/akos.kitta/Library/Arduino15/packages/arduino/tools/openocd/0.10.0-arduino7/bin/, I would like to get /Users/akos.kitta/Library/Arduino15/packages/arduino/tools/openocd/0.10.0-arduino7/bin/openocd. Is it possible?

As a workaround, I can do path.join(server_path, server). (Assuming the server type will be always the same as the name of the executable, which might not be always true.)

@kittaakos
Copy link
Contributor

I verified the functionality. I could integrate the new CLI changes into IDE2; the debugger works very well! 👏 I tried with my Zero boards from both VS Code and IDE2.

@kittaakos
Copy link
Contributor

I tried with my Zero boards from both VS Code and IDE2.

For the record: I tried it on macOS only by building things locally without the pipeline.

@matthijskooijman
Copy link
Collaborator

@cmaglie thanks for clarifying. I still think this approach has downsides, but I can see the pragmatism of it, so let's see where this leads :-)

Copy link
Contributor

@rsora rsora left a comment

Choose a reason for hiding this comment

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

👍

@cmaglie cmaglie merged commit 1d49b6a into arduino:master Nov 3, 2020
@cmaglie cmaglie deleted the debug_metadata branch November 3, 2020 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants