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 all commits
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
78 changes: 75 additions & 3 deletions cmake/sysbuild/fast_pair_hex.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
#
# Copyright (c) 2022-2023 Nordic Semiconductor
# Copyright (c) 2022-2024 Nordic Semiconductor
#
# SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
#

function(fast_pair_hex)
function(fast_pair_hex_pm)
set(fp_partition_name bt_fast_pair)

set(
Expand Down Expand Up @@ -47,4 +47,76 @@ function(fast_pair_hex)
)
endfunction()

fast_pair_hex()
function(fast_pair_hex_dts)
include(${CMAKE_CURRENT_LIST_DIR}/suit_utilities.cmake)

if(NOT SB_CONFIG_SOC_SERIES_NRF54HX)
message(FATAL_ERROR "Fast Pair data provisioning using DTS partitions is only supported"
"for nRF54H series.")
endif()

set(fp_partition_name bt_fast_pair_partition)

sysbuild_dt_nodelabel(
bt_fast_pair_partition_nodelabel
IMAGE
${DEFAULT_IMAGE}
NODELABEL
"${fp_partition_name}"
)
sysbuild_dt_reg_addr(
bt_fast_pair_partition_relative_address
IMAGE
${DEFAULT_IMAGE}
PATH
"${bt_fast_pair_partition_nodelabel}"
)

# This part assumes that the Fast Pair partition resides in MRAM1x.
# TODO: Rewrite it to be more generic and support other memory regions.
sysbuild_dt_nodelabel(mram1x_nodelabel IMAGE ${DEFAULT_IMAGE} NODELABEL "mram1x")
sysbuild_dt_reg_addr(mram1x_address IMAGE ${DEFAULT_IMAGE} PATH "${mram1x_nodelabel}")

math(
EXPR
bt_fast_pair_partition_address
"${mram1x_address} + ${bt_fast_pair_partition_relative_address}"
OUTPUT_FORMAT
HEXADECIMAL)

set(
fp_provisioning_data_hex
${CMAKE_BINARY_DIR}/modules/nrf/subsys/bluetooth/services/fast_pair/fp_provisioning_data.hex
)

set(fp_provisioning_data_address ${bt_fast_pair_partition_address})

add_custom_command(
OUTPUT
${fp_provisioning_data_hex}
COMMAND
${PYTHON_EXECUTABLE} ${ZEPHYR_NRF_MODULE_DIR}/scripts/nrf_provision/fast_pair/fp_provision_cli.py
-o ${fp_provisioning_data_hex} -a ${fp_provisioning_data_address}
-m ${FP_MODEL_ID} -k ${FP_ANTI_SPOOFING_KEY}
COMMENT
"Generating Fast Pair provisioning data hex file"
USES_TERMINAL
)

add_custom_target(
${fp_partition_name}_target
ALL
DEPENDS
"${fp_provisioning_data_hex}"
)

suit_add_merge_hex_file(FILES ${fp_provisioning_data_hex}
DEPENDENCIES ${fp_partition_name}_target
)
endfunction()

if(SB_CONFIG_PARTITION_MANAGER)
fast_pair_hex_pm()
else()
fast_pair_hex_dts()
endif()
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()
32 changes: 32 additions & 0 deletions samples/bluetooth/fast_pair/input_device/boards/nrf54h20dk_nrf54h20_cpuapp.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright (c) 2024 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
*/

&mram1x {
/delete-node/ cpuapp_rw_partitions;

cpuapp_rw_partitions: cpuapp-rw-partitions {
compatible = "nordic,owned-partitions", "fixed-partitions";
status = "okay";
perm-read;
perm-write;
perm-secure;
#address-cells = <1>;
#size-cells = <1>;

dfu_partition: partition@100000 {
reg = <0x100000 DT_SIZE_K(908)>;
};

storage_partition: partition@1e3000 {
reg = <0x1e3000 DT_SIZE_K(20)>;
};

bt_fast_pair_partition: partition@1e8fb8 {
label = "bt_fast_pair";
reg = <0x1e8fb8 0x48>;
};
Comment on lines +23 to +30
Copy link
Contributor

@alstrzebonski alstrzebonski Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure if the addresses are correct. 20kB is equal to 0x5000 so I think the bt_fast_pair_partition address should be set to 0x1e8000 (0x1e3000 + 0x5000) instead of 0x1e8fb8. Correct me if I'm wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kapi-no

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I positioned it at the end of the 0x1000 memory chunk to mimick the PM solution in the case of automatic builds without the specific memory layout file (pm.yaml). On the other hand, we typically position the provisioning data at the beginning of the 0x1000 memory chunk in builds where we define the memory layout file (pm.yaml).

@alstrzebonski, I don't have a preference here. We can align it according to your suggestion or we can leave this part and think about the position of the fast pair provisioning data hex in the nRF54H20 memory in the follow-up task

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be aligned to my suggestion or, if we want to mimic the PM, we should add the comment explaining the gap in the memory map. I'm okay with both solutions. It can be also left as is and improved in follow-up task - maybe that's the best option considering it's @nordicjm's PR. It will be easier to leave it as is for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

@alstrzebonski, could you create a follow-up task?

Copy link
Contributor

Choose a reason for hiding this comment

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

https://nordicsemi.atlassian.net/browse/NCSDK-30269

};
};
2 changes: 2 additions & 0 deletions samples/bluetooth/fast_pair/input_device/sample.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ tests:
- nrf5340dk/nrf5340/cpuapp
- nrf5340dk/nrf5340/cpuapp/ns
- nrf54l15dk/nrf54l15/cpuapp
- nrf54h20dk/nrf54h20/cpuapp
platform_allow:
- nrf52dk/nrf52832
- nrf52840dk/nrf52840
- nrf5340dk/nrf5340/cpuapp
- nrf5340dk/nrf5340/cpuapp/ns
- nrf54l15dk/nrf54l15/cpuapp
- nrf54h20dk/nrf54h20/cpuapp
tags: bluetooth ci_build sysbuild
2 changes: 1 addition & 1 deletion subsys/bluetooth/services/fast_pair/Kconfig.fast_pair
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ endif # BT_FAST_PAIR_GATT_SERVICE
config BT_FAST_PAIR_REGISTRATION_DATA
bool
default y
select PM_SINGLE_IMAGE
select PM_SINGLE_IMAGE if !(SOC_SERIES_NRF54HX)
help
Add Fast Pair registration data source files.

Expand Down
7 changes: 6 additions & 1 deletion subsys/bluetooth/services/fast_pair/fp_registration_data.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <string.h>
#include <zephyr/device.h>
#include <zephyr/storage/flash_map.h>
#include <pm_config.h>

#include <zephyr/logging/log.h>
LOG_MODULE_DECLARE(fast_pair, CONFIG_BT_FAST_PAIR_LOG_LEVEL);
Expand All @@ -18,8 +17,14 @@ LOG_MODULE_DECLARE(fast_pair, CONFIG_BT_FAST_PAIR_LOG_LEVEL);
#include "fp_crypto.h"
#include "fp_registration_data.h"

#if defined(CONFIG_PARTITION_MANAGER_ENABLED)
#include <pm_config.h>
#define FP_PARTITION_ID PM_BT_FAST_PAIR_ID
#define FP_PARTITION_SIZE PM_BT_FAST_PAIR_SIZE
#else
#define FP_PARTITION_ID FIXED_PARTITION_ID(bt_fast_pair_partition)
#define FP_PARTITION_SIZE FIXED_PARTITION_SIZE(bt_fast_pair_partition)
#endif

static const uint8_t fp_magic[] = {0xFA, 0x57, 0xFA, 0x57};

Expand Down
20 changes: 14 additions & 6 deletions sysbuild/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,10 @@ function(${SYSBUILD_CURRENT_MODULE_NAME}_pre_cmake)
set_config_bool(${DEFAULT_IMAGE} CONFIG_BT_FAST_PAIR y)

if(DEFINED FP_MODEL_ID AND DEFINED FP_ANTI_SPOOFING_KEY)
include(${ZEPHYR_NRF_MODULE_DIR}/cmake/sysbuild/fast_pair_hex.cmake)
if(SB_CONFIG_PARTITION_MANAGER)
include(${ZEPHYR_NRF_MODULE_DIR}/cmake/sysbuild/fast_pair_hex.cmake)
endif()

set(FP_DATA_PRESENT "y" CACHE INTERNAL "Fast Pair provisioning data provided" FORCE)
else()
message(WARNING "Fast Pair support is enabled but `FP_MODEL_ID` or `FP_ANTI_SPOOFING_KEY` were not provided, this is likely to cause a build error")
Expand Down Expand Up @@ -610,18 +613,23 @@ function(${SYSBUILD_CURRENT_MODULE_NAME}_post_cmake)

set_property(GLOBAL PROPERTY DOMAIN_APP_APP ${DEFAULT_IMAGE})

# Include any files that need to merge files with uicr_merged.hex before including suit
if(FP_DATA_PRESENT AND NOT SB_CONFIG_PARTITION_MANAGER)
include(${ZEPHYR_NRF_MODULE_DIR}/cmake/sysbuild/fast_pair_hex.cmake)
endif()

if(SB_CONFIG_MATTER_FACTORY_DATA_GENERATE)
include(${ZEPHYR_CONNECTEDHOMEIP_MODULE_DIR}/config/nrfconnect/chip-module/generate_factory_data_sysbuild.cmake)
nrfconnect_generate_factory_data()
endif()

include_packaging()
include_suit()

if(SB_CONFIG_SECURE_BOOT OR SB_CONFIG_MCUBOOT_HARDWARE_DOWNGRADE_PREVENTION)
include_provision_hex()
endif()

if(SB_CONFIG_MATTER_FACTORY_DATA_GENERATE AND SB_CONFIG_PARTITION_MANAGER)
include(${ZEPHYR_CONNECTEDHOMEIP_MODULE_DIR}/config/nrfconnect/chip-module/generate_factory_data_sysbuild.cmake)
nrfconnect_generate_factory_data()
endif()

if(SB_CONFIG_MATTER_OTA)
include(${ZEPHYR_CONNECTEDHOMEIP_MODULE_DIR}/config/zephyr/ota-image_sysbuild.cmake)
if(SB_CONFIG_DFU_MULTI_IMAGE_PACKAGE_BUILD OR SB_CONFIG_SUIT_MULTI_IMAGE_PACKAGE_BUILD)
Expand Down
2 changes: 1 addition & 1 deletion west.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ manifest:
- name: matter
repo-path: sdk-connectedhomeip
path: modules/lib/matter
revision: 81eb1e99a9fd48bbf3bca2a56e7ac888c7f24a16
revision: pull/511/head
west-commands: scripts/west/west-commands.yml
submodules:
- name: nlio
Expand Down
Loading