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.

Sign up for GitHub

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

Jump to bottom

matter/fast pair: Add support for merging hex files without partition manager #18792

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+155 −13
Open

matter/fast pair: Add support for merging hex files without partition manager #18792

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6c06685
cmake: sysbuild: suit: Add support for merging other files
nordicjm Nov 8, 2024
53b9449
cmake: sysbuild: fast_pair: add support for nRF54H20 non-PM
kapi-no Oct 14, 2024
c51efb6
samples: bluetooth: fast_pair: input_device: add support for nrf54h20dk
kapi-no Oct 17, 2024
7491f01
manifest: Update sdk-connectedhomeip
nordicjm Nov 8, 2024
2b3619b
sysbuild: Add support for matter non-PM factory data generation
nordicjm Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion cmake/sysbuild/suit.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,12 +534,18 @@ function(suit_setup_merge)
list(APPEND ARTIFACTS_TO_MERGE ${BINARY_DIR}/zephyr/suit_mpi_${IMAGE_TARGET_NAME}_merged.hex)
endif()

# Get a list of files (and their dependencies) which need merging into the uicr merged file and
# add them at the end of the list, allowing for overwriting
get_property(merge_files GLOBAL PROPERTY SUIT_MERGE_FILE)
get_property(merge_dependencies GLOBAL PROPERTY SUIT_MERGE_DEPENDENCIES)

set_property(
GLOBAL APPEND PROPERTY SUIT_POST_BUILD_COMMANDS
COMMAND ${PYTHON_EXECUTABLE} ${ZEPHYR_BASE}/scripts/build/mergehex.py
--overlap replace
-o ${OUTPUT_HEX_FILE}
${ARTIFACTS_TO_MERGE}
${ARTIFACTS_TO_MERGE} ${merge_files}
Copy link
Contributor

Choose a reason for hiding this comment

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

observation: this is really not self-explaining what the difference between ARTIFACTS_TO_MERGE and merge_files is.
But as this file in general is messy, then there is not change requested for this PR.

DEPENDS ${merge_dependencies}
# fixme: uicr_merged is overwritten by new content, runners_yaml_props_target could be used to define
# what shall be flashed, but not sure where to set this! Remove --overlap if ready!
# example usage: set_property(TARGET runners_yaml_props_target PROPERTY hex_file ${merged_hex_file})
Expand Down
17 changes: 17 additions & 0 deletions cmake/sysbuild/suit_utilities.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,20 @@ function(suit_create_cache_partition args output_file partition_num recovery)
"${output_file_name}type=cache;${output_file_name}partition=${partition_num};")
endif()
endfunction()

# Usage:
# suit_add_merge_hex_file(FILES <files> [DEPENDENCIES <dependencies>]
#
# Add files which should be merged into the uicr_merged.hex output file, respecting any
# dependencies that need to be generated before hand. This will overwrite existing data if it is
# present in other files
function(suit_add_merge_hex_file)
cmake_parse_arguments(VAR "" "" "FILES;DEPENDENCIES" ${ARGN})
Copy link
Contributor

Choose a reason for hiding this comment

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

better to start using arg as prefix.
See: https://gitlab.kitware.com/cmake/cmake/-/issues/25773, and https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9788/diffs

Suggested change
cmake_parse_arguments(VAR "" "" "FILES;DEPENDENCIES" ${ARGN})
cmake_parse_arguments(arg "" "" "FILES;DEPENDENCIES" ${ARGN})


if(NOT VAR_FILES)
message(FATAL_ERROR "suit_add_merge_hex_file missing required argument FILES")
endif()
Comment on lines +121 to +123
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be simplified and also ensure common way of printing argument errors:

Suggested change
if(NOT VAR_FILES)
message(FATAL_ERROR "suit_add_merge_hex_file missing required argument FILES")
endif()
zephyr_check_arguments_required(${CMAKE_CURRENT_FUNCTION} arg FILES)


set_property(GLOBAL APPEND PROPERTY SUIT_MERGE_FILE ${VAR_FILES})
set_property(GLOBAL APPEND PROPERTY SUIT_MERGE_DEPENDENCIES ${VAR_DEPENDENCIES})
endfunction()