shell: Fix shell init procedure when configured as inactive on startup #67400
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jakub-uC
force-pushed
the
shell-fix-locked-start-feature
branch
2 times, most recently
from
76f2714 to
08a27a0
Compare
jakub-uC
added
bug
nordic-krch
reviewed
Jan 10, 2024
The previous behavior of the CONFIG_SHELL_AUTOSTART option, where setting it to 'n' disabled shell interaction at startup but kept the logger active as a shell backend, was inconsistent. Now, when CONFIG_SHELL_AUTOSTART is set to 'n', both the shell and the logger are inactive at startup. Calling the shell_start function will activate them and print any pending logs. This change ensures a more consistent and logical behavior regarding shell and logger activity at startup. Additionally, now when the shell_stop function is called, both the shell and the logger are halted. This enhancement ensures that stopping the shell also effectively stops the associated logger. Signed-off-by: Jakub Rzeszutko <[email protected]>
jakub-uC
force-pushed
the
shell-fix-locked-start-feature
branch
from
08a27a0 to
68097cc
Compare
|
@nordic-krch @henrikbrixandersen please let me know if there is anything else to do? |
nordic-krch
approved these changes
Jan 18, 2024
|
@carlescufi I think we can merge this one. |
henrikbrixandersen
approved these changes
Jan 21, 2024
kartben
reviewed
Jan 22, 2024
| @@ -798,6 +799,9 @@ struct shell_ctx { | |||
| /** When bypass is set, all incoming data is passed to the callback. */ | |||
| shell_bypass_cb_t bypass; | |||
|
|
|||
| /*!< Logging level for a backend. */ | |||
There was a problem hiding this comment.
this should be:
Suggested change
| /*!< Logging level for a backend. */ | |
| /** Logging level for a backend. */ |
Any chance you could follow up with a commit to fix this? given this is API, it's always nicer if documentation is accurate there. Thanks!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The previous behavior of the
CONFIG_SHELL_AUTOSTARToption, where setting it to 'n' disabled shell interaction at startup but kept the logger active as a shell backend, was inconsistent.Now, when
CONFIG_SHELL_AUTOSTARTis set to 'n', both the shell and the logger are inactive at startup. Calling theshell_starfunction will activate them and print any pending logs. This change ensures a more consistent and logical behavior regarding shell and logger activity at startup.Additionally, now when the
shell_stopfunction is called, both the shell and the logger are halted.