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

native_posix: kconfig: Set logging mode conditionally #34141

Merged
merged 1 commit into from
Apr 10, 2021

Conversation

keith-zephyr
Copy link
Collaborator

Projects based on native_posix get a warning indicating that the
LOG_MODE_IMMEDIATE choice symbol is selected but no symbol ended up as
the choice selection.

Only set the logging mode if logging is enabled.

Signed-off-by: Keith Short [email protected]

Copy link
Member

@aescolar aescolar left a comment

Choose a reason for hiding this comment

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

Thanks. It is a good thing to fix that warning. But it would seem you are not just fixing the warning, but changing the default CONFIG_LOG_MODE_IMMEDIATE to "no" in native_posix.
Would something like this below be better?

index ca8039b399..a226c26d60 100644
--- a/boards/posix/native_posix/Kconfig.defconfig
+++ b/boards/posix/native_posix/Kconfig.defconfig
@@ -45,7 +45,9 @@ config LOG_BACKEND_NATIVE_POSIX
 
 # For native_posix we can log synchronously without any problem
 # Doing so will be nicer for debugging
-config LOG_MODE_IMMEDIATE
+choice LOG_MODE
+       default LOG_MODE_IMMEDIATE
+endchoice
 
 endif # LOG

Projects based on native_posix get a warning indicating that the
LOG_MODE_IMMEDIATE choice symbol is selected but no symbol ended up as
the choice selection.

Only set the logging mode if logging is enabled.

Signed-off-by: Keith Short <[email protected]>
@keith-zephyr
Copy link
Collaborator Author

Thanks. It is a good thing to fix that warning. But it would seem you are not just fixing the warning, but changing the default CONFIG_LOG_MODE_IMMEDIATE to "no" in native_posix.
Would something like this below be better?

index ca8039b399..a226c26d60 100644
--- a/boards/posix/native_posix/Kconfig.defconfig
+++ b/boards/posix/native_posix/Kconfig.defconfig
@@ -45,7 +45,9 @@ config LOG_BACKEND_NATIVE_POSIX
 
 # For native_posix we can log synchronously without any problem
 # Doing so will be nicer for debugging
-config LOG_MODE_IMMEDIATE
+choice LOG_MODE
+       default LOG_MODE_IMMEDIATE
+endchoice
 
 endif # LOG

Thanks for the suggestion. Done.

coreboot-bot pushed a commit to coreboot/zephyr-cros that referenced this pull request Apr 10, 2021
Projects based on native_posix get a warning indicating that the
LOG_MODE_IMMEDIATE choice symbol is selected but no symbol ended up as
the choice selection.

Only set the logging mode if logging is enabled.

Signed-off-by: Keith Short <[email protected]>
(cherry picked from commit 91e5b75cbae7d575ff106cfed12395fe98867f9ai
zephyrproject-rtos/zephyr#34141)

BUG=none
TEST=zmake testall

Change-Id: Idc676551e4d010595e9f2be76e6bf4dd7fc140cb
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/2818867
Reviewed-by: Jack Rosenthal <[email protected]>
Reviewed-by: Simon Glass <[email protected]>
Commit-Queue: Simon Glass <[email protected]>
Tested-by: Simon Glass <[email protected]>
@aescolar aescolar merged commit 5a49116 into zephyrproject-rtos:master Apr 10, 2021
@keith-zephyr keith-zephyr deleted the posix-log-fix branch August 5, 2022 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Boards area: native port Host native arch port (native_sim)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants