Ask Your Question
1

Missing log4cxx on latest ros_comm

asked 2014-01-09 04:59:18 -0500

ahendrix gravatar image

updated 2014-01-09 05:32:37 -0500

With the latest release of ros_comm (1.9.52) and roscpp_core (0.3.17), the dependency on log4cxx has been removed. This breaks a couple of very old packages that relied on log4cxx being the underlying implementation of ros logging.

So far I've seen this show up in the Hokuyo driver: https://github.com/ros-drivers/hokuyo_node/issues/7 and in the PR2 Ethercat Driver: https://github.com/PR2/pr2_ethercat_drivers/issues/62

In both cases, the node in question is using log4cxx calls to adjust the verbosity of the underlying logger: https://github.com/PR2/pr2_ethercat_drivers/blob/hydro-devel/ethercat_hardware/src/motorconf.cpp#L642-L643 https://github.com/ros-drivers/hokuyo_node/blob/hydro-devel/src/getFirmwareVersion.cpp#L56

I don't see any recommendations on how to adjust the logger levels from C++ on the roscpp logging or the rosconsole wiki pages. While adding a log4cxx configuration file is suggested, it's a system-wide change that isn't viable for this sort of single-executable logging adjustment, and probably isn't compatible with the non-log4cxx logging backends.

Is there a recommendation on how migrate these calls to the new backed-agnostic logging?

edit retag flag offensive close merge delete

Comments

1

The real question is: why is such a breaking change introduced in the released distribution which should only receive non-breaking bugfixes?

AHornung gravatar image AHornung  ( 2014-01-09 05:35:22 -0500 )edit

This is not a breaking change. The above described code is using `log4cxx` without depending and including `log4cxx` explicitly. Relying on transitive dependencies is not stable and by no means guaranteed. If the code would state its dependency on `log4cxx` correctly it can continue as it is (and use the log4cxx log level enum directly rathern then trying to resolve it via an internal global variable which is not part of the public API).

Dirk Thomas gravatar image Dirk Thomas  ( 2014-01-09 13:21:15 -0500 )edit
1

This is an API-breaking change because you're removing variables from the public headers: https://github.com/ros/ros_comm/commit/4225dd5b528b3f977a1e3ecc4924c9f267071530

ahendrix gravatar image ahendrix  ( 2014-01-10 07:23:15 -0500 )edit

1 Answer

Sort by ยป oldest newest most voted
1

answered 2014-01-09 13:19:00 -0500

Dirk Thomas gravatar image

updated 2014-01-14 13:14:46 -0500

You might consider using the function used by the get/set logger level RPCs:

ros::console::set_logger_level(...)

But you have to explicitly invoke ros::console::notifyLoggerLevelsChanged() when logger levels have been successfully set in order for them to be applied.

Update:

The removed symbol has been readded in https://github.com/ros/ros_comm/pull/336 But the missing explicit dependency is still something all affected packages will have to address.

edit flag offensive delete link more

Comments

1

Update: I _REALLY_ don't like this solution, because it doesn't compile against the previous version of rosconsole in Hydro, because the set_logger_level function isn't public. I'm going to agree with @AHornung that this is a breaking change. It should be reverted in Hydro and delayed to Igloo.

ahendrix gravatar image ahendrix  ( 2014-01-09 17:50:00 -0500 )edit

Final solution, using the re-added symbol here: https://github.com/PR2/pr2_ethercat_drivers/compare/1.8.5...1.8.7

ahendrix gravatar image ahendrix  ( 2014-01-15 07:47:42 -0500 )edit

Your Answer

Please start posting anonymously - your entry will be published after you log in or create a new account.

Add Answer

Question Tools

1 follower

Stats

Asked: 2014-01-09 04:59:18 -0500

Seen: 755 times

Last updated: Jan 14 '14