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: PyOCD: Honor OUTPUT_EXT #329

Closed
wants to merge 1 commit into from

Conversation

ccli8
Copy link

@ccli8 ccli8 commented Aug 23, 2024

Summary of changes

This PR tries to honor OUTPUT_EXT defined in targets.json5 for PYOCD upload method flash. If it is defined, use image file with the specified extension .<OUTPUT_EXT> for flash, or default to .bin.

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)

@multiplemonomials
Copy link
Collaborator

multiplemonomials commented Aug 23, 2024

Hmm... I assume that this MR is being submitted because there's a target using .hex only as the output extension, and that's breaking the upload methods that use the BIN_FILE argument. And the fact is, this is very easy to do when writing an upload method, because gen_upload_target() accepts the bin file in the parameters list.

What if we instead change the signature of gen_upload_target to something like

function(gen_upload_target TARGET_NAME BINARY_FILE)

where BINARY_FILE was the path to the bin or the hex file, whichever exists?

this function would be changed to something like

# Generate an upload target for an executable
function(mbed_generate_upload_target target)
    if ("${MBED_OUTPUT_EXT}" STREQUAL "" OR MBED_OUTPUT_EXT STREQUAL "bin")
        gen_upload_target(${target} ${CMAKE_CURRENT_BINARY_DIR}/$<TARGET_FILE_BASE_NAME:${target}>.bin)
    else()
        gen_upload_target(${target} ${CMAKE_CURRENT_BINARY_DIR}/$<TARGET_FILE_BASE_NAME:${target}>.hex)
    endif()
endfunction()

Then we would refactor all the upload methods to be able to handle a bin or hex file, and the logic in them could be a lot simpler.

@ccli8 ccli8 mentioned this pull request Aug 23, 2024
@ccli8
Copy link
Author

ccli8 commented Aug 23, 2024

Create #331 to apply above suggestion

In PyOCD flash, if OUTPUT_EXT is defined in targets.json5, use image
file with this extension <target>.<OUTPUT_EXT> for flash, or default
to <target>.bin.
@ccli8 ccli8 force-pushed the upload_pyocd_honor_output_ext branch from d01e32b to fbf692a Compare August 23, 2024 08:44
@multiplemonomials
Copy link
Collaborator

Closing this now since #331 is merged

@ccli8 ccli8 deleted the upload_pyocd_honor_output_ext branch August 26, 2024 06:57
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