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

Restructure programmers by removing hardcoded build.openocdscript #636

Merged
merged 1 commit into from
Oct 20, 2021

Conversation

facchinm
Copy link
Member

This allows selecting multiple programmers; J-Link added as an example.

@ubidefeo

This allows selecting multiple programmers; J-Link added as an example.
@facchinm facchinm requested review from cmaglie and rsora June 23, 2021 15:03
@ArduinoBot
Copy link

✅ Build completed.

⬇️ Build URL: http://downloads.arduino.cc/PR/samd/package_samd-b213_index.json

ℹ️ To test this build:

  1. Open the Preferences of the Arduino IDE.
  2. Add the Build URL above in the Additional Boards Manager URLs field, and click OK.
  3. Open the Boards Manager (menu Tools->Board->Board Manager...)
  4. Install Arduino SAMD core - Pull Request Restructure programmers by removing hardcoded build.openocdscript #636
  5. Select one of the boards under SAMD Pull Request Restructure programmers by removing hardcoded build.openocdscript #636 in Tools->Board menu
  6. Compile/Upload as usual

@@ -198,22 +198,22 @@ tools.openocd.cmd.windows=bin/openocd.exe

tools.openocd.upload.params.verbose=-d2
tools.openocd.upload.params.quiet=-d0
tools.openocd.upload.pattern="{path}/{cmd}" {upload.verbose} -s "{path}/share/openocd/scripts/" -f "{runtime.platform.path}/variants/{build.variant}/{build.openocdscript}" -c "telnet_port disabled; program {{build.path}/{build.project_name}.bin} verify reset 0x2000; shutdown"
tools.openocd.upload.pattern="{path}/{cmd}" {upload.verbose} -s "{path}/share/openocd/scripts/" -f "interface/{protocol}" -c "set telnet_port 0" {extra_params} -f "target/at91samdXX.cfg" -c "telnet_port disabled; program {{build.path}/{build.project_name}.bin} verify reset 0x2000; shutdown"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't it weird that this hardcodes the target configuration? Sounds like that should be board-configurable? Or is the samd core specific enough that all samd/samd21 boards it supports use the same openocd target?

Copy link
Member Author

Choose a reason for hiding this comment

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

In fact, it may sound strange but all samd{11,21,51} are supported by the same configuration 🙂 . It doesn't apply to SAMC and SAML though (I didn't check if derived cores targeting these mcus exist). Anyway, moving to a property in boards.txt wouldn't hurt but could in fact break backwards compatibility with existing derived cores (due to the missing entry)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Couldn't you define a default platform-wide and have that (optionally) overridden by boards.txt? Not entirely sure if that's possible, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll test it and report back 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

Checked and unfortunately the boards.txt entry doesn't override the default one 😞
Example patch (should fail but it doesn't)

diff --git a/boards.txt b/boards.txt
index 91adede0..2ebf09a9 100644
--- a/boards.txt
+++ b/boards.txt
@@ -111,6 +111,7 @@ mkr1000.build.vid=0x2341
mkr1000.build.pid=0x804e
mkr1000.bootloader.tool=openocd
mkr1000.bootloader.file=mkr1000/samd21_sam_ba_arduino_mkr1000.bin
+mkr1000.target=at91samdXXX.cfg

# Arduino MKRZero
# ---------------
diff --git a/platform.txt b/platform.txt
index 3df9b1ab..2bc409a4 100644
--- a/platform.txt
+++ b/platform.txt
@@ -192,20 +192,23 @@ tools.bossacI.upload.network_pattern="{network_cmd}" -address {serial.port} -por
# OpenOCD sketch upload
#

+# Can be overridden by boards.txt
+tools.openocd.target=at91samdXX.cfg
+
tools.openocd.path={runtime.tools.openocd-0.10.0-arduino7.path}
tools.openocd.cmd=bin/openocd
tools.openocd.cmd.windows=bin/openocd.exe

tools.openocd.upload.params.verbose=-d2
tools.openocd.upload.params.quiet=-d0
-tools.openocd.upload.pattern="{path}/{cmd}" {upload.verbose} -s "{path}/share/openocd/scripts/" -f "interface/{protocol}" -c "set telnet_port 0" {extra_params} -f "target/at91samdXX.cfg" -c "telnet_port disabled; program {{build.path}/{build.project_name}.bin} verify reset 0x2000; shutdown"
+tools.openocd.upload.pattern="{path}/{cmd}" {upload.verbose} -s "{path}/share/openocd/scripts/" -f "interface/{protocol}" -c "set telnet_port 0" {extra_params} -f "target/{target}" -c "telnet_port disabled; program {{build.path}/{build.project_name}.bin} verify reset 0x2000; shutdown"

tools.openocd.network_cmd={runtime.tools.arduinoOTA.path}/bin/arduinoOTA
tools.openocd.upload.network_pattern={network_cmd} -address {serial.port} -port 65280 -username arduino -password "{network.password}" -sketch "{build.path}/{build.project_name}.bin" -upload /sketch -b

tools.openocd.program.params.verbose=-d2
tools.openocd.program.params.quiet=-d0
-tools.openocd.program.pattern="{path}/{cmd}" {program.verbose} -s "{path}/share/openocd/scripts/" -f "interface/{protocol}" -c "set telnet_port 0" {extra_params} -f "target/at91samdXX.cfg" -c "telnet_port disabled; program {{build.path}/{build.project_name}.hex} verify reset; shutdown"
+tools.openocd.program.pattern="{path}/{cmd}" {program.verbose} -s "{path}/share/openocd/scripts/" -f "interface/{protocol}" -c "set telnet_port 0" {extra_params} -f "target/{target}" -c "telnet_port disabled; program {{build.path}/{build.project_name}.hex} verify reset; shutdown"

tools.openocd.erase.params.verbose=-d3
tools.openocd.erase.params.quiet=-d0
@@ -213,7 +216,7 @@ tools.openocd.erase.pattern=

tools.openocd.bootloader.params.verbose=-d2
tools.openocd.bootloader.params.quiet=-d0
-tools.openocd.bootloader.pattern="{path}/{cmd}" {bootloader.verbose} -s "{path}/share/openocd/scripts/" -f "interface/{protocol}" -c "set telnet_port 0" {extra_params} -f "target/at91samdXX.cfg" -c "telnet_port disabled; init; halt; at91samd bootloader 0; program {{runtime.platform.path}/bootloaders/{bootloader.file}} verify reset; shutdown"
+tools.openocd.bootloader.pattern="{path}/{cmd}" {bootloader.verbose} -s "{path}/share/openocd/scripts/" -f "interface/{protocol}" -c "set telnet_port 0" {extra_params} -f "target/{target}" -c "telnet_port disabled; init; halt; at91samd bootloader 0; program {{runtime.platform.path}/bootloaders/{bootloader.file}} verify reset; shutdown"

#
# OpenOCD sketch upload - version with configurable bootloader size


tools.openocd.network_cmd={runtime.tools.arduinoOTA.path}/bin/arduinoOTA
tools.openocd.upload.network_pattern={network_cmd} -address {serial.port} -port 65280 -username arduino -password "{network.password}" -sketch "{build.path}/{build.project_name}.bin" -upload /sketch -b

tools.openocd.program.params.verbose=-d2
tools.openocd.program.params.quiet=-d0
tools.openocd.program.pattern="{path}/{cmd}" {program.verbose} -s "{path}/share/openocd/scripts/" -f "{runtime.platform.path}/variants/{build.variant}/{build.openocdscript}" -c "telnet_port disabled; program {{build.path}/{build.project_name}.hex} verify reset; shutdown"
tools.openocd.program.pattern="{path}/{cmd}" {program.verbose} -s "{path}/share/openocd/scripts/" -f "interface/{protocol}" -c "set telnet_port 0" {extra_params} -f "target/at91samdXX.cfg" -c "telnet_port disabled; program {{build.path}/{build.project_name}.hex} verify reset; shutdown"
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the deal with telnet_port disabled and set telnet_port 0 here? Seems duplicate?

Copy link
Member Author

Choose a reason for hiding this comment

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

set telnet_port 0 was hidden in the variant configuration; it was necessary alongside telnet_port disabled due to this issue arduino/Arduino#4330 , so I'd leave it there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm, I'm a bit confused about that issue you linked. Sounds like that issue is really only about gdb_port not accepting disabled (but that was since fixed in openocd upstream, not sure if the Arduino version already updated to include that fix)? And it seems that disabled is the right value for all openocd versions (using 0 would, AFAICS, in older version be used as disabled, but in newer versions cause a random port to be used).

So are you sure that set telnet_port 0 is needed? I would think it never was really required, even?

Also, I can imagine that tcl_port disabled and (if openocd is sufficiently new) gdb_port disabled would make sense here as well? Or are we actually using the gdb_port over TCP now (as opposed to using it over stdin/stdout)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure it's still needed, the best way to check this would be trying to reproduce the issue but it's a bit impractical. About gdb_port disabled, I need to check how the debugging bridge is implemented in IDE 2.0 ; is it's over stdio we can safely add it.

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.

3 participants