Skip to content

applications: nrf5340_audio: Ensure button names are printed in a standard manner #22459

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
Merged
merged 1 commit into from
May 28, 2025
Merged

applications: nrf5340_audio: Ensure button names are printed in a standard manner #22459

merged 1 commit into from
May 28, 2025

Conversation

gWacey
Copy link
Contributor

@gWacey gWacey commented May 22, 2025

OCT-3382

Assign the button names dependent on the GPIO pin, so as to match the button correctly to it's name across multiple boards.
Included a new API to convert a pin number to it's name string.

@gWacey gWacey requested a review from a team as a code owner May 22, 2025 11:43
@gWacey gWacey requested review from koffes, rick1082 and alexsven May 22, 2025 11:43
@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 May 22, 2025
@NordicBuilder
Copy link
Contributor

NordicBuilder commented May 22, 2025 •
edited
Loading

CI Information

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

Inputs:

Sources:

sdk-nrf: PR head: 277e447130f61649440b5b5ad67679ffeab10dca

more details

sdk-nrf:

PR head: 277e447130f61649440b5b5ad67679ffeab10dca
merge base: 078748ac132e96b9178f3040cb89af0feef4dd48
target head (main): 732a32319c6608c769fd2403d8b5559a715db31f
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 (2) 
applications
│  ├── nrf5340_audio
│  │  ├── src
│  │  │  ├── modules
│  │  │  │  ├── button_assignments.h
│  │  │  │  │ button_handler.c

Outputs:

Toolchain

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

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

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 44
  • ✅ Integration tests
    • ✅ test-sdk-audio
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-chip
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_cloud
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_mosh
    • 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-proprietary_esb
    • 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-find-my
    • test-sdk-mcuboot
    • test-sdk-pmic-samples
    • test-sdk-wifi
    • test-secdom-samples-public

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

@github-actions GitHub Actions
Copy link

You can find the documentation preview for this PR here.

@gWacey gWacey changed the title applications: Ensure button names are printed in a standard manner applications: nrf5340_audio: Ensure button names are printed in a standard manner May 22, 2025
koffes
koffes requested changes May 23, 2025
View reviewed changes
applications/nrf5340_audio/src/modules/button_handler.c
.name = STRINGIFY(BUTTON_##button_node_id), \
.gpio = GPIO_DT_SPEC_GET(button_node_id, gpios), \
},
.name = BUTTON_NAME(DT_GPIO_PIN(button_node_id, gpios)), \
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see what this change does. It seems to me that line 53-60 does the same as stringify would do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The STRINGIFY does not expand the enclosed macro. In the above the replacement happens before the STRINGIFY but I need to select a string at the preprocessor stage that matches the required name. Therefore the BUTTON_NAME macro compares the pin number of the button with the node being parsed. This will then detect the correct string for the name.
The pins for each button are different for each DK and hence I need a way to select which button does what. This approach also allows the action of the button to be changed in the assignment file (e.g. sw0 volume down could be sw2 instead).

applications/nrf5340_audio/src/modules/button_handler.h Outdated
*
* @return 0 if success, an error code otherwise.
*/
int button_handler_pin_to_name(gpio_pin_t button_pin, char **button_name);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, but this is not called anywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did have this as a helper function, but removed as no longer needed.

@gWacey gWacey force-pushed the OCT-3382-Ensure-button-names-are-printed-in-a-standard-manner branch from 47fec87 to 191151f Compare May 27, 2025 09:08
@gWacey gWacey requested a review from koffes May 27, 2025 09:08
@gWacey gWacey force-pushed the OCT-3382-Ensure-button-names-are-printed-in-a-standard-manner branch 3 times, most recently from 2e00bc9 to de86161 Compare May 27, 2025 14:02
@gWacey
applications: Ensure button names are printed in a standard manner
277e447 
Assign the button names dependent on the GPIO pin, so as to
match the button correctly to it's name across multiple boards.

Signed-off-by: Graham Wacey <[email protected]>
@gWacey gWacey force-pushed the OCT-3382-Ensure-button-names-are-printed-in-a-standard-manner branch from de86161 to 277e447 Compare May 27, 2025 14:25
Frodevan
Frodevan approved these changes May 27, 2025
View reviewed changes
Copy link
Contributor

@Frodevan Frodevan left a comment

Choose a reason for hiding this comment

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

Prints the button-names correctly on nRF5340 audio devkit now

@koffes koffes merged commit 8d3db86 into nrfconnect:main May 28, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@koffes koffes koffes approved these changes

@Frodevan Frodevan Frodevan approved these changes

@rick1082 rick1082 Awaiting requested review from rick1082 rick1082 is a code owner automatically assigned from nrfconnect/ncs-audio

@alexsven alexsven Awaiting requested review from alexsven alexsven is a code owner automatically assigned from nrfconnect/ncs-audio

Assignees
No one assigned
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gWacey @NordicBuilder @koffes @Frodevan