hungpham2511 / toppra

robotic motion planning library
https://hungpham2511.github.io/toppra/index.html
MIT License
625 stars 170 forks source link

Logging for the cpp codebase #96

Closed hungpham2511 closed 3 years ago

hungpham2511 commented 4 years ago

I am thinking of adding a logger to the cpp codebase to ease debugging. Using and commenting std::cout is quite tedious.

This https://github.com/gabime/spdlog looks like a solid option.

@jmirabel What do you think?

jmirabel commented 4 years ago

I always hesitate between adding a dependency for logging and writing my own macros. A new dependency makes installation more fastidious. For logging, we could start with something very simple like:

#ifdef TOPPRA_ENABLE_LOGGING
#define LOG std::cout
#else
#define LOG if (true) {} else std::cout
#endif
...

LOG << "this goes into the logs.";

This very simple code already enables plenty of things. Eventually, some log levels can be added with

#define _TOPPRA_LOG_ENABLED std::cout
#define _TOPPRA_LOG_DISABLED if (true) {} else std::cout
#if TOPPRA_LOG_LEVEL > 1
# define LOG_DEBUG _TOPPRA_LOG_ENABLED << "DEBUG: "
# define LOG_ERROR _TOPPRA_LOG_ENABLED << "ERROR: "
#elif TOPPRA_LOG_LEVEL > 2
# define LOG_DEBUG _TOPPRA_LOG_DISABLED
# define LOG_ERROR _TOPPRA_LOG_ENABLED << "ERROR: "
#else
# define LOG_DEBUG _TOPPRA_LOG_DISABLED
# define LOG_ERROR _TOPPRA_LOG_DISABLED
#endif
hungpham2511 commented 4 years ago

That looks neat. I have this simple log macro in #90, https://github.com/hungpham2511/toppra/blob/11c15bd198713d253ae87b85b4a5a724de7537f8/cpp/src/toppra/toppra.hpp#L11

Are we able to change log level at run time with this solution? With a global variable in toppra namespace perharps?

jmirabel commented 4 years ago

With the above snippets, everything is decided at compile time.

A runtime log level isn't too hard to achieve, one can use

#define TOPPRA_LOG(level, msg) if (::toppra::loglevel() >= level) { std::cout << msg; }
#define TOPPRA_LOG_DEBUG(msg) TOPPRA_LOG(1, "DEBUG: " << msg)
#define TOPPRA_LOG_ERROR(msg) TOPPRA_LOG(1, __FILE__ << ':' << __LINE__ << ":ERROR: " << msg)
...

Then define two functions

namespace toppra {
void loglevel(int level);
int loglevel();
}

Note that, the runtime option generates a larger binary. This can have as effect to increase the number of cache misses. For now, this is of minor importance.

github-actions[bot] commented 4 years ago

Stale issue message