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

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gWacey
Copy link
Contributor

@gWacey gWacey commented Feb 19, 2025

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.

@gWacey gWacey requested review from a team as code owners February 19, 2025 10:57
@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 Feb 19, 2025
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Feb 19, 2025

CI Information

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

Inputs:

Sources:

sdk-nrf: PR head: 238384ffe16279bb78e2e04b75cf16dea2321cda

more details

sdk-nrf:

PR head: 238384ffe16279bb78e2e04b75cf16dea2321cda
merge base: 3e3b689a6dffdf867f61f9d2bc438bf7e5eb7de7
target head (main): 34740f13c763ed222fc0b9d5a7fce3352c59a00b
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 (24)
applications
│  ├── nrf5340_audio
│  │  ├── boards
│  │  │  ├── nrf5340_audio_dk_nrf5340_cpuapp.overlay
│  │  │  ├── nrf5340_audio_dk_nrf5340_cpuapp_fota.overlay
│  │  │  ├── nrf5340dk_nrf5340_cpuapp.conf
│  │  │  │ nrf5340dk_nrf5340_cpuapp.overlay
│  │  ├── broadcast_sink
│  │  │  │ main.c
│  │  ├── broadcast_source
│  │  │  │ main.c
│  │  ├── sample.yaml
│  │  ├── src
│  │  │  ├── audio
│  │  │  │  ├── audio_datapath.c
│  │  │  │  │ audio_system.c
│  │  │  ├── bluetooth
│  │  │  │  ├── bt_management
│  │  │  │  │  │ bt_mgmt.c
│  │  │  ├── modules
│  │  │  │  ├── button_assignments.h
│  │  │  │  ├── button_handler.c
│  │  │  │  ├── led.c
│  │  │  │  ├── led.h
│  │  │  │  │ led_assignments.h
│  │  │  ├── utils
│  │  │  │  ├── CMakeLists.txt
│  │  │  │  ├── Kconfig
│  │  │  │  ├── board_init.c
│  │  │  │  ├── board_init.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: 4cff34261a
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:4cff34261a_bece0367df

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

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ❌ Build twister
    • sdk-nrf test count: 476
  • ❌ 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_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
    • 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-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.

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

@@ -0,0 +1,39 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

commit message is indented when it should not be

<&gpio0 26 GPIO_ACTIVE_HIGH>;
label = "RGB LED 0";
};
rgb2: rgb-2 {
Copy link
Contributor

Choose a reason for hiding this comment

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

line spacing between child nodes

psels = <NRF_PSEL(I2S_MCK, 0, 12)>;
nordic,drive-mode = <NRF_DRIVE_H0H1>;
};
group2 {
Copy link
Contributor

Choose a reason for hiding this comment

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

apply throughout PR

/** @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)
Copy link
Contributor

Choose a reason for hiding this comment

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

use DT macros to check if node exists

@@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

as above


#include <zephyr/logging/log.h>
LOG_MODULE_REGISTER(led, CONFIG_MODULE_LED_LOG_LEVEL);
LOG_MODULE_REGISTER(led, 4); /* CONFIG_MODULE_LED_LOG_LEVEL); */
Copy link
Contributor

Choose a reason for hiding this comment

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

no

@@ -8,8 +8,8 @@ target_sources(app PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}/channel_assignment.c
${CMAKE_CURRENT_SOURCE_DIR}/error_handler.c
${CMAKE_CURRENT_SOURCE_DIR}/uicr.c
${CMAKE_CURRENT_SOURCE_DIR}/board_init.c
Copy link
Contributor

Choose a reason for hiding this comment

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

fix indent

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

Choose a reason for hiding this comment

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

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

Comment on lines +7 to +8
#ifndef _BOARD_H_
#define _BOARD_H_
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@gWacey 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
Copy link

github-actions bot commented Feb 20, 2025

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

Preview links for modified nRF Connect SDK documents:

https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/app_dev/device_guides/fem/fem_nrf21540_gpio.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/app_dev/device_guides/fem/fem_nrf21540_gpio_spi.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/app_dev/device_guides/fem/fem_nrf2220.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/app_dev/device_guides/fem/fem_simple_gpio.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/app_dev/device_guides/nrf54h/ug_nrf54h20_gs.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/app_dev/device_guides/nrf54h/ug_nrf54h20_keys.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/libraries/networking/downloader.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/protocols/lte/index.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/protocols/matter/index.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/2.4.99-cs3_to_2.6.99-cs2/migration_guide_2.4.99-cs3_to_2.6.99-cs2_application.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/2.4.99-cs3_to_2.6.99-cs2/migration_guide_2.4.99-cs3_to_2.6.99-cs2_environment.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/migration_guide_1.x_to_2.x.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/migration_guide_2.4.99-cs3_to_2.6.99-cs2.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/migration_guide_2.5.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/migration_guide_2.6.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/migration_guide_2.7.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/migration_guide_2.8.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/migration_guide_2.9.0-nRF54H20-1-rc2.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/migration_guide_2.9.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/migration_guide_3.0.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/migration_guide_nRF54H20_cs_to_2_7.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/migration_guide_nRF54H20_cs_to_2_7_99-cs1.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/migration_guide_nRF54H20_cs_to_2_7_99-cs2.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/migration_guide_spm_to_tf-m.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/migration_hwmv2.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/migration_sysbuild.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/nRF54H20_migration_2.7/migration_guide_2.4.99-cs3_to_2.7_application.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/nRF54H20_migration_2.7/migration_guide_2.6.99-cs2_to_2.7_application.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/nRF54H20_migration_2.7/migration_guide_2.6.99-cs2_to_2_7_environment.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration/nRF54H20_migration_2.7/transition_guide_2.4.99-cs3_to_2.7_environment.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/migration_guides.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/release_notes.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/releases_and_maturity/releases/release-notes-changelog.html
https://ncsdoc.z6.web.core.windows.net/PR-20483/nrf/samples/bluetooth/fast_pair/locator_tag/README.html

@gWacey 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
Copy link

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

@gWacey gWacey force-pushed the OCT-3012-LED-module-for-kits-with-other-LED-setup branch from 1e62820 to a50c58c Compare February 20, 2025 16:18
	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.

Signed-off-by: Graham Wacey <[email protected]>
@gWacey gWacey force-pushed the OCT-3012-LED-module-for-kits-with-other-LED-setup branch from a50c58c to 238384f Compare February 20, 2025 16:36
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.

3 participants