wonder-mice / zf_log

Core logging library for C/ObjC/C++
MIT License
196 stars 51 forks source link

support for embedded cpu´s #11

Closed mgiaco closed 7 years ago

mgiaco commented 7 years ago

Hello,

Do you have any good idea what define I should use for this improvement? You use _WIN32, _WIN64, ANDROID, linux, MACH. So what do you think should I use for BARE ARM or for an Embedded RTOS like FreeRTOS?

regards, mathias

wonder-mice commented 7 years ago

Hi, that depends on what piece of code needs to be different on these platforms. It easier to solve that in case by case basis, than to have global "bare arm" macro, since current platforms have a lot in common (like malloc() function). What particular features your platform is lacking?

wonder-mice commented 7 years ago

Another approach you can take is to add a compile time option to zf_log.c, say ZF_LOG_NO_MALLOC or ZF_LOG_NO_LIBC. And then you can define that in your build system when building zf_log.

mgiaco commented 7 years ago

So it was more a how should I made it that it fits your thinking. I do not find any malloc in the lib? So I ended up by adding some defines to disable the time and pid feature. Instead of that i add the system tick as a time indicator. I do not found any suggestion of a general systick handling over linux, windows and arm bare metal.

wonder-mice commented 7 years ago

Yes, currently it allocates everything on stack, so no malloc.

What you describe is handled by put_ctx() function. One option is to allow to override this function, for example the same way it's done with ZF_LOG_EXTERN_GLOBAL_OUTPUT_LEVEL (when defined, code outside must define required symbol). So, it could be something like ZF_LOG_EXTERN_PUT_CONTEXT and when defined, user of library must define zf_log_put_context(zf_log_message *const msg) function.

If code that you come up with looks like generic enough and that other developer can benefit from it, then it seems logical to have ZF_LOG_ARM_SYSTICK_CONTEXT macro, that when defined will use simplified systick based timestamp. Don't see anything bad about it, but if we go this route, I suspect it will change eventually to something more generic when we will have more data points about how people actually use it.

wonder-mice commented 7 years ago

I actually curious to see the patch, maybe will have better ideas once I see it.

wonder-mice commented 7 years ago

Hi, checkout top of the master branch. You can now define ZF_LOG_MESSAGE_CTX_FORMAT to override default context format, for example:

#define ZF_LOG_MESSAGE_CTX_FORMAT \              
     (F_INIT(( unsigned tickcount(); )), \        
      F_UINT(5, tickcount()), S(" "), LEVEL, S(" "))                                     

tickcount() is your function that you must define else where. This will result in following message:

  456 I TAG function@filename.c:line Message text

See comments in zf_log.c for details.

mgiaco commented 7 years ago

Hello,

Sorry I was on vacation - many thanks i will check this out.

wonder-mice commented 7 years ago

I didn't introduce more "fields" like F_UINT(width, value) because it was hard to tell what common requirements are. I can imagine that somebody will need, say, F_UINT64() or F_DOUBLE(). Let me know if current set of supported fields doesn't fit your use case.

mgiaco commented 7 years ago

Hi

I use this a lot now but I always get some warning on the build. My problem is that I use this tick Funktion.

uint32_t mytick(void)
{
 return ...
}

warning:

warning: format '%u' expects argument of type 'unsigned int', but argument 4 has type 'uint32_t {aka long unsigned int}' [-Wformat=]

I did not figure it out how that F_INIT works. Can you explain that a little bit?

By the way can you add a zf_log_config.h or so? Or how do you handle this because modifying the zf_log.c file is not what i want.

Why not add this to zf_log.c


#ifdef ZF_LOG_CONFIG
#include "zf_log_config.h"
#endif

/* When defined, Android log (android/log.h) will be used by default instead of
 * stderr (ignored on non-Android platforms). Date, time, pid and tid (context)
 * will be provided by Android log. Android log features will be used to output
 * log level and tag.
 */
#ifdef ZF_LOG_USE_ANDROID_LOG
    #undef ZF_LOG_USE_ANDROID_LOG
....
....

With this it is possible to define ZF_LOG_CONFIG and all the complicated defines like the ZF_LOG_MESSAGE_CTX_FORMAT can be defined in that file which is much better than write it into compiler UI´s one line textbox :-)

wonder-mice commented 7 years ago

By the way can you add a zf_log_config.h or so? Or how do you handle this because modifying the zf_log.c file is not what i want.

You shouldn't modify it. Idea is that you provide these defines with your build system (which in the end will just pass several -D options to your compiler). Or you can create zf_log_config.h and instruct compiler to include it with -include command line option. Basically these defines must be specified using build system or you compiler, not by modifying zf_log source.

My problem is that I use this tick Funktion.

Any reason why you don't want to do the cast? For example, declare your function as

unsigned mytick(void) {
}

or when you use it:

unsigned t = (unsigned) mytick();

I did not figure it out how that F_INIT works. Can you explain that a little bit?

Which part you don't understand? How it works or how to use it?

wonder-mice commented 7 years ago

I'll think about ZF_LOG_CONFIG, but still not convinced, since compilers usually have -include option that already does exactly the same thing.

mgiaco commented 7 years ago

Hi,

Ah okay the cast is okay. I do not understand this two defines

#define F_INIT(statements) F_INIT(statements) 
#define F_UINT(width, value) F_UINT(width, value)

I do not find any replacement for F_UINT and F_INIT :-) Is this some magic?

ZF_LOG_CONFIG: But if you work with some IDE like Keil ARM or so then it is really not that easy to add such special thinks like -include. Also it is not transparent for others if there is some big define anywhere in the compiler settings... because on IDE´s the -D stuff is mostly anywhere in some IDE windows.

wonder-mice commented 7 years ago

I do not find any replacement for F_UINT and F_INIT :-) Is this some magic?

A bit of a magic, yes. Though it explained in the comment:

/* Fields that can be used in log message format specifications (see above).
 * Mentioning them here explicitly, so we know that nobody else defined them
 * before us. See ZF_LOG_MESSAGE_CTX_FORMAT for details.
 */

The key part is Mentioning them here explicitly, so we know that nobody else defined them before us.. So it's just a way to generate a compiler error if one of these macros was defined before.

If you want to understand how it works, look at this line:

#define _ZF_LOG_MESSAGE_FORMAT_INIT__F_INIT(expr) _PP_UNTUPLE(expr);

And also find places where _ZF_LOG_MESSAGE_FORMAT_INIT macro is used, like this:

_PP_MAP(_ZF_LOG_MESSAGE_FORMAT_INIT, ZF_LOG_MESSAGE_TAG_FORMAT)

It basically goes through items in ZF_LOG_MESSAGE_TAG_FORMAT and applies _ZF_LOG_MESSAGE_FORMAT_INIT to each. And _ZF_LOG_MESSAGE_FORMAT_INIT is implemented as:

#define _ZF_LOG_MESSAGE_FORMAT_INIT(field) \
    _PP_CONCAT_3(_ZF_LOG_MESSAGE_FORMAT_INIT_, _, field)

So it will concat "_ZF_LOG_MESSAGE_FORMATINIT", "_" and field, which (in case you interested in) is F_INIT((<something>)). F_INIT expands to itself by default, but after concatenation it produces _ZF_LOG_MESSAGE_FORMAT_INIT__F_INIT((<something>)) which expands to _PP_UNTUPLE((<something>)) which expands to just <something>.

So, basically, this:

_PP_MAP(_ZF_LOG_MESSAGE_FORMAT_INIT, ZF_LOG_MESSAGE_TAG_FORMAT)

Just extracts all contents of F_INIT fields from ZF_LOG_MESSAGE_TAG_FORMAT and filters out everything else.

mgiaco commented 6 years ago

Hi, any news here for this suggestion?

#ifdef ZF_LOG_CONFIG
#include "zf_log_config.h"
#endif

The -include dose not work for embedded targets for me. I tried this in cmake

add_definitions(-include "zf_log_cofig.h")  

the problem is then the linke use that define for every file and that brings compiler errors. So please add this. With the change I can add

add_definitions(-DZF_LOG_CONFIG)

and then I can use the header for my configs. That works perfect also on some special compilers like keil arm.