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

samples: matter: Reduce memory usage #18554

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

Conversation

adigie
Copy link
Member

@adigie adigie commented Nov 5, 2024

  • Remove custom heap size from nRF54L15 configs.
  • Reduce Packet Buffer Pool size to 11.

@adigie adigie requested review from a team as code owners November 5, 2024 09:20
@CLAassistant
Copy link

CLAassistant commented Nov 5, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added manifest changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Nov 5, 2024
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Nov 5, 2024

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

Name Old Revision New Revision Diff
matter nrfconnect/sdk-connectedhomeip@93c712e (master) nrfconnect/sdk-connectedhomeip#507 nrfconnect/sdk-connectedhomeip#507/files

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

@NordicBuilder
Copy link
Contributor

NordicBuilder commented Nov 5, 2024

CI Information

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

Inputs:

Sources:

sdk-nrf: PR head: 498ab89990f04b732bc5f1a0f6ba715302d19ee8
matter: PR head: 94ebf53247a8a7a9cdd727a2db1e130e532deac4

more details

sdk-nrf:

PR head: 498ab89990f04b732bc5f1a0f6ba715302d19ee8
merge base: 19945a632cd9d772f5d094f22a97b5180b7be33b
target head (main): 251af5d13d75fe85c430629d17b1a8a0068d8120
Diff

matter:

PR head: 94ebf53247a8a7a9cdd727a2db1e130e532deac4
merge base: 93c712e6e0be9211507cb9e559860b32daa984ed
target head (master): 93c712e6e0be9211507cb9e559860b32daa984ed
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 (18)
doc
│  ├── nrf
│  │  ├── protocols
│  │  │  ├── matter
│  │  │  │  ├── getting_started
│  │  │  │  │  │ memory_optimization.rst
modules
│  ├── lib
│  │  ├── matter
│  │  │  ├── config
│  │  │  │  ├── nrfconnect
│  │  │  │  │  ├── chip-module
│  │  │  │  │  │  │ Kconfig
│  │  │  ├── src
│  │  │  │  ├── platform
│  │  │  │  │  ├── nrfconnect
│  │  │  │  │  │  │ SystemPlatformConfig.h
samples
│  ├── matter
│  │  ├── light_bulb
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp_ns.conf
│  │  ├── light_switch
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp_ns.conf
│  │  ├── lock
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp_ns.conf
│  │  ├── smoke_co_alarm
│  │  │  ├── boards
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp.conf
│  │  ├── template
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp_internal.conf
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp_ns.conf
│  │  ├── thermostat
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp_ns.conf
│  │  ├── window_covering
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp_ns.conf
west.yml

Outputs:

Toolchain

Version: 8611848abf
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:8611848abf_912848a074

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

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

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

@NordicBuilder
Copy link
Contributor

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.

@@ -8,7 +8,7 @@
CONFIG_BT_LL_SOFTDEVICE_MULTIROLE=y

# nRF54L15 requires bigger stack sizes than nRF52/nRF53 families
CONFIG_CHIP_MALLOC_SYS_HEAP_SIZE=10240
CONFIG_CHIP_MALLOC_SYS_HEAP_SIZE=7168
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we can remove this line completely and have a default values in sdk-connectedhomeip only (potentially different for nRF52/53/54).

Btw. In this PR you have changes configuration only for nRF54L15 - what about 52 and 53?

Also i believe we should run nightly on this PR before merging - as previously some of the Matter tests were failing (maybe its not the case anymore): cdcb580

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we can. We will use default configuration for all platforms - 8KB.

Use default heap size on nRF54L15.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@NordicBuilder
Copy link
Contributor

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.

* Add Kconfig for packet buffer pool size.
* Reduce packet buffer pool size to 11.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@adigie adigie requested a review from a team as a code owner November 19, 2024 15:22
@github-actions github-actions bot added the doc-required PR must not be merged without tech writer approval. label Nov 19, 2024
Mention `CONFIG_CHIP_SYSTEM_PACKETBUFFER_POOL_SIZE` Kconfig option.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@wiba-nordic
Copy link
Contributor

I'll hold on approving for a bit since I'm also part of ncs-matter, so one of the guys can properly approve.

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. DNM doc-required PR must not be merged without tech writer approval. manifest manifest-matter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants