Re: RFC[2/2] Common logging infrastructure and API


Saucedo Tejada, Genaro <genaro.saucedo.tejada@...>
 

On Thu, 2016-02-25 at 07:54 -0800, Dirk Brandewie wrote:

On 02/25/2016 07:47 AM, Benjamin Walsh wrote:
On Thu, Feb 25, 2016 at 07:05:49AM -0800, Dirk Brandewie wrote:


On 02/24/2016 02:13 PM, Saucedo Tejada, Genaro wrote:
 From 9baee79d211bfb94aeed970c55f31cd3c4b2a8ad Mon Sep 17
00:00:00 2001
From: Genaro Saucedo Tejada <genaro.saucedo.tejada(a)intel.com>
Date: Fri, 19 Feb 2016 17:10:28 -0600
Subject: [PATCH] Log macros unified in a common API

Introduces a header to concentrate logging macro definitions
for all
code to
reuse, change aims to provide all currently existing logging
functionality so
every C file can replace it's compile-unit definitions by
common code.

Later enhancements to log can now be performed in a single
file.

Features:
* Optional printing of thread pointer
* Optional printing of colored messages
* Optional printing of a label to indicate logging level (info,
error,
warning, debug)
* Caller function name printing
* Incremental log levels
* One point log disable
I like this in general we need a common set of debug output
macros
that all drivers can use.  Currently we have a *bunch* of
different
versions of the same thing in use throughout the driver tree.

I don't like the naming, we already have a kernel event logging
API
people may assume that the two facilities are connected based on
the name.

Can we change the names to debug_* or something else?
Although a namespace is needed (as noted bellow) debug is not the only
goal of this infrastructure INFO and ERROR level might be used for big
production environments in the future.

I reused some existing Kconfig variables but, as Anas pointed out, they
should be updated to reflect the wider nature of logging, some of them
do control debugging and should be split.



Again, do we want to take over _another_ namespace ? Maybe it
should be
sys_debug or sys_dbg or sys_dbg_log ?

The sample implementation is adding all these in logging.h, which
is a
public API file:

   DBG, ERR, INF, WRN, THREAD_FN_CALL, IS_LOGGING_ACTIVE

which are very generic names. Don't forget that the application
writer
is operating in the same C namespace, and we have been trying to
keep
the namespaces the kernel owns to a minimum to minimize the number
or
potential clashes:
It's the MACRO names the need fixed up IMO.  Any functions that are
added should be in the sys_* space sure.  I didn't see any new C
functions being added.
I propose 'LOG_' as the prefix of all the function-like macros and
Kconfig variables, I'm using it for the improved patch set as place
holder anyway but expect to mass rename if, for example, it gets
decided to keep such name for the kernel event logger facility.

An actual name space would be needed for a run-time implementation of a
more complex logging facility, such as sys_log() instead of printf.



See doc/collaboration/code/naming_conventions.rst.

Join devel@lists.zephyrproject.org to automatically receive all group messages.