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

cmake: sysbuild: partition_manager: Fix domain handling issues #20479

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

nordicjm
Copy link
Contributor

Fixes some issues around domain handling:

  • The domain was not got and was always supplied as empty
  • The wrong board was provided when using other board targets e.g. network core
  • Checksum handling was done with a singular file instead of once per domain, which would output an erroneous warning for other domains

Seemingly not noticed that the domain handling for PM in sysbuild has never worked. This does now mean users need really silly named files like pm_static_nrf5340dk_nrf5340_cpunet_CPUNET.yml but that's PM.

@nordicjm nordicjm requested a review from a team as a code owner February 19, 2025 08:56
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Feb 19, 2025
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Feb 19, 2025

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 6

Inputs:

Sources:

sdk-nrf: PR head: ce094bb80f847c8c5d6a0438ecc92962579884eb

more details

sdk-nrf:

PR head: ce094bb80f847c8c5d6a0438ecc92962579884eb
merge base: 4b87c45590ae4715e38e0003bb327f6be69db06c
target head (main): 1883f06ef4c288b7265552421a71afd22462566f
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 (1)
cmake
│  ├── sysbuild
│  │  │ partition_manager.cmake

Outputs:

Toolchain

Version: 4cff34261a
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:4cff34261a_bece0367df

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

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister - Skipped: Skipping Build & Test as it succeeded in a previous run: 3
  • ✅ Integration tests
    • ✅ test-sdk-audio - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ desktop52_verification - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-boot - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-apps - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test_ble_nrf_config - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-ble_mesh - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-ble_samples - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-chip - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nfc - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf-iot_cloud - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf-iot_serial_lte_modem - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • ✅ test-fw-nrfconnect-nrf-iot_samples - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf-iot_lwm2m - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ doc-internal - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf-iot_thingy91 - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf_crypto - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-proprietary_esb - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-rpc - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-rs - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-fem - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-tfm - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-thread - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-zigbee - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-sdk-find-my - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf-iot_mosh - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf-iot_positioning - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-sdk-sidewalk - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-sdk-wifi - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-low-level - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-sdk-pmic-samples - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-sdk-mcuboot - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-sdk-dfu - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-ps - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-secdom-samples-public - Skipped: Job was skipped as it succeeded in a previous run

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

@nordicjm nordicjm requested review from gmarull and 57300 February 19, 2025 09:44
"this may cause images to use the original static partition manager file "
"configuration data, which is incorrect. It is recommended that a pristine build be "
"performed when a static partition manager file is updated."
if(DEFINED PM_DOMAIN)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I'd suggest factoring out the common parts to something like:

if(DEFINED PM_DOMAIN)
  set(static_configuration_checksum_var STATIC_PM_FILE_HASH_${PM_DOMAIN})
else()
  set(static_configuration_checksum_var STATIC_PM_FILE_HASH)
endif()
if(NOT DEFINED ${static_configuration_checksum_var} OR NOT "${${static_configuration_checksum_var}}" STREQUAL "${static_configuration_checksum}")
  if(DEFINED ${static_configuration_checksum_var})
    message(WARNING "Static partition manager file has changed since this project was last configured, "
                    "this may cause images to use the original static partition manager file "
                    "configuration data, which is incorrect. It is recommended that a pristine build be "
                    "performed when a static partition manager file is updated."
    )
  endif()

  set(${static_configuration_checksum_var} "${static_configuration_checksum}" CACHE INTERNAL
      "nRF Connect SDK static partition manager file hash for ${static_configuration_file}" FORCE
  )
endif()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied

Fixes some issues around domain handling:
- The domain was not got and was always supplied as empty
- The wrong board was provided when using other board targets e.g.
  network core
- Checksum handling was done with a singular file instead of once
  per domain, which would output an erroneous warning for other
  domains

Signed-off-by: Jamie McCrae <[email protected]>
@nordicjm nordicjm merged commit ae518d0 into nrfconnect:main Feb 20, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants