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

[nrf fromlist] kconfig: shell: increase shell stack size for OpenThread #1560

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

maciejbaczmanski
Copy link
Member

Shell stack size is too low for OpenThread without joiner functionality, causing overflow. This commit increases it.

upstream PR: zephyrproject-rtos/zephyr#69783

Signed-off-by: Maciej Baczmanski [email protected]
(cherry picked from commit 4fb493e49fef245e62df21620382e027a7200907)

Copy link
Contributor

@jfischer-no jfischer-no left a comment

Choose a reason for hiding this comment

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

Does it fix stack size for NCS or native Zephyr?

@maciejbaczmanski
Copy link
Member Author

Does it fix stack size for NCS or native Zephyr?

For NCS. Currently shell stack size is too small for some variants causing overflow on openthread samples

@jfischer-no
Copy link
Contributor

Does it fix stack size for NCS or native Zephyr?

For NCS. Currently shell stack size is too small for some variants causing overflow on openthread samples

Then it is better to resolve it in NCS not in Zephyr, maybe like other application and samples in NCS do it. How many releases will the stack be sufficient and we see another fromlist patch?

@maciejbaczmanski
Copy link
Member Author

Does it fix stack size for NCS or native Zephyr?

For NCS. Currently shell stack size is too small for some variants causing overflow on openthread samples

Then it is better to resolve it in NCS not in Zephyr, maybe like other application and samples in NCS do it. How many releases will the stack be sufficient and we see another fromlist patch?

This is a bugfix. As it can be seen in changed file, value of stack size is not changed. Incorrect assigning of value for opentrhead with no joiner functionality has been removed. PR has been approved upstream. Resolving it in NCS would only cause doubling of configs with the same value.

@jfischer-no
Copy link
Contributor

Your commit message: "Shell stack size is too low for OpenThread without joiner
functionality, causing overflow. This commit increases it."

As it can be seen in changed file, value of stack size is not changed. Incorrect assigning of value for opentrhead with no joiner functionality has been removed.

Is it just me who sees two different statements?

Resolving it in NCS would only cause doubling of configs with the same value.

But you answer to my question "Does it fix stack size for NCS or native Zephyr?" is "For NCS.". According to you answer this value does not applies to native Zephyr RTOS but to NCS only, therefore is should be fixed in NCS.

Also changed here: 4b94355

@carlescufi
Copy link
Contributor

Does it fix stack size for NCS or native Zephyr?

For NCS. Currently shell stack size is too small for some variants causing overflow on openthread samples

Then it is better to resolve it in NCS not in Zephyr, maybe like other application and samples in NCS do it. How many releases will the stack be sufficient and we see another fromlist patch?

This is a bugfix. As it can be seen in changed file, value of stack size is not changed. Incorrect assigning of value for opentrhead with no joiner functionality has been removed. PR has been approved upstream. Resolving it in NCS would only cause doubling of configs with the same value.

I discussed with @maciejbaczmanski and we agreed that this does apply upstream. The commit message will be amended.

@SeppoTakalo SeppoTakalo requested a review from jfischer-no March 6, 2024 11:09
Shell stack size is too low for OpenThread without joiner
functionality, causing overflow.

In this commit, the value of stack size is not changed.
Incorrect assigning of value for opentrhead with
no joiner functionality has been removed.

Upstream PR: zephyrproject-rtos/zephyr#69783

Signed-off-by: Maciej Baczmanski <[email protected]>
(cherry picked from commit c346df93cf8d8a3be8a82fd2b23ae083dda64abb)
@jfischer-no
Copy link
Contributor

I discussed with @maciejbaczmanski and we agreed that this does apply upstream. The commit message will be amended.

okay. I will merged it asap depending on rebase process.

@jfischer-no jfischer-no dismissed their stale review March 7, 2024 07:49

addressed

@jfischer-no jfischer-no merged commit d10178e into nrfconnect:main Mar 7, 2024
10 checks passed
@maciejbaczmanski maciejbaczmanski deleted the ot_shell_size branch March 7, 2024 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Shell bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants