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

Build improvements #674

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Build improvements #674

wants to merge 5 commits into from
+956 −161

Conversation

ktaborowski
Copy link
Contributor

CI parameters

Github_actions:
  #(branch, hash, pull/XXX/head)
  NRF_revision: main

  # Do not change after creating PR
  Create_NRF_PR: false
Jenkins:
  test-sdk-sidewalk: master

Description

JIRA ticket:

Self review

  • There is no commented code.
  • There are no TODO/FIXME comments without associated issue ticket.
  • Commits are properly organized.
  • Change has been tested.
  • Tests were updated (if applicable).

@github-actions github-actions bot added the source PR changing src files label Jan 23, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 23, 2025 •
edited
Loading

Sample diff used total
nrf52840dk/nrf52840:sample.sidewalk.demo RAM 864 B 124.53 KB 0 B
ROM 43.39 KB 522.54 KB 0 B
nrf52840dk/nrf52840:sample.sidewalk.demo.ble_only RAM 576 B 108.91 KB 0 B
ROM 43.39 KB 440.07 KB 0 B
nrf52840dk/nrf52840:sample.sidewalk.dut RAM 864 B 148.17 KB 0 B
ROM 43.39 KB 588.52 KB 0 B
nrf52840dk/nrf52840:sample.sidewalk.dut.ble_only RAM 576 B 132.49 KB 0 B
ROM 43.4 KB 505.39 KB 0 B
nrf5340dk/nrf5340/cpuapp:sample.sidewalk.demo RAM 864 B 116 KB 0 B
ROM 43.39 KB 445.21 KB 0 B
nrf5340dk/nrf5340/cpuapp:sample.sidewalk.demo.ble_only RAM 576 B 100.53 KB 0 B
ROM 43.39 KB 361.61 KB 0 B
nrf5340dk/nrf5340/cpuapp:sample.sidewalk.dut RAM 864 B 139.77 KB 0 B
ROM 43.39 KB 511.91 KB 0 B
nrf5340dk/nrf5340/cpuapp:sample.sidewalk.dut.ble_only RAM 576 B 124.25 KB 0 B
ROM 43.39 KB 427.84 KB 0 B
nrf54l15dk/nrf54l10/cpuapp:sample.sidewalk.demo RAM 288 B 121.89 KB 0 B
ROM 0 B 458.25 KB 0 B
nrf54l15dk/nrf54l10/cpuapp:sample.sidewalk.demo.ble_only RAM 48 B 106.61 KB 0 B
ROM 124 B 380.63 KB 0 B
nrf54l15dk/nrf54l10/cpuapp:sample.sidewalk.dut RAM 288 B 145.64 KB 0 B
ROM 0 B 524.71 KB 0 B
nrf54l15dk/nrf54l10/cpuapp:sample.sidewalk.dut.ble_only RAM 48 B 130.3 KB 0 B
ROM 124 B 446.53 KB 0 B
nrf54l15dk/nrf54l15/cpuapp:sample.sidewalk.demo RAM 288 B 121.89 KB 0 B
ROM 0 B 458.28 KB 0 B
nrf54l15dk/nrf54l15/cpuapp:sample.sidewalk.demo.ble_only RAM 48 B 106.61 KB 0 B
ROM 124 B 380.66 KB 0 B
nrf54l15dk/nrf54l15/cpuapp:sample.sidewalk.dut RAM 288 B 145.64 KB 0 B
ROM 0 B 524.74 KB 0 B
nrf54l15dk/nrf54l15/cpuapp:sample.sidewalk.dut.ble_only RAM 48 B 130.3 KB 0 B
ROM 124 B 446.56 KB 0 B
nrf54l15dk/nrf54l15/cpuapp/ns:sample.sidewalk.demo RAM 288 B 121.12 KB 0 B
ROM 0 B 406.89 KB 0 B
nrf54l15dk/nrf54l15/cpuapp/ns:sample.sidewalk.demo.ble_only RAM 48 B 105.83 KB 0 B
ROM 124 B 329.27 KB 0 B
nrf54l15dk/nrf54l15/cpuapp/ns:sample.sidewalk.dut RAM 288 B 144.87 KB 0 B
ROM 0 B 473.57 KB 0 B
nrf54l15dk/nrf54l15/cpuapp/ns:sample.sidewalk.dut.ble_only RAM 48 B 129.53 KB 0 B
ROM 124 B 395.4 KB 0 B
nrf52840dk/nrf52840:sample.sidewalk.dut.no_secure RAM 864 B 148.15 KB 0 B
ROM 43.4 KB 579.97 KB 0 B
nrf52840dk/nrf52840:sample.sidewalk.hello RAM 864 B 125.76 KB 0 B
ROM 43.4 KB 537.86 KB 0 B
nrf52840dk/nrf52840:sample.sidewalk.hello.ble_only RAM 576 B 98.73 KB 0 B
ROM 43.4 KB 438.39 KB 0 B
nrf5340dk/nrf5340/cpuapp:sample.sidewalk.dut.no_secure RAM 864 B 139.75 KB 0 B
ROM 43.39 KB 504.48 KB 0 B
nrf5340dk/nrf5340/cpuapp:sample.sidewalk.hello RAM 864 B 117.5 KB 0 B
ROM 43.39 KB 460.77 KB 0 B
nrf5340dk/nrf5340/cpuapp:sample.sidewalk.hello.ble_only RAM 576 B 90.61 KB 0 B
ROM 43.39 KB 360.17 KB 0 B
nrf54l15dk/nrf54l10/cpuapp:sample.sidewalk.dut.no_secure RAM 288 B 145.63 KB 0 B
ROM 0 B 517.02 KB 0 B
nrf54l15dk/nrf54l10/cpuapp:sample.sidewalk.hello RAM 288 B 123.34 KB 0 B
ROM 0 B 473.55 KB 0 B
nrf54l15dk/nrf54l10/cpuapp:sample.sidewalk.hello.ble_only RAM 48 B 96.66 KB 0 B
ROM 124 B 378.93 KB 0 B
nrf54l15dk/nrf54l15/cpuapp:sample.sidewalk.dut.no_secure RAM 288 B 145.63 KB 0 B
ROM 0 B 517.06 KB 0 B
nrf54l15dk/nrf54l15/cpuapp:sample.sidewalk.hello RAM 288 B 123.34 KB 0 B
ROM 0 B 473.58 KB 0 B
nrf54l15dk/nrf54l15/cpuapp:sample.sidewalk.hello.ble_only RAM 48 B 96.66 KB 0 B
ROM 124 B 378.96 KB 0 B
nrf54l15dk/nrf54l15/cpuapp/ns:sample.sidewalk.dut.no_secure RAM 288 B 144.86 KB 0 B
ROM 0 B 470.87 KB 0 B
nrf54l15dk/nrf54l15/cpuapp/ns:sample.sidewalk.hello RAM 288 B 122.56 KB 0 B
ROM 0 B 422.18 KB 0 B
nrf54l15dk/nrf54l15/cpuapp/ns:sample.sidewalk.hello.ble_only RAM 48 B 95.88 KB 0 B
ROM 124 B 327.57 KB 0 B
nrf52840dk/nrf52840:sample.sidewalk.hello.ble_only.release RAM 576 B 93.92 KB 0 B
ROM 43.4 KB 369.01 KB 0 B
nrf52840dk/nrf52840:sample.sidewalk.hello.release RAM 864 B 109.56 KB 0 B
ROM 43.39 KB 447.98 KB 0 B
nrf5340dk/nrf5340/cpuapp:sample.sidewalk.hello.ble_only.release RAM 576 B 85.96 KB 0 B
ROM 43.39 KB 288.5 KB 0 B
nrf5340dk/nrf5340/cpuapp:sample.sidewalk.hello.release RAM 864 B 101.44 KB 0 B
ROM 43.39 KB 368.27 KB 0 B
nrf54l15dk/nrf54l10/cpuapp:sample.sidewalk.hello.ble_only.release RAM 48 B 92.02 KB 0 B
ROM 124 B 307.5 KB 0 B
nrf54l15dk/nrf54l10/cpuapp:sample.sidewalk.hello.release RAM 288 B 107.32 KB 0 B
ROM 0 B 383.25 KB 0 B
nrf54l15dk/nrf54l15/cpuapp:sample.sidewalk.hello.ble_only.release RAM 48 B 92.02 KB 0 B
ROM 128 B 307.53 KB 0 B
nrf54l15dk/nrf54l15/cpuapp:sample.sidewalk.hello.release RAM 288 B 107.32 KB 0 B
ROM 0 B 383.28 KB 0 B
nrf54l15dk/nrf54l15/cpuapp/ns:sample.sidewalk.hello.ble_only.release RAM 48 B 91.25 KB 0 B
ROM 128 B 257.08 KB 0 B
nrf54l15dk/nrf54l15/cpuapp/ns:sample.sidewalk.hello.release RAM 288 B 106.54 KB 0 B
ROM 0 B 332.83 KB 0 B

RobertGalatNordic
RobertGalatNordic approved these changes Jan 24, 2025
View reviewed changes
Copy link
Collaborator

@RobertGalatNordic RobertGalatNordic left a comment

Choose a reason for hiding this comment

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

Minor comments for future improvement.
Tests need to be fixed, but it seems it is a simple job of updating include path in tests.

CMakeLists.txt Outdated Show resolved Hide resolved
Kconfig.dependencies
@@ -127,6 +180,18 @@ config PSA_WANT_ALG_CBC_PKCS7
config PSA_WANT_ALG_HKDF
default n

config PSA_USE_CC3XX_KEY_AGREEMENT_DRIVER
default n if SOC_NRF52840 || SOC_NRF5340_CPUAPP
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering if we could find a more generic reason for the if statement than specific SOC,
I thought that both nrf52 and nrf53 use crypto cell, so maybe this should be unconditionally default to false ? (as nrf54 do should not have and use this config? )

Copy link
Contributor Author

@ktaborowski ktaborowski Jan 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

the nrf crypto configuration is a minefield, I would rather not change this.
Here is the original PR, I just move it #598

subsys/sal/common/sid_pal_types/sid_pal_common_ifc.h Outdated Show resolved Hide resolved
@ktaborowski
pal: separate type headers
7ec40d7 
[KRKNWK-15837]

Signed-off-by: Krzysztof Taborowski <[email protected]>
@github-actions github-actions bot added the tests label Jan 30, 2025
@ktaborowski
config: add sidewalk prebuilds
1dc421a 
[KRKNWK-15837]

Signed-off-by: Krzysztof Taborowski <[email protected]>
@ktaborowski
samples: remove defconfig
f29b021 
Move configuration to generic files

Signed-off-by: Krzysztof Taborowski <[email protected]>
@ktaborowski
pal: fix ifdef for log flush
817b0db 
if logs enalbed and different than minimal

Signed-off-by: Krzysztof Taborowski <[email protected]>
@ktaborowski ktaborowski force-pushed the build_improvements branch 3 times, most recently from 47fd60e to 49fe672 Compare February 3, 2025 11:34
@ktaborowski
tests: use prebuilds in tests
5bfd8f5 
fix build isses

Signed-off-by: Krzysztof Taborowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RobertGalatNordic RobertGalatNordic RobertGalatNordic approved these changes

Assignees
No one assigned
Labels
source PR changing src files tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ktaborowski @RobertGalatNordic