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.

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

shell: kernel: fix reset command echo abrupt termination #35326

Merged
merged 1 commit into from
May 18, 2021
Merged

shell: kernel: fix reset command echo abrupt termination #35326

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions subsys/shell/modules/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,18 @@ config KERNEL_SHELL
This shell provides access to basic kernel data like version, uptime
and other useful information.

config KERNEL_SHELL_REBOOT_DELAY
int "Delay between reception of shell reboot command and reboot (ms)"
depends on KERNEL_SHELL
depends on REBOOT
default 0
help
This delay allows time for the shell to successfully echo the reboot
command input before the reboot abruptly terminates it. This can help
external systems that interact with the shell and require the reboot
command's echo to successfully complete to synchronise with the
device.

config DEVICE_SHELL
bool "Enable device shell"
default y if !SHELL_MINIMAL
Expand Down
6 changes: 6 additions & 0 deletions subsys/shell/modules/kernel_service.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,9 @@ static int cmd_kernel_reboot_warm(const struct shell *shell,
{
ARG_UNUSED(argc);
ARG_UNUSED(argv);
#if (CONFIG_KERNEL_SHELL_REBOOT_DELAY > 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

To be considered if IS_ENABLED is better. @nordic-krch any thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

IS_ENABLED cannot be used because it is not binary value you might do

if (CONFIG_KERNEL_SHELL_REBOOT_DELAY > 0) {
   k_sleep();
}

but compilation will fail when CONFIG_REBOOT is not defined. So i guess it should stay as is.

k_sleep(K_MSEC(CONFIG_KERNEL_SHELL_REBOOT_DELAY));
#endif
sys_reboot(SYS_REBOOT_WARM);
return 0;
}
Expand All @@ -229,6 +232,9 @@ static int cmd_kernel_reboot_cold(const struct shell *shell,
{
ARG_UNUSED(argc);
ARG_UNUSED(argv);
#if (CONFIG_KERNEL_SHELL_REBOOT_DELAY > 0)
k_sleep(K_MSEC(CONFIG_KERNEL_SHELL_REBOOT_DELAY));
#endif
sys_reboot(SYS_REBOOT_COLD);
return 0;
}
Expand Down