applications: nrf_desktop: Disable advertising when connected via USB #23006

alstrzebonski · 2025-06-26T13:45:00Z

Disables advertising when connected via USB to improve USBHS report rate. To achieve that the new suspend event is added in CAF that allows for suspending modules. It also adds advertising controller that controls suspending/resuming advertising.

Documentation will be done is separate PR: https://nordicsemi.atlassian.net/browse/NCSDK-34237.

Add events used to temporarily suspend an application module. Change also adds a new module state: MODULE_STATE_SUSPENDED. Signed-off-by: Marek Pieta <[email protected]>

An application module may be suspended during power down sequence. Make sure that this would not break application power down sequence. Jira: NCSDK-24220 Signed-off-by: Marek Pieta <[email protected]>

Allow to suspend module in runtime. Signed-off-by: Marek Pieta <[email protected]>

Change adds a small application module that suspends BLE advertising while USB HID is active. Jira: NCSDK-24220 Signed-off-by: Marek Pieta <[email protected]>

Forces power down on USB disconnected event for consistency as it can also be forced on disconnection by USB suspended event. Adds option to delay system off after USB disconnection by few seconds to allow user to take actions like restart advertising without going through reboot. Jira: NCSDK-34092 Signed-off-by: Aleksander Strzebonski <[email protected]>

Suspends Bluetooth advertising when USBHS is connected to improve USBHS report rate. Jira: NCSDK-17701 Signed-off-by: Aleksander Strzebonski <[email protected]>

NordicBuilder · 2025-06-26T13:48:41Z

CI Information

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

Inputs:

Sources:

sdk-nrf: PR head: 4c1ab50cd73c21025dbae61cace3ba5b8f333866

more details

sdk-nrf:

PR head: 4c1ab50cd73c21025dbae61cace3ba5b8f333866
merge base: 65ece6ad9db702456664d9e6d856483541705a85
target head (main): 16275c5a70d8c34805215c50b0b0d31d5b955c8d
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 (20)

applications
│  ├── nrf_desktop
│  │  ├── configuration
│  │  │  ├── nrf54h20dk_nrf54h20_cpuapp
│  │  │  │  ├── prj.conf
│  │  │  │  │ prj_release.conf
│  │  │  ├── nrf54lm20pdk_nrf54lm20a_cpuapp
│  │  │  │  ├── prj.conf
│  │  │  │  │ prj_release.conf
│  │  ├── src
│  │  │  ├── modules
│  │  │  │  ├── CMakeLists.txt
│  │  │  │  ├── Kconfig
│  │  │  │  ├── Kconfig.ble_adv_ctrl
│  │  │  │  ├── Kconfig.usb_state
│  │  │  │  ├── ble_adv_ctrl.c
│  │  │  │  │ usb_state_pm.c
include
│  ├── caf
│  │  ├── events
│  │  │  ├── module_state_event.h
│  │  │  │ module_suspend_event.h
subsys
│  ├── caf
│  │  ├── events
│  │  │  ├── CMakeLists.txt
│  │  │  ├── Kconfig
│  │  │  ├── Kconfig.module_suspend_event
│  │  │  ├── module_state_event.c
│  │  │  │ module_suspend_event.c
│  │  ├── modules
│  │  │  ├── Kconfig.ble_adv
│  │  │  ├── ble_adv.c
│  │  │  │ power_manager.c

Outputs:

Toolchain

Version: 3ae5dc3c63
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:3ae5dc3c63_776d264d2e

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

◻️ Toolchain - Skipped: existing toolchain is used
✅ Build twister
- sdk-nrf test count: 324
✅ Integration tests
- ✅ desktop52_verification
- ✅ test-fw-nrfconnect-apps
- ✅ test-fw-nrfconnect-nrf-iot_cloud

Disabled integration tests

- test-fw-nrfconnect-nrf_lrcs_mosh
- test-fw-nrfconnect-nrf_lrcs_positioning
- doc-internal
- test_ble_nrf_config
- 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_libmodem-nrf
- test-fw-nrfconnect-nrf-iot_lwm2m
- 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-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-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

pdunaj · 2025-06-27T02:24:34Z

applications/nrf_desktop/src/modules/ble_adv_ctrl.c

@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2024 Nordic Semiconductor ASA


pdunaj · 2025-06-27T02:29:30Z

applications/nrf_desktop/src/modules/usb_state_pm.c

@@ -58,7 +72,23 @@ static bool app_event_handler(const struct app_event_header *aeh)
 			power_manager_restrict(MODULE_IDX(MODULE), POWER_MANAGER_LEVEL_ALIVE);
 			break;
 		case USB_STATE_DISCONNECTED:
-			power_manager_restrict(MODULE_IDX(MODULE), POWER_MANAGER_LEVEL_MAX);
+			if (CONFIG_DESKTOP_USB_PM_RESTRICT_REMOVE_DELAY_MS) {


the code LGTM but it's overall a bit complex.
I suggest to break it into subfunctions.
Maybe in a separate PR to simplify this review (i.e., avoid adding new functionality and moving code at the same time).

pdunaj · 2025-06-27T02:37:32Z

subsys/caf/modules/ble_adv.c

LGTM but one thing is missing - we need to count the number of suspend and resume request, in case more then one client would like to control the state. Or alternatively we could describe that only one can do this (plus maybe assert if we get double suspend). Personally I think we should refcount.

BTW - please do not add ref counter yet. To it in a separate PR to not make this change more complicated. Let's focus on suspending first.

I think that using module bitmask (similarly to power_manager_restrict_event) might allow multiple application modules to suspend a given module. It would be even better than refcount as a module would not be able to accidentally resume from suspended enforced by other application module.

pdunaj · 2025-06-27T02:38:26Z

subsys/caf/events/module_suspend_event.c

@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2024 Nordic Semiconductor ASA


dates in new files

MarekPieta

Still reviewing CAF BLE advertising module part

MarekPieta · 2025-06-27T06:03:16Z

include/caf/events/module_suspend_event.h

@@ -0,0 +1,46 @@
+/*


Consider adding missing Jira references in commit messages

MarekPieta · 2025-06-27T06:05:40Z

include/caf/events/module_suspend_event.h

+	/** Event header. */
+	struct app_event_header header;
+
+	/** ID of the module. */


Consider also adding source_id (ID of module that is source of the request). Also for module_resume_req_event. That would allow to support more than one client in the future. It might also simplify debugging now.

MarekPieta · 2025-06-27T06:09:55Z

include/caf/events/module_suspend_event.h

+ */
+
+
+#include <zephyr/sys/atomic.h>


Is #include <zephyr/sys/atomic.h> needed here?

MarekPieta · 2025-06-27T06:12:56Z

subsys/caf/events/module_suspend_event.c

+ */
+
+#include <stdio.h>
+#include <assert.h>


Do we need assert.h and zephyr/sys/util.h?

MarekPieta · 2025-06-27T06:14:49Z

subsys/caf/events/module_suspend_event.c

+{
+	const struct module_suspend_req_event *event = cast_module_suspend_req_event(aeh);
+
+	APP_EVENT_MANAGER_LOG(aeh, "module_suspend_req: %s", module_name_get(event->module_id));


Since we have CONFIG_APP_EVENT_MANAGER_LOG_EVENT_TYPE, there is no need to explicitly log event type

MarekPieta · 2025-06-27T06:45:33Z

applications/nrf_desktop/src/modules/usb_state_pm.c


 		if (IS_ENABLED(CONFIG_DESKTOP_USB_PM_REQ_NO_PM_LATENCY)) {
 			update_pm_policy_latency_req(event->state == USB_STATE_ACTIVE);
 		}

+		if (CONFIG_DESKTOP_USB_PM_RESTRICT_REMOVE_DELAY_MS) {


Consider:

Suggested change

if (CONFIG_DESKTOP_USB_PM_RESTRICT_REMOVE_DELAY_MS) {

if (CONFIG_DESKTOP_USB_PM_RESTRICT_REMOVE_DELAY_MS > 0) {

MarekPieta · 2025-06-27T06:46:53Z

applications/nrf_desktop/src/modules/usb_state_pm.c

+				 * take actions like e.g. restart advertising without going
+				 * through system off.
+				 */
+				k_work_schedule(


Missing (void) cast

MarekPieta · 2025-06-27T06:49:03Z

applications/nrf_desktop/src/modules/usb_state_pm.c

+			 * down for application to behave consistently even if the
+			 * USB_STATE_SUSPENDED event doesn't appear.
+			 */
+			force_power_down();


What do you think about separating 2 small functions (for force power down handling and applying power manager restrictions - both based on USB state)? Similarly to update_pm_policy_latency_req.

That might improve readability here I think (as @pdunaj mentioned, the function is already quite complex)

MarekPieta · 2025-06-27T06:54:43Z

applications/nrf_desktop/src/modules/Kconfig.usb_state

+	help
+	  Delay in milliseconds before removing power manager level restriction.
+	  This is used to allow the USB to be disconnected without immediate system off.
+	  It allows user to take actions, like e.g. restart advertising, after disconnecting


restart BLE advertising?

MarekPieta · 2025-06-27T06:57:31Z

applications/nrf_desktop/src/modules/usb_state_pm.c


 		if (IS_ENABLED(CONFIG_DESKTOP_USB_PM_REQ_NO_PM_LATENCY)) {
 			update_pm_policy_latency_req(event->state == USB_STATE_ACTIVE);
 		}

+		if (CONFIG_DESKTOP_USB_PM_RESTRICT_REMOVE_DELAY_MS) {
+			/* Cancel the work if it is scheduled. */
+			k_work_cancel_delayable(&pm_restrict_remove_work);


Can we cancel the work only if state != USB_STATE_DISCONNECTED?

MarekPieta

One more comment regarding the CAF BLE advertising module

MarekPieta · 2025-06-27T08:59:41Z

subsys/caf/modules/ble_adv.c

@@ -970,7 +1043,7 @@ static bool handle_ble_peer_operation_event(const struct ble_peer_operation_even
 		}

 		if (event->op == PEER_OPERATION_ERASE_ADV) {
-			update_peer_is_rpa(PEER_RPA_ERASED);
+			store_peer_is_rpa(PEER_RPA_ERASED);


I wonder if we shouldn't call the store_peer_is_rpa also while suspended/off (as we update cur_identity)

MarekPieta and others added 6 commits June 26, 2025 15:16

caf: events: Add module_suspend_req_event and module_resume_req_event

b958567

Add events used to temporarily suspend an application module. Change also adds a new module state: MODULE_STATE_SUSPENDED. Signed-off-by: Marek Pieta <[email protected]>

caf: power_manager: Allow to suspend module during power down

bac3968

An application module may be suspended during power down sequence. Make sure that this would not break application power down sequence. Jira: NCSDK-24220 Signed-off-by: Marek Pieta <[email protected]>

caf: ble_adv: Add suspended states

c8a7916

Allow to suspend module in runtime. Signed-off-by: Marek Pieta <[email protected]>

applications: nrf_desktop: Add BLE advertising control module

1ac772f

Change adds a small application module that suspends BLE advertising while USB HID is active. Jira: NCSDK-24220 Signed-off-by: Marek Pieta <[email protected]>

applications: nrf_desktop: Suspend advertising when using USBHS

4c1ab50

Suspends Bluetooth advertising when USBHS is connected to improve USBHS report rate. Jira: NCSDK-17701 Signed-off-by: Aleksander Strzebonski <[email protected]>

alstrzebonski requested review from a team as code owners June 26, 2025 13:45

github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jun 26, 2025

alstrzebonski removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jun 26, 2025

pdunaj approved these changes Jun 27, 2025

View reviewed changes

MarekPieta requested changes Jun 27, 2025

View reviewed changes

		@@ -0,0 +1,71 @@
		/*
		* Copyright (c) 2024 Nordic Semiconductor ASA

		@@ -0,0 +1,40 @@
		/*
		* Copyright (c) 2024 Nordic Semiconductor ASA

	if (CONFIG_DESKTOP_USB_PM_RESTRICT_REMOVE_DELAY_MS) {
	if (CONFIG_DESKTOP_USB_PM_RESTRICT_REMOVE_DELAY_MS > 0) {

		*/


		#include <zephyr/sys/atomic.h>

applications: nrf_desktop: Disable advertising when connected via USB #23006

Are you sure you want to change the base?

applications: nrf_desktop: Disable advertising when connected via USB #23006

Uh oh!

Conversation

alstrzebonski commented Jun 26, 2025

Uh oh!

NordicBuilder commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Information

Inputs:

Sources:

Github labels

Outputs:

Toolchain

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarekPieta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarekPieta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NordicBuilder commented Jun 26, 2025 •

edited

Loading