ros / urdfdom

URDF parser
http://ros.org/wiki/urdf
Other
96 stars 132 forks source link

Removing console_bridge for Logging #198

Closed CursedRock17 closed 7 months ago

CursedRock17 commented 7 months ago

Feature Request

Remove console_bridge

Details

Connected to this issue in geometry2 where we removed the console_bridge library, we could remove console_bridge from this library to become less dependency heavy. Stated as:

Actually, if we also did this for urdfdom, we could get rid of the console_bridge dependency from ROS 2 completely.

Implementation

Use RCLCPPLOG or RCUTILSLOG instead of console_bridge, which hasn't been updated for years. console_bridge simply adds additional boilerplate.

Going off of this PR

traversaro commented 7 months ago

Using RCLCPP_LOG_* and RCUTILS_LOG_* will add back the dependency on ROS 2 core packages on this library, that differently from tf2 is indeed also packaged outside of ROS distributions (see for example https://github.com/Microsoft/vcpkg/tree/master/ports/urdfdom, https://github.com/conan-io/conan-center-index/tree/master/recipes/urdfdom and https://github.com/conda-forge/urdfdom-feedstock). So I am not sure if this will make the library less "dependency heavy".

clalancette commented 7 months ago

Using RCLCPP_LOG_* and RCUTILS_LOG_* will add back the dependency on ROS 2 core packages on this library, that differently from tf2 is indeed also packaged outside of ROS distributions (see for example https://github.com/Microsoft/vcpkg/tree/master/ports/urdfdom, https://github.com/conan-io/conan-center-index/tree/master/recipes/urdfdom and https://github.com/conda-forge/urdfdom-feedstock). So I am not sure if this will make the library less "dependency heavy".

Yeah, I don't think we should make the same change here.

CursedRock17 commented 7 months ago

So, should we just close the issue, or would there be a different library that would suit this better? I didn't know if we wanted to stay more in with the core of ROS with rcutils.

clalancette commented 7 months ago

So, should we just close the issue, or would there be a different library that would suit this better? I didn't know if we wanted to stay more in with the core of ROS with rcutils.

Yeah, I think we should just close this. Sorry for my earlier comment where I suggested we do this here in urdfdom. I forgot that it is in a different situation than geometry2 and class_loader, which are well integrated into the core of ROS 2. This package can be compiled without ROS 2, so I think we should leave it as-is.

As far as choosing something else; while console_bridge has not had a huge amount of movement in the past few years, I think that just means it is more-or-less "done", rather than not having any use. And it is relatively lightweight. So I'd say we just do nothing here and leave everything as-is.

For all of those reasons, I'm going to close this out. Thanks for bringing it up.