Skip to content

Conversation

@nordic-krch
Copy link
Contributor

When log is locally disabled then structures associated with the module
are not created. Logger API macros were evaluating those structues even
when log was disabled for given module. That resulted in compilation
error. Macros has been improved to not touch those structures when
log is disabled for given module.

Signed-off-by: Krzysztof Chruscinski [email protected]

When log is locally disabled then structures associated with the module
are not created. Logger API macros were evaluating those structues even
when log was disabled for given module. That resulted in compilation
error. Macros has been improved to not touch those structures when
log is disabled for given module.

Signed-off-by: Krzysztof Chruscinski <[email protected]>
@codecov-io
Copy link

Codecov Report

Merging #8840 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #8840   +/-   ##
=======================================
  Coverage   52.32%   52.32%           
=======================================
  Files         195      195           
  Lines       24730    24730           
  Branches     5140     5140           
=======================================
  Hits        12941    12941           
  Misses       9715     9715           
  Partials     2074     2074
Impacted Files Coverage Δ
include/logging/log_core.h 100% <ø> (ø) ⬆️
include/logging/log_instance.h 80% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1952c56...b1c990d. Read the comment docs.

@nordic-krch
Copy link
Contributor Author

this PR should solve the same issue as #8828 tried to solve. However, it has advantage that it does not remove completely log API when it is disabled but keeps the C code underneath (but compiler removes it).

Copy link
Member

@jukkar jukkar left a comment

Choose a reason for hiding this comment

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

Thanks @nordic-krch, this fixed the issue I was seeing.

@nashif nashif merged commit 67d6987 into zephyrproject-rtos:master Jul 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants