applications: nrf5340_audio: LED module for kits with other LED setup #20483

gWacey · 2025-02-19T10:56:59Z

OCT-3012

The LED module is currently written in a way that’s highly dependent on all of the LEDs present n the audio DK, which prevents the application from running on the nRF5340 DK. The module must be refactored so that it can still show the state of the application with different selection of LEDs.

Currently the button handler expects kits to have five buttons - which is true for the Audio DK, but not for the nRF5340 DK. To have the audio application running on the nRF5340 DK (and other kits in the future), the handler must be refactored to not fail compilation when fewer buttons are present, and reduce the application's overall dependency on button availability.

NordicBuilder · 2025-02-19T11:00:00Z

CI Information

^{To view the history of this post, clich the 'edited' button above}
Build number: 37

Inputs:

Sources:

sdk-nrf: PR head: 3cd0f2152a5bcf3e246d8a4861c5777fc419ea39

more details

sdk-nrf:

PR head: 3cd0f2152a5bcf3e246d8a4861c5777fc419ea39
merge base: 8798f86aa24582bd18256cbaca76cb25e25096cb
target head (main): 8798f86aa24582bd18256cbaca76cb25e25096cb
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 (30)

applications
│  ├── nrf5340_audio
│  │  ├── boards
│  │  │  ├── nrf5340_audio_dk_nrf5340_cpuapp.overlay
│  │  │  ├── nrf5340_audio_dk_nrf5340_cpuapp_fota.overlay
│  │  │  │ nrf5340dk_nrf5340_cpuapp.overlay
│  │  ├── broadcast_sink
│  │  │  │ main.c
│  │  ├── broadcast_source
│  │  │  │ main.c
│  │  ├── doc
│  │  │  ├── building.rst
│  │  │  ├── requirements.rst
│  │  │  │ user_interface.rst
│  │  ├── sample.yaml
│  │  ├── src
│  │  │  ├── audio
│  │  │  │  ├── audio_datapath.c
│  │  │  │  │ audio_system.c
│  │  │  ├── bluetooth
│  │  │  │  ├── bt_management
│  │  │  │  │  │ bt_mgmt.c
│  │  │  ├── modules
│  │  │  │  ├── button_assignments.h
│  │  │  │  ├── button_handler.c
│  │  │  │  ├── button_handler.h
│  │  │  │  ├── led.c
│  │  │  │  ├── led.h
│  │  │  │  ├── led_assignments.h
│  │  │  │  │ nrf5340_dk.h
│  │  │  ├── utils
│  │  │  │  ├── CMakeLists.txt
│  │  │  │  ├── Kconfig
│  │  │  │  ├── audio_nrf5340_dk_init.c
│  │  │  │  ├── audio_nrf5340_dk_init.h
│  │  │  │  ├── audio_nrf5340_dk_version.c
│  │  │  │  ├── audio_nrf5340_dk_version.h
│  │  │  │  ├── error_handler.c
│  │  │  │  │ nrf5340_audio_dk.h
│  │  ├── unicast_client
│  │  │  │ main.c
│  │  ├── unicast_server
│  │  │  │ main.c
samples
│  ├── bluetooth
│  │  ├── nrf_auraconfig
│  │  │  ├── src
│  │  │  │  │ nrf_auraconfig.c

Outputs:

Toolchain

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

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

◻️ Toolchain - Skipped: existing toolchain is used
❌ Build twister
- sdk-nrf test count: 479
❌ Integration tests
- ❌ test-sdk-audio

Disabled integration tests

- desktop52_verification
- doc-internal
- test-fw-nrfconnect-apps
- 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_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
- test-fw-nrfconnect-rpc
- test-fw-nrfconnect-rs
- test-fw-nrfconnect-tfm
- test-fw-nrfconnect-thread
- test-low-level
- test-sdk-dfu
- 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

NordicBuilder · 2025-02-19T11:16:44Z

You can find the documentation preview for this PR at this link.

Note: This comment is automatically posted by the Documentation Publish GitHub Action.

applications/nrf5340_audio/boards/nrf5340_audio_dk_nrf5340_cpuapp.overlay

applications/nrf5340_audio/boards/nrf5340dk_nrf5340_cpuapp.overlay

nordicjm · 2025-02-19T11:43:00Z

applications/nrf5340_audio/src/modules/button_assignments.h

 /** @brief List of buttons and associated metadata
 */
 enum button_pin_names {
 	BUTTON_VOLUME_DOWN = DT_GPIO_PIN(DT_ALIAS(sw0), gpios),
 	BUTTON_VOLUME_UP = DT_GPIO_PIN(DT_ALIAS(sw1), gpios),
 	BUTTON_PLAY_PAUSE = DT_GPIO_PIN(DT_ALIAS(sw2), gpios),
+#if defined(CONFIG_BOARD_NRF5340_AUDIO_DK_NRF5340_CPUAPP)


use DT macros to check if node exists

not addressed

Merged in the button PR so as to sort this.

nordicjm · 2025-02-19T11:43:07Z

applications/nrf5340_audio/src/modules/button_handler.c

@@ -52,6 +52,7 @@ const static struct btn_config btn_cfg[] = {
 		.btn_pin = BUTTON_PLAY_PAUSE,
 		.btn_cfg_mask = DT_GPIO_FLAGS(DT_ALIAS(sw2), gpios),
 	},
+#if defined(CONFIG_BOARD_NRF5340_AUDIO_DK_NRF5340_CPUAPP)


nordicjm · 2025-02-19T11:43:15Z

applications/nrf5340_audio/src/modules/led.c


 #include <zephyr/logging/log.h>
-LOG_MODULE_REGISTER(led, CONFIG_MODULE_LED_LOG_LEVEL);
+LOG_MODULE_REGISTER(led, 4); /* CONFIG_MODULE_LED_LOG_LEVEL); */


applications/nrf5340_audio/src/utils/CMakeLists.txt

nordicjm · 2025-02-19T11:44:45Z

applications/nrf5340_audio/src/utils/board_init.h

+ *
+ * @return	0 if successful, error otherwise.
+ */
+int board_init(void);


do not name function with board_* likewise with Kconfigs, that is reserved for actual boards files in the boards folder not applications

changed to audio_nrf5340_dk_*

nordicjm · 2025-02-19T11:44:54Z

applications/nrf5340_audio/src/utils/board_init.h

+#ifndef _BOARD_H_
+#define _BOARD_H_


github-actions · 2025-02-20T07:57:38Z

After documentation is built, you will find the preview for this PR here.

github-actions · 2025-02-20T07:59:36Z

After documentation is built, you will find the preview for this PR here.

applications/nrf5340_audio/boards/nrf5340_audio_dk_nrf5340_cpuapp_fota.overlay

applications/nrf5340_audio/boards/nrf5340dk_nrf5340_cpuapp.conf

applications/nrf5340_audio/boards/nrf5340_audio_dk_nrf5340_cpuapp.overlay

applications/nrf5340_audio/boards/nrf5340dk_nrf5340_cpuapp.overlay

applications/nrf5340_audio/src/audio/audio_datapath.c

applications/nrf5340_audio/src/utils/board_init.c

applications/nrf5340_audio/src/utils/board_init.h

applications/nrf5340_audio/src/utils/error_handler.c

applications/nrf5340_audio/unicast_server/main.c

github-actions · 2025-02-27T11:40:37Z

You can find the documentation preview for this PR here.

Preview links for modified nRF Connect SDK documents:

https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/applications/nrf5340_audio/doc/building.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/applications/nrf5340_audio/doc/user_interface.html

greg-fer

There should be no reference to the nRF5340 DK until the https://docs.nordicsemi.com/bundle/ncs-latest/page/nrf/applications/nrf5340_audio/doc/requirements.html mentions the nRF5340 DK as supported platform and all of the applications have been fully updated to support the nRF5340 DK.

For Doxygen group assigments, please see https://docs.nordicsemi.com/bundle/ncs-latest/page/nrf/dev_model_and_contributions/documentation/styleguide.html#file_headers_and_groups

greg-fer · 2025-03-07T07:22:12Z

applications/nrf5340_audio/doc/user_interface.rst

+| **POWER**         | Turns the development kit on or off.                                                | All                                   |         Y        |      Y     |
+-------------------+-------------------------------------------------------------------------------------+---------------------------------------+------------------+------------+
+| **DEBUG ENABLE**  | Turns on or off power for debug features.                                           | All                                   |         Y        |      N     |


https://docs.nordicsemi.com/bundle/ncs-latest/page/nrf/applications/nrf5340_audio/doc/requirements.html are not mentioning the nRF5340 DK as supported platform for the audio applications, so you cannot add mentions of the nRF5340 DK to the docs until that page is updated. This is because the documentation will become inconsistent and confusing to users.

Added nRF5340 DK as a supported platform.

greg-fer · 2025-03-07T07:24:29Z

applications/nrf5340_audio/doc/user_interface.rst

+---------------+-----------------------------------------------------------------------------------------------------------+---------------------------------------------+-------------------------------+
+| Button        | Function                                                                                                  | Applications                                | Development Kit               |
+|               |                                                                                                           |                                             +------------------+------------+
+|               |                                                                                                           |                                             | nRF5340 Audio DK | nRF5340 DK |


This addition is not matching the list of supported boards in Requirements.

greg-fer · 2025-03-07T07:37:19Z

applications/nrf5340_audio/doc/user_interface.rst

+--------------------------+-----------------------------------------------------------------------------------------------------------+---------------------------------------------+-------------------------------+
+| LED                      |Indication                                                                                                 | Applications                                | Development Kit               |
+|                          |                                                                                                           |                                             +------------------+------------+
+|                          |                                                                                                           |                                             | nRF5340 Audio DK | nRF5340 DK |


Same comment as above.

greg-fer · 2025-03-07T07:38:25Z

applications/nrf5340_audio/src/modules/led_assignments.h

+#define _LED_ASSIGNMENTS_H_
+
+#include <zephyr/devicetree.h>
+#include <stdint.h>


Doxygen group assignment is missing.

greg-fer · 2025-03-07T07:39:04Z

applications/nrf5340_audio/src/utils/nrf5340_dk_init.h

+#ifndef _NRF5340_BOARD_H_
+#define _NRF5340_BOARD_H_
+
+/**


Doxygen group assignment is missing.

greg-fer · 2025-03-07T07:39:33Z

applications/nrf5340_audio/src/utils/nrf5340_dk_version.h


-#include "board.h"
+#include "nrf5340_dk.h"



Same as above: doxygen group assignment is missing.

nordicjm · 2025-03-17T09:58:37Z

applications/nrf5340_audio/src/drivers/Kconfig

@@ -8,6 +8,7 @@ menu "Drivers"

 menuconfig NRF5340_AUDIO_CS47L63_DRIVER
 	bool "CS47L63 HW codec driver"
+	default n


Suggested change

default n

nordicjm · 2025-03-17T09:58:40Z

applications/nrf5340_audio/src/modules/Kconfig

@@ -20,6 +20,7 @@ config AUDIO_SYNC_TIMER_USES_RTC
 #----------------------------------------------------------------------------#
 menuconfig NRF5340_AUDIO_POWER_MEASUREMENT
 	bool "Power measurement"
+	default n


Suggested change

default n

nordicjm · 2025-03-17T09:58:44Z

applications/nrf5340_audio/src/modules/Kconfig

@@ -146,6 +147,7 @@ endmenu # Zbus
 #----------------------------------------------------------------------------#
 config NRF5340_AUDIO_SD_CARD_MODULE
 	bool "Audio SD Card Module"
+	default n


Suggested change

default n

nordicjm · 2025-03-17T09:58:57Z

applications/nrf5340_audio/src/modules/button_assignments.h

 /** @brief List of buttons and associated metadata
 */
 enum button_pin_names {
 	BUTTON_VOLUME_DOWN = DT_GPIO_PIN(DT_ALIAS(sw0), gpios),
 	BUTTON_VOLUME_UP = DT_GPIO_PIN(DT_ALIAS(sw1), gpios),
 	BUTTON_PLAY_PAUSE = DT_GPIO_PIN(DT_ALIAS(sw2), gpios),
+#if defined(CONFIG_BOARD_NRF5340_AUDIO_DK_NRF5340_CPUAPP)


not addressed

applications/nrf5340_audio/boards/nrf5340_audio_dk_nrf5340_cpuapp.overlay

greg-fer

Thanks for adding the doxygen groups. I will check with @koffes on other channels if we want to publish the APIs for the apps somewhere. Until then, I consider the API doc updates complete.

Question remains about the update to sample.yaml for the nRF5340 DK that is not included in the doc preview. @gmarull , would you know if the preview needs to be purged and the doc build triggered again for the .. table-from-sample-yaml:: tag to pull the new entry in sample.yaml.platform_allow?

greg-fer · 2025-03-19T14:18:57Z

applications/nrf5340_audio/doc/requirements.rst

-.. table-from-rows:: /includes/sample_board_rows.txt
-   :header: heading
-   :rows: nrf5340_audio_dk_nrf5340
+.. table-from-sample-yaml::


This does not seem to work to include nRF5340 or the preview build for docs has gotten stale...

greg-fer · 2025-03-19T14:21:39Z

applications/nrf5340_audio/src/utils/audio_nrf5340_dk_init.h

+ */
+
+/** @file
+ *  @brief Board initialiszation function


Suggested change

* @brief Board initialiszation function

* @brief Board initialization function

applications/nrf5340_audio/boards/nrf5340_audio_dk_nrf5340_cpuapp_fota.overlay

applications/nrf5340_audio/boards/nrf5340dk_nrf5340_cpuapp.overlay

applications/nrf5340_audio/unicast_server/main.c

koffes · 2025-03-19T14:26:45Z

applications/nrf5340_audio/doc/user_interface.rst

+|               |                                                                                                           |                                             +------------------+------------+
+|               |                                                                                                           |                                             | nRF5340 Audio DK | nRF5340 DK |
+===============+===========================================================================================================+=============================================+==================+============+
+| **VOL-**      | Long-pressed during startup: Changes the headset to the left channel one.                                 | * :ref:`nrf53_audio_broadcast_sink_app`     |         Y        |      Y     |


How do you want this to look @greg-fer: Now the buttons are in the leftmost column. We could delete that column all together and put the button names in the two rightmost columns.

applications/nrf5340_audio/doc/user_interface.rst

koffes · 2025-03-19T14:34:03Z

applications/nrf5340_audio/doc/user_interface.rst

+--------------------------+-----------------------------------------------------------------------------------------------------------+---------------------------------------------+------------------+------------+
+| **LED3**                 | Blinking green - The nRF5340 application core is running.                                                 | All                                         |         Y        |      Y     |
+--------------------------+-----------------------------------------------------------------------------------------------------------+---------------------------------------------+------------------+------------+
+| **LED5**                 | Off - No PC connection available.                                                                         | All                                         |         N        |      Y     |


This LED also exists on the 5340 DK. Discuss as above if we should write "IFMCU" in the Audio DK column and "LED5" in the vanilla DK column.

Also, this info is not that crucial. @greg-fer, do we usually document this IFMCU/Segger behavior on each DK?

applications/nrf5340_audio/doc/user_interface.rst

koffes · 2025-03-19T14:36:57Z

applications/nrf5340_audio/doc/user_interface.rst

+|                          | Solid yellow - External hardware codec selected.                                                          |                                             |                  |            |
+|                          | This LED turns solid yellow also when the devices are reset, for the time then pins are floating.         |                                             |                  |            |
+--------------------------+-----------------------------------------------------------------------------------------------------------+---------------------------------------------+------------------+------------+
+| **FTDI SPI**             | Off - No data is written to the hardware codec using SPI.                                                 | All                                         |         Y        |      N     |


General: To ease readability: Do we need subrows for "y"/"n" within the same LED?

sonarqubecloud · 2025-03-20T09:39:26Z

Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

applications/nrf5340_audio/boards/nrf5340_audio_dk_nrf5340_cpuapp_fota.overlay

koffes · 2025-03-25T14:26:40Z

applications/nrf5340_audio/boards/nrf5340dk_nrf5340_cpuapp.overlay

+	i2s0_sleep: i2s0_sleep {
+		group1 {
+			psels = <NRF_PSEL(I2S_MCK, 0, 12)>,
+			<NRF_PSEL(I2S_SCK_M, 0, 14)>,


Check pin assignments with Rick.

The LED module is currently written in a way that’s highly dependent on all of the LEDs present on the audio DK, which prevents the application from running on the nRF5340 DK. The module must be refactored so that it can still show the state of the application with a different selection of LEDs. Currently the button handler expects kits to have five buttons - which is true for the Audio DK, but not for the nRF5340 DK. To have the audio application running on the nRF5340 DK (and other kits in the future), the handler must be refactored to not fail compilation when fewer buttons are present, and reduce the application's overall dependency on button availability. Signed-off-by: Graham Wacey <graham.wacey@nordicsemi.no>

gWacey requested review from a team as code owners February 19, 2025 10:57

gWacey requested review from koffes, rick1082 and alexsven February 19, 2025 10:57

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

nordicjm reviewed Feb 19, 2025

View reviewed changes

applications/nrf5340_audio/boards/nrf5340_audio_dk_nrf5340_cpuapp.overlay Show resolved Hide resolved

nordicjm requested changes Feb 19, 2025

View reviewed changes

gWacey force-pushed the OCT-3012-LED-module-for-kits-with-other-LED-setup branch from cc4d61d to 7fb59b9 Compare February 20, 2025 07:57

gWacey force-pushed the OCT-3012-LED-module-for-kits-with-other-LED-setup branch from 7fb59b9 to 1e62820 Compare February 20, 2025 07:59

gWacey force-pushed the OCT-3012-LED-module-for-kits-with-other-LED-setup branch 8 times, most recently from 6617c47 to 0c343b6 Compare February 27, 2025 11:21

koffes requested changes Feb 27, 2025

View reviewed changes

gWacey force-pushed the OCT-3012-LED-module-for-kits-with-other-LED-setup branch 2 times, most recently from 29d5d74 to 7929b99 Compare February 27, 2025 16:56

gWacey requested a review from nordicjm February 27, 2025 16:58

gWacey force-pushed the OCT-3012-LED-module-for-kits-with-other-LED-setup branch 3 times, most recently from 54eaf1b to 27b99ac Compare February 27, 2025 17:43

gWacey requested a review from a team as a code owner March 6, 2025 09:42

github-actions bot added the doc-required PR must not be merged without tech writer approval. label Mar 6, 2025

gWacey force-pushed the OCT-3012-LED-module-for-kits-with-other-LED-setup branch 4 times, most recently from 7f1fe4e to 7ef56ad Compare March 6, 2025 14:24

greg-fer requested changes Mar 7, 2025

View reviewed changes

gWacey force-pushed the OCT-3012-LED-module-for-kits-with-other-LED-setup branch from 7ef56ad to de8b1ca Compare March 17, 2025 08:09

gWacey mentioned this pull request Mar 17, 2025

Oct 3005 button handler working for kits with different buttons #20964

Closed

gWacey force-pushed the OCT-3012-LED-module-for-kits-with-other-LED-setup branch 3 times, most recently from d558ad2 to 56cb9fe Compare March 17, 2025 09:23

nordicjm reviewed Mar 17, 2025

View reviewed changes

gWacey force-pushed the OCT-3012-LED-module-for-kits-with-other-LED-setup branch 2 times, most recently from 1fbc5f5 to ace34ce Compare March 17, 2025 14:02

gWacey requested review from koffes and greg-fer March 17, 2025 20:23

gWacey force-pushed the OCT-3012-LED-module-for-kits-with-other-LED-setup branch 2 times, most recently from bd4db70 to a2dd41b Compare March 19, 2025 09:20

greg-fer reviewed Mar 19, 2025

View reviewed changes

koffes requested changes Mar 19, 2025

View reviewed changes

gWacey force-pushed the OCT-3012-LED-module-for-kits-with-other-LED-setup branch 3 times, most recently from 6f26f2d to 6978c97 Compare March 20, 2025 09:33

gWacey force-pushed the OCT-3012-LED-module-for-kits-with-other-LED-setup branch 2 times, most recently from 5d7cf2c to 9ae6604 Compare March 25, 2025 11:55

koffes requested changes Mar 25, 2025

View reviewed changes

gWacey force-pushed the OCT-3012-LED-module-for-kits-with-other-LED-setup branch from 9ae6604 to 3cd0f21 Compare March 25, 2025 14:34

	* @brief Board initialiszation function
	* @brief Board initialization function

		#ifndef _BOARD_H_
		#define _BOARD_H_

applications: nrf5340_audio: LED module for kits with other LED setup #20483

Are you sure you want to change the base?

applications: nrf5340_audio: LED module for kits with other LED setup #20483

Conversation

gWacey commented Feb 19, 2025 • edited Loading

NordicBuilder commented Feb 19, 2025 • edited Loading

CI Information

Inputs:

Sources:

Github labels

Outputs:

Toolchain

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

NordicBuilder commented Feb 19, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 20, 2025 • edited Loading

github-actions bot commented Feb 20, 2025

github-actions bot commented Feb 27, 2025 • edited Loading

greg-fer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greg-fer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Mar 20, 2025

Quality Gate passed

Choose a reason for hiding this comment

gWacey commented Feb 19, 2025 •

edited

Loading

NordicBuilder commented Feb 19, 2025 •

edited

Loading

github-actions bot commented Feb 20, 2025 •

edited

Loading

github-actions bot commented Feb 27, 2025 •

edited

Loading