Start using spdlog in the repo #225

GiulioRomualdi · 2021-03-03T19:47:29Z

This pr should be seen as a point where we can discuss. Then if we found a common agreement we may think to merge it.

In this PR I added spdlog. It is a nice log that can be used to print stuff in the terminal.
The dependency can be installed with apt vcpkg and brew.

This is the output of the TimeVaryingDCMTest with the code implemented in this PR.

We can further customize the output: https://github.com/gabime/spdlog/wiki/3.-Custom-formatting.

It's also possible to automatically print the name of the function, name of the file and line. However, we should define some macros.
The nice thing is that it can be used in a multithread application https://cran.r-project.org/web/packages/RcppSpdlog/vignettes/introduction.html

cc @traversaro @prashanthr05 @S-Dafarra

traversaro · 2021-03-04T09:09:13Z

src/TextLogging/src/Logger.cpp

+
+TextLogging::Logger* const log()
+{
+    auto logger = spdlog::get("console");


I don't know if it make sense to use a blf-specific name for this, especially given that we modify it (set level, set pattern) so it it may be better to avoid conflicts with other libraries that already may use spdlog "console"?

prashanthr05 · 2021-03-04T10:15:22Z

src/TextLogging/CMakeLists.txt

+add_bipedal_locomotion_library(
+  NAME                   TextLogging
+  PUBLIC_HEADERS         include/BipedalLocomotion/TextLogging/Logger.h
+  SOURCES                src/Logger.cpp
+  PUBLIC_LINK_LIBRARIES  spdlog::spdlog)


Nitpick: Why the names Textlogging and a Logger? I believe if we are going to use this only for debug prints then Logger is too generic for a name, maybe?

I'm terrible with the names 😃 . Do you have any suggestions?

Now that, I've seen these names, I am not able to think of others quickly ;D

I think it's ok.

GiulioRomualdi · 2021-03-04T10:45:17Z

Windows is failing with

  By not providing "Findspdlog.cmake" in CMAKE_MODULE_PATH this project has
See also "D:/a/bipedal-locomotion-framework/bipedal-locomotion-framework/build/CMakeFiles/CMakeOutput.log".
  asked CMake to find a package configuration file provided by "spdlog", but
  CMake did not find one.

  Could not find a package configuration file provided by "spdlog" with any
  of the following names:

    spdlogConfig.cmake
    spdlog-config.cmake

  Add the installation prefix of "spdlog" to CMAKE_PREFIX_PATH or set
  "spdlog_DIR" to a directory containing one of the above files.  If "spdlog"
  provides a separate development package or SDK, be sure it has been
  installed.

.github/workflows/ci.yml

traversaro · 2021-03-04T10:52:49Z

Can you update the superbuild to document and install in CI spdlog? Even in the usual base dependencies is fine. Otherwise I can do it myself.

GiulioRomualdi · 2021-03-04T11:31:23Z

Can you update the superbuild to document and install in CI spdlog? Even in the usual base dependencies is fine. Otherwise I can do it myself.

sure

GiulioRomualdi · 2021-03-04T14:52:39Z

Let me know if we can merge it or we should update the superbuild before

traversaro · 2021-03-04T17:31:10Z

Let me know if we can merge it or we should update the superbuild before

If we can update the superbuild before it would avoid a bit of red sadness in superbuild CI. : )
The 0.6.2 vcpkg binaries are being prepared in https://github.com/robotology/robotology-superbuild-dependencies-vcpkg/actions/runs/621356334, once that is done I think we can update the superbuild.

traversaro · 2021-03-05T09:07:55Z

Let me know if we can merge it or we should update the superbuild before

If we can update the superbuild before it would avoid a bit of red sadness in superbuild CI. : )
The 0.6.2 vcpkg binaries are being prepared in https://github.com/robotology/robotology-superbuild-dependencies-vcpkg/actions/runs/621356334, once that is done I think we can update the superbuild.

0.6.2 released with spdlog: https://github.com/robotology/robotology-superbuild-dependencies-vcpkg/releases/tag/v0.6.2 .

traversaro · 2021-03-05T09:18:32Z

Furthermore, given that we use master as Stable branch of bipedal-locomotion-control, we need to make sure that people that use ROBOTOLOGY_USES_DYNAMICS have libspdlog-dev installed, probably an announcement in internal chat is enough.

GiulioRomualdi · 2021-03-05T09:35:15Z

PR opened robotology/robotology-superbuild#645

GiulioRomualdi · 2021-03-05T12:32:54Z

same error in windows-debug

2021-03-05T12:26:53.2753752Z CMake Error at C:/robotology/vcpkg/scripts/buildsystems/vcpkg.cmake:644 (_find_package):
2021-03-05T12:26:53.2755113Z   By not providing "Findspdlog.cmake" in CMAKE_MODULE_PATH this project has
2021-03-05T12:26:53.2755928Z -- Configuring incomplete, errors occurred!
2021-03-05T12:26:53.2757307Z See also "D:/a/bipedal-locomotion-framework/bipedal-locomotion-framework/build/CMakeFiles/CMakeOutput.log".
2021-03-05T12:26:53.2758666Z   asked CMake to find a package configuration file provided by "spdlog", but
2021-03-05T12:26:53.2759436Z   CMake did not find one.
2021-03-05T12:26:53.2759827Z 
2021-03-05T12:26:53.2760463Z   Could not find a package configuration file provided by "spdlog" with any
2021-03-05T12:26:53.2761182Z   of the following names:
2021-03-05T12:26:53.2761561Z 
2021-03-05T12:26:53.2762069Z     spdlogConfig.cmake
2021-03-05T12:26:53.2762678Z     spdlog-config.cmake
2021-03-05T12:26:53.2763108Z 
2021-03-05T12:26:53.2763734Z   Add the installation prefix of "spdlog" to CMAKE_PREFIX_PATH or set
2021-03-05T12:26:53.2764586Z   "spdlog_DIR" to a directory containing one of the above files.  If "spdlog"
2021-03-05T12:26:53.2765786Z   provides a separate development package or SDK, be sure it has been
2021-03-05T12:26:53.2766343Z   installed.
2021-03-05T12:26:53.2768198Z Call Stack (most recent call first):
2021-03-05T12:26:53.2769330Z   cmake/BipedalLocomotionFrameworkDependencies.cmake:15 (find_package)
2021-03-05T12:26:53.2770324Z   CMakeLists.txt:21 (include)

but now spdlog is installed within robotology-superbuild-dependencies-vcpkg

traversaro · 2021-03-05T12:54:58Z

Sorry, the problem is described and fixed in robotology/robotology-superbuild-dependencies-vcpkg#43 .

GiulioRomualdi · 2021-03-05T13:51:21Z

Waiting the approval before merging

GiulioRomualdi · 2021-03-08T19:29:48Z

Friendly Ping @S-Dafarra and @prashanthr05 :)

S-Dafarra

It looks good to me

S-Dafarra · 2021-03-09T08:47:13Z

Not a big deal, but for @traversaro there were still requested changes. I guess they were resolved right?

traversaro · 2021-03-09T08:57:01Z

Not a big deal, but for @traversaro there were still requested changes. I guess they were resolved right?

Yes!

prashanthr05 · 2021-03-09T09:21:54Z

Waiting the approval before merging

Sorry, I had not noticed this. I had taken a look but forgot to approve.

GiulioRomualdi added 3 commits March 3, 2021 20:41

Implement TextLogging component

912cb83

Use Logger() instead of std::cerr in ParameterHandler::StdImplementation

4dd0be5

Use Logger() instead of std::cerr in TimeVaryingDCMPlanner

3970c95

traversaro approved these changes Mar 4, 2021

View reviewed changes

traversaro reviewed Mar 4, 2021

View reviewed changes

prashanthr05 reviewed Mar 4, 2021

View reviewed changes

GiulioRomualdi added 4 commits March 4, 2021 11:22

Rename the logger into blf

cf4c8a8

Add spdlog to GitHub CI

ab7534a

Update the README.md

97a7f9b

Update the CHANGELOG.md

7a25ffd

GiulioRomualdi requested a review from S-Dafarra March 4, 2021 10:29

GiulioRomualdi marked this pull request as ready for review March 4, 2021 10:29

GiulioRomualdi self-assigned this Mar 4, 2021

traversaro requested changes Mar 4, 2021

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

GiulioRomualdi force-pushed the spdlogger branch 2 times, most recently from 598c79a to a3038a0 Compare March 5, 2021 11:47

[GitHub CI] Bump vcpkg_robotology_TAG to v0.6.2

0f0e37e

GiulioRomualdi force-pushed the spdlogger branch from a3038a0 to 0f0e37e Compare March 5, 2021 12:54

S-Dafarra approved these changes Mar 9, 2021

View reviewed changes

GiulioRomualdi merged commit 7f02bcd into master Mar 9, 2021

GiulioRomualdi deleted the spdlogger branch March 9, 2021 08:45

This was referenced May 17, 2021

Add HumanDynamicsEstimation library robotology/human-dynamics-estimation#246

Merged

Add logging tool in HDE library robotology/human-dynamics-estimation#248

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Start using spdlog in the repo #225

Start using spdlog in the repo #225

GiulioRomualdi commented Mar 3, 2021 •

edited

Loading

traversaro Mar 4, 2021

prashanthr05 Mar 4, 2021 •

edited

Loading

GiulioRomualdi Mar 4, 2021

prashanthr05 Mar 4, 2021 •

edited

Loading

prashanthr05 Mar 4, 2021

GiulioRomualdi commented Mar 4, 2021 •

edited

Loading

traversaro commented Mar 4, 2021

GiulioRomualdi commented Mar 4, 2021

GiulioRomualdi commented Mar 4, 2021

traversaro commented Mar 4, 2021 •

edited

Loading

traversaro commented Mar 5, 2021

traversaro commented Mar 5, 2021

GiulioRomualdi commented Mar 5, 2021

GiulioRomualdi commented Mar 5, 2021

traversaro commented Mar 5, 2021

GiulioRomualdi commented Mar 5, 2021

GiulioRomualdi commented Mar 8, 2021 •

edited

Loading

S-Dafarra left a comment

S-Dafarra commented Mar 9, 2021

traversaro commented Mar 9, 2021

prashanthr05 commented Mar 9, 2021

Start using spdlog in the repo #225

Start using spdlog in the repo #225

Conversation

GiulioRomualdi commented Mar 3, 2021 • edited Loading

traversaro Mar 4, 2021

Choose a reason for hiding this comment

prashanthr05 Mar 4, 2021 • edited Loading

Choose a reason for hiding this comment

GiulioRomualdi Mar 4, 2021

Choose a reason for hiding this comment

prashanthr05 Mar 4, 2021 • edited Loading

Choose a reason for hiding this comment

prashanthr05 Mar 4, 2021

Choose a reason for hiding this comment

GiulioRomualdi commented Mar 4, 2021 • edited Loading

traversaro commented Mar 4, 2021

GiulioRomualdi commented Mar 4, 2021

GiulioRomualdi commented Mar 4, 2021

traversaro commented Mar 4, 2021 • edited Loading

traversaro commented Mar 5, 2021

traversaro commented Mar 5, 2021

GiulioRomualdi commented Mar 5, 2021

GiulioRomualdi commented Mar 5, 2021

traversaro commented Mar 5, 2021

GiulioRomualdi commented Mar 5, 2021

GiulioRomualdi commented Mar 8, 2021 • edited Loading

S-Dafarra left a comment

Choose a reason for hiding this comment

S-Dafarra commented Mar 9, 2021

traversaro commented Mar 9, 2021

prashanthr05 commented Mar 9, 2021

GiulioRomualdi commented Mar 3, 2021 •

edited

Loading

prashanthr05 Mar 4, 2021 •

edited

Loading

prashanthr05 Mar 4, 2021 •

edited

Loading

GiulioRomualdi commented Mar 4, 2021 •

edited

Loading

traversaro commented Mar 4, 2021 •

edited

Loading

GiulioRomualdi commented Mar 8, 2021 •

edited

Loading