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

nordicjm · 2024-11-11T06:58:42Z

For nRF54H20 only

NordicBuilder · 2024-11-11T06:59:13Z

The following west manifest projects have been modified in this Pull Request:

Name	Old Revision	New Revision	Diff
matter	nrfconnect/sdk-connectedhomeip@`81eb1e9`	nrfconnect/sdk-connectedhomeip@`93c712e` (master)	nrfconnect/sdk-connectedhomeip@81eb1e99..93c712e6

Note: This message is automatically posted and updated by the Manifest GitHub Action.

NordicBuilder · 2024-11-11T07:01:13Z

CI Information

^{To view the history of this post, clich the 'edited' button above}
Build number: 9

Inputs:

Sources:

sdk-nrf: PR head: 63ac22028d253f0e691b971246f6d53d5994a37e
matter: PR head: 93c712e6e0be9211507cb9e559860b32daa984ed

more details

sdk-nrf:

PR head: 63ac22028d253f0e691b971246f6d53d5994a37e
merge base: 399d9ab75eebef6f29748cda385f237b41255ee5
target head (main): 399d9ab75eebef6f29748cda385f237b41255ee5
Diff

matter:

PR head: 93c712e6e0be9211507cb9e559860b32daa984ed
merge base: 81eb1e99a9fd48bbf3bca2a56e7ac888c7f24a16
Diff

Github labels

Enabled	Name	Description
	ci-disabled	Disable the ci execution
	ci-all-test	Run all of ci, no test spec filtering will be done
	ci-force-downstream	Force execution of downstream even if twister fails
	ci-run-twister	Force run twister
	ci-run-zephyr-twister	Force run zephyr twister

List of changed files detected by CI (10)

cmake
│  ├── sysbuild
│  │  ├── fast_pair_hex.cmake
│  │  ├── suit.cmake
│  │  │ suit_utilities.cmake
modules
│  ├── lib
│  │  ├── matter
│  │  │  ├── config
│  │  │  │  ├── nrfconnect
│  │  │  │  │  ├── chip-module
│  │  │  │  │  │  │ generate_factory_data_sysbuild.cmake
samples
│  ├── bluetooth
│  │  ├── fast_pair
│  │  │  ├── input_device
│  │  │  │  ├── boards
│  │  │  │  │  │ nrf54h20dk_nrf54h20_cpuapp.overlay
│  │  │  │  │ sample.yaml
subsys
│  ├── bluetooth
│  │  ├── services
│  │  │  ├── fast_pair
│  │  │  │  ├── Kconfig.fast_pair
│  │  │  │  │ fp_registration_data.c
sysbuild
│  │ CMakeLists.txt
west.yml

Outputs:

Toolchain

Version: f51bdba1d9
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:f51bdba1d9_912848a074

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

◻️ Toolchain - Skipped: existing toolchain is used
✅ Build twister
✅ Integration tests
- ✅ test-sdk-audio
- ✅ desktop52_verification
- ✅ test-fw-nrfconnect-boot
- ✅ test-fw-nrfconnect-apps
- ✅ test_ble_nrf_config
- ✅ test-fw-nrfconnect-ble_mesh
- ✅ test-fw-nrfconnect-ble_samples
- ✅ test-fw-nrfconnect-chip
- ✅ test-fw-nrfconnect-nfc
- ✅ test-fw-nrfconnect-nrf-iot_libmodem-nrf
- ✅ test-fw-nrfconnect-nrf-iot_serial_lte_modem
- ✅ test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
- ✅ test-fw-nrfconnect-nrf-iot_samples
- ✅ test-fw-nrfconnect-nrf-iot_lwm2m
- ✅ doc-internal
- ✅ test-fw-nrfconnect-nrf-iot_thingy91
- ✅ test-fw-nrfconnect-nrf_crypto
- ✅ test-fw-nrfconnect-rpc
- ✅ test-fw-nrfconnect-rs
- ✅ test-fw-nrfconnect-fem
- ✅ test-fw-nrfconnect-tfm
- ✅ test-fw-nrfconnect-thread
- ✅ test-fw-nrfconnect-zigbee
- ✅ test-sdk-find-my
- ✅ test-fw-nrfconnect-nrf-iot_mosh
- ✅ test-fw-nrfconnect-nrf-iot_positioning
- ✅ test-sdk-sidewalk
- ✅ test-sdk-wifi
- ✅ test-low-level
- ✅ test-fw-nrfconnect-nrf-iot_nrf_provisioning
- ✅ test-sdk-pmic-samples
- ✅ test-sdk-mcuboot
- ✅ test-sdk-dfu
- ✅ test-fw-nrfconnect-ps

Note: This message is automatically posted and updated by the CI

NordicBuilder · 2024-11-11T07:15:08Z

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

NordicBuilder · 2024-11-11T08:11:29Z

Memory footprint analysis revealed the following potential issues

sample.matter.template.release[nrf7002dk/nrf5340/cpuapp]: High ROM usage: 811902[B] - link (cc: @kkasperczyk-no @ArekBalysNordic @markaj-nordic)

Note: This message is automatically posted and updated by the CI (latest/sdk-nrf/PR-18792/9)

alstrzebonski

Looks good.

alstrzebonski · 2024-11-12T13:46:53Z

samples/bluetooth/fast_pair/input_device/boards/nrf54h20dk_nrf54h20_cpuapp.overlay

+		storage_partition: partition@1e3000 {
+			reg = <0x1e3000 DT_SIZE_K(20)>;
+		};
+
+		bt_fast_pair_partition: partition@1e8fb8 {
+			label = "bt_fast_pair";
+			reg = <0x1e8fb8 0x48>;
+		};


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.

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

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.

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

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

FYI: 0x1e8fb8 is not divisible by 128 bits, which is the "erase block" for MRAM, so this assignment is OK as long as you do not need to erase this partition using flash_* APIs.

@tomchy - just to make sure - you're referring here to write-block-size which is equal to 16 bytes? That means that addresses should be divisible by 0x10. Am I correct?

kapi-no

The Fast Pair part looks good to me. The configurations aspects of Fast Pair (e.g. partition position) can be improved in the follow-up PRs

tejlmand

couple of observations and a small proposal improvement, but nothing blocking.

cmake/sysbuild/suit_utilities.cmake

tejlmand · 2024-11-14T13:17:19Z

cmake/sysbuild/suit.cmake

    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}


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.

NordicBuilder · 2024-11-15T09:09:26Z

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publish GitHub Action.

tomchy · 2024-11-18T10:50:50Z

cmake/sysbuild/suit.cmake

    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}


Is it intentional to merge those files multiple times (see a for loop in line 505)?

Why is this ran multiple times? Is this for multiple cores?

Correct, once for each domain (app and radio in case of nRF54H). This is the most challenging part of the whole mergehex process - it is important which debugger passes which HEX files, thus two uicr_merged.hex files are generated.

So how do you know what device the current image is for?

We could define the SUIT_MERGE_FILE property independently for each image and make the caller of suit_add_merge_hex_file (e.g. Fast Pair CMake) responsible for indicating the target image.

However, since the Matter PR has already been merged, I suggest making this improvement in the follow-up PR.

Since SUIT is supported on nRF54H20 only, it is assumed in this preliminary implementation that the device is the nRF54H SoC.
When it comes to the domains - any sysbuild image that has IMAGE_TARGET_NAME and CONFIG_NRF_REGTOOL_GENERATE_UICR defined is assumed to be the main image for the domain.

Please note that we may have customer use cases in which the provisioned data needs to be placed in the Radio Core image together with their "main application". Some customers won't use the Application Core to minimize the latency in their radio-based applications (IPC delay). That's why I suggested adding the image argument to the suit_add_merge_hex_file function to make the overall provisioning solution more flexible.

However, for now, I think it is enough to support only provisioning for the main image of the Application Core.

Adds support for merging other files onto the uicr_merged.hex file of the application core Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

Adds support for generating fast pair data for nRF54H20 which does not use partition manager Signed-off-by: Kamil Piszczek <Kamil.Piszczek@nordicsemi.no> Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

Added nRF54H20 DK support to the Fast Pair Input Device sample. Ref: NCSDK-29602 Signed-off-by: Kamil Piszczek <Kamil.Piszczek@nordicsemi.no>

Updates matter to include support for generating factory data when partition manager is not used Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

Calls the updated factory data function irrespective of partition manager enablement Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

nordicjm added this to the 2.9.0 milestone Nov 11, 2024

nordicjm requested review from a team as code owners November 11, 2024 06:58

github-actions bot added manifest changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Nov 11, 2024

NordicBuilder added manifest-matter DNM labels Nov 11, 2024

nordicjm force-pushed the fast_pair_dts_provisioning_wip branch from 4df741a to 2b3619b Compare November 11, 2024 10:02

alstrzebonski reviewed Nov 12, 2024

View reviewed changes

kapi-no approved these changes Nov 12, 2024

View reviewed changes

alstrzebonski self-requested a review November 12, 2024 14:37

alstrzebonski approved these changes Nov 12, 2024

View reviewed changes

maciejpietras requested review from robertstypa and ahasztag November 12, 2024 16:46

robertstypa approved these changes Nov 13, 2024

View reviewed changes

kkasperczyk-no approved these changes Nov 14, 2024

View reviewed changes

tejlmand approved these changes Nov 14, 2024

View reviewed changes

nordicjm force-pushed the fast_pair_dts_provisioning_wip branch from 2b3619b to 5e5c4f7 Compare November 15, 2024 08:48

NordicBuilder removed the DNM label Nov 15, 2024

kapi-no mentioned this pull request Nov 15, 2024

cmake: sysbuild: fast_pair: add support for DTS-based memory maps #17923

Closed

tomchy reviewed Nov 18, 2024

View reviewed changes

nordicjm and others added 3 commits November 19, 2024 07:38

cmake: sysbuild: suit: Add support for merging other files

61b0dee

Adds support for merging other files onto the uicr_merged.hex file of the application core Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

cmake: sysbuild: fast_pair: add support for nRF54H20 non-PM

f23a0a7

Adds support for generating fast pair data for nRF54H20 which does not use partition manager Signed-off-by: Kamil Piszczek <Kamil.Piszczek@nordicsemi.no> Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

samples: bluetooth: fast_pair: input_device: add support for nrf54h20dk

f9f853d

Added nRF54H20 DK support to the Fast Pair Input Device sample. Ref: NCSDK-29602 Signed-off-by: Kamil Piszczek <Kamil.Piszczek@nordicsemi.no>

nordicjm added 2 commits November 19, 2024 07:38

manifest: Update sdk-connectedhomeip

3770e68

Updates matter to include support for generating factory data when partition manager is not used Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

sysbuild: Add support for matter non-PM factory data generation

63ac220

Calls the updated factory data function irrespective of partition manager enablement Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

nordicjm force-pushed the fast_pair_dts_provisioning_wip branch from 5e5c4f7 to 63ac220 Compare November 19, 2024 07:39

nordicjm requested a review from tomchy November 19, 2024 07:39

nordicjm merged commit 7fdaf5b into nrfconnect:main Nov 19, 2024
13 checks passed

nordicjm deleted the fast_pair_dts_provisioning_wip branch November 19, 2024 12:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

nordicjm commented Nov 11, 2024

NordicBuilder commented Nov 11, 2024 •

edited

Loading

NordicBuilder commented Nov 11, 2024 •

edited

Loading

NordicBuilder commented Nov 11, 2024

NordicBuilder commented Nov 11, 2024 •

edited

Loading

alstrzebonski left a comment

alstrzebonski Nov 12, 2024 •

edited

Loading

nordicjm Nov 12, 2024

kapi-no Nov 12, 2024

alstrzebonski Nov 12, 2024

kapi-no Nov 12, 2024

alstrzebonski Nov 13, 2024

tomchy Nov 19, 2024

alstrzebonski Dec 2, 2024

kapi-no left a comment

tejlmand left a comment

tejlmand Nov 14, 2024

NordicBuilder commented Nov 15, 2024

tomchy Nov 18, 2024

nordicjm Nov 18, 2024

tomchy Nov 18, 2024

nordicjm Nov 18, 2024 •

edited

Loading

kapi-no Nov 18, 2024

tomchy Nov 18, 2024

nordicjm Nov 19, 2024

kapi-no Nov 19, 2024

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

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

Conversation

nordicjm commented Nov 11, 2024

NordicBuilder commented Nov 11, 2024 • edited Loading

NordicBuilder commented Nov 11, 2024 • edited Loading

CI Information

Inputs:

Sources:

Github labels

Outputs:

Toolchain

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

NordicBuilder commented Nov 11, 2024

NordicBuilder commented Nov 11, 2024 • edited Loading

Memory footprint analysis revealed the following potential issues

alstrzebonski left a comment

Choose a reason for hiding this comment

alstrzebonski Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kapi-no left a comment

Choose a reason for hiding this comment

tejlmand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NordicBuilder commented Nov 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nordicjm Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NordicBuilder commented Nov 11, 2024 •

edited

Loading

NordicBuilder commented Nov 11, 2024 •

edited

Loading

NordicBuilder commented Nov 11, 2024 •

edited

Loading

alstrzebonski Nov 12, 2024 •

edited

Loading

nordicjm Nov 18, 2024 •

edited

Loading