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

Upload: NUVOTON: Fix GDB restart command for pyOCD #349

Closed

Conversation

ccli8
Copy link

@ccli8 ccli8 commented Sep 19, 2024

Summary of changes

This PR includes two parts:

  1. Give chance to adjust GDB commands MBED_UPLOAD_LAUNCH_COMMANDS or MBED_UPLOAD_RESTART_COMMANDS for debug launch by implementing mbed_adjust_upload_debug_commands cmake function.
  2. Based on above, for Nuvoton targets, fix GDB restart command for pyOCD.

Pull request type

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

This gives chance to adjust GDB commands MBED_UPLOAD_LAUNCH_COMMANDS
or MBED_UPLOAD_RESTART_COMMANDS for debug launch by implementing
mbed_adjust_upload_debug_commands cmake function.
PyOCD debug restart is failed without this somehow. It appears the device
under debug must be halted after MBED_UPLOAD_RESTART_COMMANDS, but this is
broken. Change to "monitor reset halt" to fix it.
function(mbed_upload_nuvoton_adjust_restart_command_for_pyocd target)
if(DEFINED MBED_UPLOAD_RESTART_COMMANDS AND
"${UPLOAD_METHOD}" STREQUAL "PYOCD")
set(MBED_UPLOAD_RESTART_COMMANDS "monitor reset halt" CACHE INTERNAL "" FORCE)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a good catch, but any reason not to adjust line 62 of UploadMethodPYOCD.cmake to just have monitor reset halt? That way it's fixed for all targets.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I tested it locally with STM32H5 and it seems to work (though to be honest I have never really used the restart functionality and I'm not entirely sure how it's supposed to be different from reset)

Copy link
Author

Choose a reason for hiding this comment

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

Not sure the restart command failure will happen on other targets. Anyway, I create another PR #350 to apply the change to all targets.

@multiplemonomials
Copy link
Collaborator

Closing since #350 is merged!

@ccli8 ccli8 deleted the nvt_upload_fix_gdb_restart_for_pyocd branch September 23, 2024 01:13
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.

2 participants