2

I'm working on embedded system, where I use printf to create log on UART.

I would like to create a debug source file where I can set the type of debug I need.

I defined this constant:

  • DEBUG_LOG_0 for system log
  • DEBUG_LOG_1 for system debugging
  • DEBUG_LOG_2 for system advanced debugging

Starting from this constant I defined this macro to wrap the standard printf:

    /* Define for debugging level 0 - System Logs */
#ifdef DEBUG_LEVEL_0
#define edi_Print_L0(...) printf(__VA_ARGS__)
#endif

#ifndef DEBUG_LEVEL_0
#define edi_Print_L0(...) printf(...)
#endif

/* Define for debugging level 1 - Debug */
#ifdef DEBUG_LEVEL_1
#define edi_Print_L0(...) printf(__VA_ARGS__)
#define edi_Print_L1(...) printf(__VA_ARGS__)
#endif

#ifndef DEBUG_LEVEL_1
#define edi_Print_L0(...) printf(...)
#define edi_Print_L1(...) printf(...)
#endif

/* Define for debugging level 2 - Advanced Debug */   
#ifdef DEBUG_LEVEL_2
#define edi_Print_L0(...) printf(__VA_ARGS__)
#define edi_Print_L1(...) printf(__VA_ARGS__)
#define edi_Print_L2(...) printf(__VA_ARGS__)
#endif

#ifndef DEBUG_LEVEL_2
#define edi_Print_L0(...) printf(...)
#define edi_Print_L1(...) printf(...)
#define edi_Print_L2(...) printf(...)
#endif

Next I would import the debug constant from a Header file in order to enable the selected level of debug.

Any suggestion about the macro definition? Is there a smart way to achieve my scope?

Thanks!

Federico
  • 1,117
  • 6
  • 20
  • 37
  • 1
    Generally I would advise to forget all about stdio and roll out your own simple debug routines. The overhead that stdio comes with typically slaughter all free memory and CPU performance in any embedded system. On top of that, it is a dangerous library with non-existent type safety. – Lundin Jun 15 '16 at 15:04
  • `printf` is not really debugging. In addition to what @Lundin wrote: use a debugger for debugging. However that UART is handled, it will eventually either loose text or block. Not to talk about how to `printf` from interrupt handlers. – too honest for this site Jun 15 '16 at 22:09

4 Answers4

3

It would make more sense to pass the log level/source throughout the code, and then simply disable/enable individual levels or sources at one place.

I.e. in your code, you would use:

Log(Log_Comm, LevelDebug, "Some minor stuff");
Log(Log_Comm, LevelWarn, "Something strange");
Log(Log_Comm, LevelError, "Something seriously wrong");
Log(Log_System, LevelDebug, "Some minor stuff");
Log(Log_System, LevelWarn, "Something strange");
Log(Log_System, LevelError, "Something seriously wrong");

And then you simply have:

// log levels
#define LevelDebug 0x01
#define LevelInfo  0x02
#define LevelWarn  0x04
#define LevelError 0x08
#define LevelAll   0x0F

// enabled levels for individual log sources
#define Log_Comm    (LevelWarn | LevelError)
#define Log_System  (LevelAll)

#define Log(source, level, message) do { \
   if (source & level) { \
        sendToPort(message); \
   } \
} while (0)

(edit)

As pointed out by @Clifford in comments, there might also be a need to globally disable a certain level, without having to go through all source defines. This can be done by specifying an additinal mask:

// if LevelDebug is omitted from this mask,
// debug message will not be logged regardless
// of individual source settings
#define Global_Level_Mask (LevelWarn | LevelError)

#define Log(source, level, message) do { \
    if (source & level & Global_Level_Mask) { \
       sendToPort(message); \
    } \
} while (0)

One additional concern might be a number of "unreachable code" warnings this will produce around your code. I am not sure how to fix this in other compilers, but, for example in Visual Studio, it can be resolved by adding a pragma around the if statement:

// visual studio will show a warning 
// C4127: "conditional expression is constant"
// when compiling with all warnings enabled (-w4)

// these pragmas will disable the warning around if's
#define Log(source, level, message) do { \
__pragma(warning(push)) \
__pragma(warning(disable:4127)) \
    if (source & level & Global_Level_Mask) { \
__pragma(warning(pop)) \
        sendToPort(message); \
    } \
} while (0)

Not it looks a bit uglier, but IMHO it allows easy usage and much more control.

Of course, nothing stops you from having a separate macro for each log level (perhaps simpler because you have one parameter less) i.e.:

#define LogDebug(source, message) Log(source, LevelDebug, message)
#define LogInfo(source, message) Log(source, LevelInfo, message)
#define LogWarn(source, message) Log(source, LevelWarn, message)

// usage example:
LogDebug(Log_Comm, "Some minor stuff");
LogWarn(Log_System, "Something strange");
vgru
  • 49,838
  • 16
  • 120
  • 201
  • I like this idea but it may need development. How would you exclude all debug altogether for example? You would have to explicitly define all possible `source` macros to zero to expand to `if( 0 & 0x01`) for example, potentially generating "always false" or "unreachable code" warnings. You need a means to exclude all the debug code from the source altogether, with an empty or null definition of `Log`. – Clifford Jun 15 '16 at 17:17
  • @Clifford: yes, this was just an idea, but you're right about warnings. Regarding excluding logs by level, there could simple be an additional "global enable" flag in the `if` condition (i.e. `if (source & level & GlobalEnabledLevels)`), which would only pass certain levels. Regarding the warnings, I am not sure if there is a neat way to address this without pragmas, which are sometimes not so well supported inside macros. I will update my code with an example for Visual Studio (i.e. Microsoft's C99 compiler). – vgru Jun 15 '16 at 20:10
  • 1
    @Groo: Sorry for the late reply but I was very busy! I followed your suggestion and my debug interface works very well!!! – Federico Jul 12 '16 at 09:08
2

You will need to define all the macros regardless of what level is defined, otherwise any place you invoke a macros that is undefined will generate an error. The macros you want to be inactive can simply be defined as empty statements.

You also have redefinitions that won't work, for example if DEBUG_LEVEL_0 is not defined but DEBUG_LEVEL_1 is you have two different definitions of edi_Print_L0(). Similarly if both DEBUG_LEVEL_0 and DEBUG_LEVEL_1 are defined you still have multiple definitions. You need to make the definitions mutually exclusive, or if multiple level macro definitions exist ensure only the highest level is active:

#if defined DEBUG_LEVEL_2
    #define edi_Print_L0(...) printf(__VA_ARGS__)
    #define edi_Print_L1(...) printf(__VA_ARGS__)
    #define edi_Print_L2(...) printf(__VA_ARGS__)
#elif defined DEBUG_LEVEL_1
    #define edi_Print_L0(...) printf(__VA_ARGS__)
    #define edi_Print_L1(...) printf(__VA_ARGS__)
    #define edi_Print_L2(...)
#elif defined DEBUG_LEVEL_0
    #define edi_Print_L0(...) printf(__VA_ARGS__)
    #define edi_Print_L1(...)
    #define edi_Print_L2(...)
#else
    #define edi_Print_L0(...)
    #define edi_Print_L1(...)
    #define edi_Print_L2(...)
#endif

I would also suggest a more useful definition for the debug macros such as:

#define edi_Print_L0( format, ... )   printf( "\nL0:%s::%s(%d) " format, __FILE__, __FUNCTION__,  __LINE__, __VA_ARGS__ )
#define edi_Print_L1( format, ... )   printf( "\nL1:%s::%s(%d) " format, __FILE__, __FUNCTION__,  __LINE__, __VA_ARGS__ )
#define edi_Print_L2( format, ... )   printf( "\nL2:%s::%s(%d) " format, __FILE__, __FUNCTION__,  __LINE__, __VA_ARGS__ )

In that way for example, the line:

edi_Print_L2( "counter=%d", counter ) ;

in say line 24 of file main.c in function main() when counter equals 25 for example will do nothing at level 0 or 1, but at level 2 will output:

L2:main.c::main(24) counter=25

So you get the debug output you require with the exact location in the code and the debug level it was issued at.

A better (more easily maintained) solution is to have a single macro DEBUG_LEVEL with a numeric value:

#if !defined DEBUG_LEVEL
    #define DEBUG_LEVEL = -1
#endif

#if DEBUG_LEVEL >= 2
    #define edi_Print_L2( format, ... )   printf( "\nL2:%s::%s(%d) " format, __FILE__, __FUNCTION__,  __LINE__, __VA_ARGS__ )
#else
    #define edi_Print_L2(...)
#endif

#if DEBUG_LEVEL >= 1
    #define edi_Print_L1( format, ... )   printf( "\nL1:%s::%s(%d) " format, __FILE__, __FUNCTION__,  __LINE__, __VA_ARGS__ )
#else
    #define edi_Print_L1(...)
#endif

#if DEBUG_LEVEL >= 0
    #define edi_Print_L0( format, ... )   printf( "\nL0:%s::%s(%d) " format, __FILE__, __FUNCTION__,  __LINE__, __VA_ARGS__ )
#else
    #define edi_Print_L0(...)
#endif

This solution allows just one definition of each macro, so maintenance is a lot easier and less error prone.

Clifford
  • 88,407
  • 13
  • 85
  • 165
  • Using concatenation like `"\nD2:%s::%s(%d) " format` works fine when `format` is a string literal, but not when `format` is a variable. Perhaps call `printf()` twice instead? – chux - Reinstate Monica Jun 15 '16 at 16:52
  • @chux : That is true, but using a variable for the format string is bad practice in any case due to the potential for format string exploits (or even accidental errors from strings that happen to contain format specifiers). This implementation actively prevents you from introducing bugs in your debug code - which is always useful! For example `printf( str )` would not behave as expected if `str` pointed to `"100%correct"` (implausibly perhaps, but you get the idea). You should always use `printf( "%s", str )` instead. – Clifford Jun 15 '16 at 17:07
  • Although I disagree with "variable for the format string is bad practice", UV for a useful answer. – chux - Reinstate Monica Jun 15 '16 at 18:13
  • @chux : I am surprised that you disagree; it is fairly conventional wisdom, and an essential secure coding practice, up there with not using `gets()` for example. – Clifford Jun 15 '16 at 18:19
  • You might find interesting another approach: [Formatted print without the need to specify type matching specifiers](http://codereview.stackexchange.com/q/115143/29485) – chux - Reinstate Monica Jun 15 '16 at 18:25
  • @chux : Interesting. An alternative solution is provided in GCC and some other compilers via the -Wformat switch allows type-match checking of format strings, and the `format` function attribute to add format checking to your own functions that use standard format specifiers. – Clifford Jun 15 '16 at 19:43
  • @chux: Nice approach, but more error prone and even more bloat for an embedded system. If you forget the wrapper-macro for the arguments, you are busted. Worse, the compiler cannot help detecting type errors as gcc and other modern compilers do for string literals (variable format strings are a no-go on embedded systems anyway, even iff the overhead of `printf` is acceptable). IMO, that is something not suited for C. If you really need such features, it might be better to use C++ or some HigherLL like Python. – too honest for this site Jun 15 '16 at 22:29
  • Note: There are good reasons for not using a blank definition as I have suggested above. I'll leave that for the reader to consider. If you are in the habit of writing single statement conditional blocks *without* braces, it will break your code if the statement is one of these "empty" macros. – Clifford Jun 16 '16 at 08:45
0
  1. I don't know your compiler but printf(...) generates a compiler error.
  2. Your header cannot compile due to the multiple definition of functions with same names.

To gives you an hint you can simply:

#ifdef DEBUG_LEVEL_0
#define edi_Print_L0(format, ...) printf(format, ##__VA_ARGS__)
#else
#define edi_Print_L0(...) do{}while(0)
#endif

But is better to rethink your header. I used to:

#define STRCAT(a, b, c, d, e) a b c d e
#define DEBUG(level, message, color, ...) if (level < DEBUG_LEVEL) fprintf(stdout, STRCAT(color, "%s: %s - ", message, MH_END, "\n"), __FILE__, __func__, ##__VA_ARGS__)

where color can b, e.g.:

#define MH_HGREEN   "\033[1m\033[32m"
#define MH_END      "\033[m"
LPs
  • 16,045
  • 8
  • 30
  • 61
  • Why do you use "do{}while(0)" when DEBUG_LEVEL_0 is not defined? Do I use "edi_Print_L0(...) printf(...)"? – Federico Jun 15 '16 at 14:42
  • I don't know which is your compiler but I'm 99% sure it does not compile. BTW what do you think is the difference between `printf(...)` and `printf(__VA_ARGS__)`? – LPs Jun 15 '16 at 14:45
  • @Federico : Yes but does your code compile!? If you want a macro to do nothing, then it should have no content at all (or the "no-loop" suggested or even just `(void)0`). Calling printf with no arguments at all is probably an error, and even if you had `printf("")` you are still calling `printf` entirely unnecessarily. See http://stackoverflow.com/questions/1306611/how-do-i-implement-no-op-macro-or-template-in-c. – Clifford Jun 15 '16 at 16:29
  • @LPs: What do you believe `printf(##__VA_ARGS__)` will do? I think it will throw a syntax error after not being able to concatenate a parenthesis with the first token in the first argument of the argument list, if any. Perhaps you are thinking of the gcc extension which only applies if the `##` is following a comma? – rici Jun 15 '16 at 16:35
  • @Clifford: Yes my code dosen't compile. To solve the problem I used the empty loop. Is it better to use no-loop or void function (for the coding style)?Thanks! – Federico Jun 16 '16 at 07:03
  • @Clifford: I use this statement "#define edi_Print_L0(...) (void(0))" I obtain a compiler error. Instead if I use "#define edi_Print_L0(...) do{}while(0)" everything works. – Federico Jun 16 '16 at 07:11
  • @Federico : I'd ask what compiler you were using and what error message you received and the exact code that provoked it, but that is a different question not suited to a comment on someone else's answer. There are *very good* reasons for not using a blank definition as I have in my answer, but my in-house coding standard requires braces for even single statement conditionals so I *get away with it*. – Clifford Jun 16 '16 at 08:41
0

The real trick with re-directing printf on embedded systems is that printf calls putch(unsigned char) for every byte it outputs. See: https://www.quora.com/What-is-the-use-of-getch-putch-getchar-putchar-Cant-we-write-a-program-without-using-these-in-C-language

If you make your own putch() function, it will override the system one (because the linker will have seen it first).

You can then bung it down a UART or what ever. Here is a simple example in microchip xc8 C for a PIC18F2480 micro-controller. Obviously the port pins and RS232 BAUD rates were set up earlier.

/* printf will now write to the UART */

void putch(char c)
{
    int count = 0;

    while (!TRMT) // still transmitting, prob should not be 
        count++;
    TXREG = c;
    wait_us(1500); // wait 1.5mS if 9600 baud so as not to over run the dual buffer
}
user50619
  • 325
  • 5
  • 14