logging: Add log initialization to system startup #8842
Conversation
There was a problem hiding this comment.
As @andrewboie proposed here: #8828 (comment), we could use static inline functions in order to not change behaviour when logger is enabled and disabled.
@nordic-krch; Have you tried this approach?
There was a problem hiding this comment.
macros gives me possibility to handle variable number of arguments without using va_list. but even when inline functions would be used I need to handle the case when some variables that macro/function access disappears when logger is disabled for given module.
There was a problem hiding this comment.
if the log is disabled you can just make empty variadic inline functions.
I really would rather see this with inline functions at some point in the layers so that variables defined specifically for use in a log point don't generate "defined but not used" warnings.
There was a problem hiding this comment.
This is ensured, if you look at macro __LOG() (log_core.h line 169) you can see that it always evaluates to if else clause. Condition in if clause evaluates to false on compile time if log is disabled for given module and log code is not included in the binary but you will never get 'defined but not used'.
subsys/logging/log_core.c
Outdated
There was a problem hiding this comment.
Is timestamp now mandatory? If not, why we have to have default function?
There was a problem hiding this comment.
it is mandatory. and actually there are 2 default functions. Dummy_timestamp is used between log_core_init() and log_init(). Then timestamp which is using k_cycle_get_32 is used and it can be later on replaced by a user function. k_cycle_get_32 cannot be used from the start because it may not be configured/available then.
subsys/logging/log_core.c
Outdated
There was a problem hiding this comment.
CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC could be 0. Are we supporting that?
There was a problem hiding this comment.
no, i will fix that in another PR.
Codecov Report
@@ Coverage Diff @@
## master #8842 +/- ##
==========================================
+ Coverage 52.31% 52.32% +0.01%
==========================================
Files 195 195
Lines 24732 24742 +10
Branches 5141 5142 +1
==========================================
+ Hits 12939 12947 +8
- Misses 9719 9721 +2
Partials 2074 2074
Continue to review full report at Codecov.
|
subsys/logging/log_core.c
Outdated
subsys/logging/log_core.c
Outdated
There was a problem hiding this comment.
Please introduce variables in the beginning of func
subsys/logging/log_core.c
Outdated
There was a problem hiding this comment.
This func can be called multiple times now. Will it cause any issues if done so?
There was a problem hiding this comment.
added protection
Log API can be used before user can explicitly initialize the logger. In order to ensure that logger core is ready to buffer log messages it must be initialize as early as possible. Initialization does not include initialization of default backend since driver may not be ready and backend is needed only when log messages are processed. Signed-off-by: Krzysztof Chruscinski <[email protected]>
nordic-krch
force-pushed
the
logger_preinit
branch
from
be90ad2 to
adb82ad
Compare
7 participants
Log API can be used before user can explicitly initialize the logger.
In order to ensure that logger core is ready to buffer log messages
it must be initialize as early as possible. Initialization does not
include initialization of default backend since driver may not be
ready and backend is needed only when log messages are processed.
PR replaces #8742
Signed-off-by: Krzysztof Chruscinski [email protected]