Add logging tool in HDE library

[lrapetti]

At this stage, the hde library depends on YARP just for the terminal logging tool. We could then remove this dependency by moving to an independent tool (e.g. spdlog as in https://github.com/dic-iit/bipedal-locomotion-framework/pull/225)

[lrapetti]

I was having a look to what contained in blf. I was thinking that if we agree on the spdlog tool, I guess we can basically duplicate what is in https://github.com/dic-iit/bipedal-locomotion-framework/tree/master/src/TextLogging. For me, I don't see it as a big problem to start by basically copying what is inside there, and then eventually diverge due to different requirements. (thanking into account that I am referring to few lines of code).

If we agree on that, one point to understand is if it is better to use same name for the tool (so it is familiar moving around the libraries), or to pick something different from TextLogging.

Another consideration might be to adding blf as dependencies for hde library, as there is already this plan, and to use the same logger. But in this case we would end-up with strong dependency of hde on blf. Indeed, on my side I would prefer a dedicated logger utility.

Any opinion on this @GiulioRomualdi @S-Dafarra @diegoferigo @Yeshasvitvs @prashanthr05 @traversaro ?

[diegoferigo]

For me, I don't see it as a big problem to start by basically copying what is inside there, and then eventually diverge due to different requirements. (thanking into account that I am referring to few lines of code).

This sounds good to me.

[traversaro]

Ok for copying the logger code for me. In this particular case it also make sense as we want to enable debug prints when hde is compiled in debug (see https://github.com/dic-iit/bipedal-locomotion-framework/blob/master/src/TextLogging/src/Logger.cpp#L29), without a strict dependency on how blf is compiled.

[GiulioRomualdi]

If you do not want to use spdlog (even if it can be easily installed) you can use this approach https://github.com/GiulioRomualdi/scs-eigen/blob/main/src/ScsEigen/include/ScsEigen/Logger.h

Another consideration might be to adding blf as dependencies for hde library, as there is already this plan, and to use the same logger. But in this case we would end-up with strong dependency of hde on blf. Indeed, on my side I would prefer a dedicated logger utility.

I agree on this. Furthermore, notice that the logger in blf is a singleton. So if you call BipedalLocomotion::log()->info(....);it will print [blf][....].

[GiulioRomualdi]

Here is how spdlog is used in drake https://github.com/RobotLocomotion/drake/blob/026804c4e89c504f657a804764de598739426d60/common/text_logging.cc#L1

[diegoferigo]

Just a nit that supports my personal fight against the usage of raw pointers. The typical Singleton pattern could use:

// Instead of:
// Logger* const ScsEigen::log()

const Logger& ScsEigen::log()
{
    static Logger l;
    return l;
}

Maybe the same would be valid in the spdlog case that uses a shared pointer.

[GiulioRomualdi]

Hi @diegoferigo when I implemented the logger in blf I didn't think about it and now I'm too lazy to change all the code 😄

A possible implementation is

const Logger& ScsEigen::log()
{
   // Since the oobject is static the memory is not deallocated    
   static std::shared_ptr<TextLogging::Logger>logger(loggerCreation());

  // the logger exist because loggerCreation is called.  
   return *logger;
}

notice that spdlog already handles the logger as a singleton and the way to get is using pointers,

https://github.com/dic-iit/bipedal-locomotion-framework/blob/0c055db96d25fa94d588626d8df8a375a0b04c19/src/TextLogging/src/Logger.cpp#L17

[diegoferigo]

Yep! My comment was really a nit in case people copy-and-past the linked code. Regarding the blf implementation, I was referring to this code. A global object is always valid, and something tickles every time I see -> on raw pointers without proper checks :) Using const references seems to be the most appropriate pattern in this case. In any case, we can discuss this tiny detail more thoroughly when a PR will be submitted to address this issue.