Skip to content

Conversation

@Olivier-ProGlove
Copy link
Contributor

Migrate from SYS_LOG to LOG logging mechanism.

Signed-off-by: Olivier Martin [email protected]

@codecov-io
Copy link

codecov-io commented Aug 9, 2018

Codecov Report

Merging #9358 into master will increase coverage by 1.12%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9358      +/-   ##
==========================================
+ Coverage   52.53%   53.66%   +1.12%     
==========================================
  Files         212      216       +4     
  Lines       25821    33720    +7899     
  Branches     5428     7942    +2514     
==========================================
+ Hits        13565    18095    +4530     
- Misses      10071    12869    +2798     
- Partials     2185     2756     +571
Impacted Files Coverage Δ
lib/posix/clock.c 78.57% <0%> (-21.43%) ⬇️
subsys/net/lib/coap/coap_link_format.c 14.74% <0%> (-2.07%) ⬇️
subsys/net/ip/tcp_internal.h 48.27% <0%> (-1.73%) ⬇️
include/net/net_pkt.h 84.14% <0%> (-1.51%) ⬇️
include/misc/stack.h 90.9% <0%> (-1.4%) ⬇️
lib/mempool/mempool.c 95.07% <0%> (-0.55%) ⬇️
include/logging/log_msg.h 81.45% <0%> (-0.37%) ⬇️
subsys/net/ip/icmpv4.c 38.09% <0%> (-0.26%) ⬇️
subsys/net/l2/ethernet/arp.c 53.87% <0%> (-0.19%) ⬇️
include/kernel.h 98.38% <0%> (-0.06%) ⬇️
... and 62 more

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 4b9d980...d157d38. Read the comment docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

newline not needed. logger adds it.

Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor

Choose a reason for hiding this comment

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

What about nrf_power? Such name will suggest where look for the code which sent the message.

Copy link
Member

Choose a reason for hiding this comment

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

or nrf_soc_power

@ioannisg
Copy link
Member

@Olivier-ProGlove just wondering: power.c is the only Nordic SOC file that contains log messages?

Copy link
Member

@ioannisg ioannisg left a comment

Choose a reason for hiding this comment

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

LGTM. I am wondering whether other Nordic SOC source files have SYS log messages that should migrate to the new logging sub system.

@Olivier-ProGlove Olivier-ProGlove force-pushed the upstream-logging-nordic branch from 1993fe7 to 394d729 Compare August 14, 2018 08:58
@Olivier-ProGlove
Copy link
Contributor Author

@pizi-nordic @nordic-krch @ioannisg Comments addressed!

@Olivier-ProGlove
Copy link
Contributor Author

@ioannisg I forgot to answer your question, here is the grep on my pull-request rebased on the current head of master:

$ grep -r LOG arch/arm/soc/
arch/arm/soc/nxp_imx/mcimx7_m4/soc_clk_freq.c:		hz = CCM_ANALOG_GetSysPllFreq(CCM_ANALOG) >> 2;
arch/arm/soc/nxp_kinetis/kwx/wdog.S:_ASM_FILE_PROLOGUE
arch/arm/soc/nxp_kinetis/k6x/wdog.S:_ASM_FILE_PROLOGUE
arch/arm/soc/nordic_nrf/nrf52/power.c:#define LOG_MODULE_NAME nrf_soc_power
arch/arm/soc/nordic_nrf/nrf52/power.c:#define LOG_LEVEL LOG_LEVEL_DEBUG
arch/arm/soc/nordic_nrf/nrf52/power.c:LOG_MODULE_REGISTER();
arch/arm/soc/nordic_nrf/nrf52/power.c:		LOG_ERR("Unsupported State");
arch/arm/soc/nordic_nrf/nrf52/power.c:		LOG_ERR("Unsupported State");
arch/arm/soc/nordic_nrf/nrf52/power.c:		LOG_ERR("Unsupported State");
arch/arm/soc/ti_lm3s6965/soc.h:#define IRQ_ANALOG_COMP0 25
arch/arm/soc/ti_lm3s6965/soc.h:#define IRQ_ANALOG_COMP1 26

Actually looking at the result, the line #define LOG_LEVEL LOG_LEVEL_DEBUG is not needed. I will remove it in the following pull-request.

Migrate from SYS_LOG to LOG logging mechanism.

Signed-off-by: Olivier Martin <[email protected]>
Copy link
Member

@ioannisg ioannisg left a comment

Choose a reason for hiding this comment

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

Approving this. @Olivier-ProGlove have you tested it, btw?

@ioannisg ioannisg added the platform: nRF Nordic nRFx label Aug 17, 2018
@nashif nashif added this to the v1.14.0 milestone Sep 6, 2018
@nashif
Copy link
Member

nashif commented Oct 8, 2018

addressed in #10029

@nashif nashif closed this Oct 8, 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.

7 participants