-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Reset/ncsdk 36564 to main #26163
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
base: main
Are you sure you want to change the base?
Reset/ncsdk 36564 to main #26163
Conversation
Added hook implementation for disabling Bluetooth in the hook of MCUmgr OS reboot command. Disabling of bluetooth allow to restart it properly after the warm boot. Ref: NCSDK-36564 Signed-off-by: Andrzej Puzdrowski <[email protected]>
Postpone disabling till half of waiting of reset time. Ref: NCSDK-36564 Signed-off-by: Andrzej Puzdrowski <[email protected]>
There was a problem hiding this 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 implements a hook for the MCUmgr OS reboot command to properly disable Bluetooth before warm boot on nRF54LX devices. This ensures Bluetooth can restart correctly after reboot.
Key Changes:
- Added hook implementation that disables Bluetooth and uninitializes MPSL before system reboot
- Configured the hook to be automatically enabled for nRF54LX devices when Bluetooth or MPSL is present
- Integrated the new source file into the build system
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| subsys/mgmt/mcumgr/grp/os_mgmt/src/os_mgmt_reboot_bt.c | New hook implementation that disables Bluetooth/MPSL before reboot, with delayed work handler for multithreading support |
| subsys/mgmt/mcumgr/grp/os_mgmt/Kconfig | Added configuration option to enable the reboot hook for nRF54LX devices with BT/MPSL |
| subsys/mgmt/mcumgr/grp/os_mgmt/CMakeLists.txt | Added build rule to include the new source file when the feature is enabled |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return MGMT_CB_ERROR_RC; | ||
| } | ||
| #ifdef CONFIG_MULTITHREADING | ||
| /* disable bluetooth from the system workqueue thread. */ |
Copilot
AI
Dec 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The magic number division by 2 lacks explanation. Add a comment explaining why half the reset timeout is used for scheduling Bluetooth disable, or define it as a named constant like CONFIG_MCUMGR_GRP_OS_RESET_MS_HALF to improve code clarity.
| /* disable bluetooth from the system workqueue thread. */ | |
| /* disable bluetooth from the system workqueue thread. */ | |
| /* | |
| * Schedule Bluetooth disable at half the reset timeout. | |
| * This ensures Bluetooth is disabled well before the actual reset occurs, | |
| * allowing time for clean shutdown and resource release. | |
| */ |
| static struct mgmt_callback cmd_reboot_bt_info_cb = { | ||
| .callback = reboot_bt_hook, | ||
| .event_id = MGMT_EVT_OP_OS_MGMT_RESET, | ||
| }; | ||
|
|
||
| static int os_mgmt_register_reboot_bt(void) | ||
| { | ||
| mgmt_callback_register(&cmd_reboot_bt_info_cb); |
Copilot
AI
Dec 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The variable name cmd_reboot_bt_info_cb is misleading as it suggests an info callback, but this handles the reboot hook. Consider renaming to reboot_bt_hook_cb or os_mgmt_reboot_bt_cb for clarity.
| static struct mgmt_callback cmd_reboot_bt_info_cb = { | |
| .callback = reboot_bt_hook, | |
| .event_id = MGMT_EVT_OP_OS_MGMT_RESET, | |
| }; | |
| static int os_mgmt_register_reboot_bt(void) | |
| { | |
| mgmt_callback_register(&cmd_reboot_bt_info_cb); | |
| static struct mgmt_callback reboot_bt_hook_cb = { | |
| .callback = reboot_bt_hook, | |
| .event_id = MGMT_EVT_OP_OS_MGMT_RESET, | |
| }; | |
| static int os_mgmt_register_reboot_bt(void) | |
| { | |
| mgmt_callback_register(&reboot_bt_hook_cb); |
| } | ||
| #ifdef CONFIG_MULTITHREADING | ||
| /* disable bluetooth from the system workqueue thread. */ | ||
| k_work_schedule(&bt_disable_work, K_MSEC(CONFIG_MCUMGR_GRP_OS_RESET_MS/2)); |
Copilot
AI
Dec 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add parentheses around the division operation: K_MSEC((CONFIG_MCUMGR_GRP_OS_RESET_MS/2)). This improves readability and prevents potential issues with macro expansion.
| k_work_schedule(&bt_disable_work, K_MSEC(CONFIG_MCUMGR_GRP_OS_RESET_MS/2)); | |
| k_work_schedule(&bt_disable_work, K_MSEC((CONFIG_MCUMGR_GRP_OS_RESET_MS/2))); |
CI InformationTo view the history of this post, click the 'edited' button above Inputs:Sources:sdk-nrf: PR head: 641b87d9863ddc078cfdaef6ba0db37e85533868 more detailssdk-nrf:
Github labels
List of changed files detected by CI (3)Outputs:ToolchainVersion: 43683a87ea Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
Memory footprint analysis revealed the following potential issuesapplications.nrf_desktop.zdebug_fast_pair.gmouse.uart[nrf54l15dk/nrf54l15/cpuapp]: ROM size increased by 1392[B] in comparison to the main[4cefdfc] branch. - link (cc: @nrfconnect/ncs-si-bluebagel) Note: This message is automatically posted and updated by the CI (latest/sdk-nrf/PR-26163/2) |
CONFIG_BT_UNINIT_MPSL_ON_DISABLE is enabled MPSL is uninitialized automatically once BT get uninitialized. This patch ensure that MPSL is uninitialized only once. Signed-off-by: Andrzej Puzdrowski <[email protected]>
Added hook implementation for disabling Bluetooth in the hook of MCUmgr OS reboot command.
Disabling of bluetooth allow to restart it properly after the warm boot.
Refs: NCSDK-36564, #28118
This is backport of v3.2.0 bugfix