Skip to content

Conversation

@ArekBalysNordic
Copy link
Contributor

If the T1 type was an enum type, then the kInvalidKey gets 0 value instead of type::max().
To fix it, conditionally get the underlying type of T1 if it is an enum, and return T1 if not.

Copilot AI review requested due to automatic review settings November 7, 2025 12:49
@ArekBalysNordic ArekBalysNordic requested a review from a team as a code owner November 7, 2025 12:49
@NordicBuilder NordicBuilder added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Nov 7, 2025
If the T1 type was an enum type, then the kInvalidKey gets 0 value
instead of type::max().
To fix it, conditionally get the underlying type of T1 if it is
an enum, and return T1 if not.

Signed-off-by: Arkadiusz Balys <arkadiusz.balys@nordicsemi.no>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an issue where enum types used as keys in FiniteMap would incorrectly get kInvalidKey set to 0 instead of the maximum value of the enum's underlying type. The fix introduces a KeyTypeHelper template struct to conditionally extract the underlying type for enums while preserving the original type for non-enum types.

  • Adds KeyTypeHelper template struct to handle type extraction for both enum and non-enum key types
  • Updates kInvalidKey initialization to use the underlying type's maximum value for enums

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@NordicBuilder
Copy link
Contributor

NordicBuilder commented Nov 7, 2025

CI Information

To view the history of this post, click the 'edited' button above
Build number: 1

Inputs:

Sources:

sdk-nrf: PR head: 9a3bb49ceb8504083f7c607908ca8047a85089ab

more details

sdk-nrf:

PR head: 9a3bb49ceb8504083f7c607908ca8047a85089ab
merge base: 35246387d4fdc7e84615716a068a57afca940ce2
target head (main): ad7d9b915b4d543e00b23c7b9be6a20dad9c63fd
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)
samples
│  ├── matter
│  │  ├── common
│  │  │  ├── src
│  │  │  │  ├── util
│  │  │  │  │  │ finite_map.h

Outputs:

Toolchain

Version: df3cc9d822
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:df3cc9d822_e595b21c39

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

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 104
  • ✅ Integration tests
    • ✅ test-fw-nrfconnect-chip
Disabled integration tests
    • test-fw-nrfconnect-nrf_lrcs_positioning
    • desktop52_verification
    • test_ble_nrf_config
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-ble_samples
    • 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_samples
    • test-fw-nrfconnect-nrf-iot_thingy91
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-ps-main
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-fw-nrfconnect-tfm
    • test-fw-nrfconnect-thread-main
    • test-low-level
    • test-sdk-audio
    • test-sdk-dfu
    • test-sdk-find-my
    • test-sdk-mcuboot
    • test-sdk-wifi
    • test-secdom-samples-public

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

@NordicBuilder
Copy link
Contributor

Memory footprint analysis revealed the following potential issues

sample.matter.template.debug[nrf5340dk/nrf5340/cpuapp]: RAM size increased by 2048[B] in comparison to the main[3524638] branch. - link (cc: @kkasperczyk-no @ArekBalysNordic @markaj-nordic)
sample.matter.template.release[nrf5340dk/nrf5340/cpuapp]: RAM size increased by 2048[B] in comparison to the main[3524638] branch. - link (cc: @kkasperczyk-no @ArekBalysNordic @markaj-nordic)
sample.matter.template.debug[nrf52840dk/nrf52840]: RAM size increased by 2048[B] in comparison to the main[3524638] branch. - link (cc: @kkasperczyk-no @ArekBalysNordic @markaj-nordic)
sample.matter.template.release[nrf52840dk/nrf52840]: RAM size increased by 2048[B] in comparison to the main[3524638] branch. - link (cc: @kkasperczyk-no @ArekBalysNordic @markaj-nordic)

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

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.

2 participants