Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Depend on libconsole-bridge-dev instead of rosconsole? #81

Open
mikepurvis opened this issue Oct 30, 2017 · 3 comments
Open

Depend on libconsole-bridge-dev instead of rosconsole? #81

mikepurvis opened this issue Oct 30, 2017 · 3 comments
Labels

Comments

@mikepurvis
Copy link
Member

We desire to introduce pluginlib as a dependency for packages inside ros_comm without creating a repo-level circular dependency, which would presently be the case on account of rosconsole being inside ros_comm (see ros/ros_comm#1206).

An obvious solution is to extract rosconsole into its own repo, but I wonder about breaking the dependency on this end, too. Excluding tests, all usages of logging in this repo are as follows:

$ grep -r \ ROS_ *
include/pluginlib/class_loader_imp.h:  ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Creating ClassLoader, base = %s, address = %p",
include/pluginlib/class_loader_imp.h:  ROS_DEBUG_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:  ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Destroying ClassLoader, base = %s, address = %p",
include/pluginlib/class_loader_imp.h:  ROS_DEBUG_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Instance created with object pointer = %p", obj);
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader", "CreateClassException about to be raised for class %s",
include/pluginlib/class_loader_imp.h:  ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Attempting to create managed instance for class %s.",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader", "%s maps to real class type %s",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader", "boost::shared_ptr to object of real type %s created.",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:  ROS_DEBUG_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader", "%s maps to real class type %s",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader", "std::unique_ptr to object of real type %s created.",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:  ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Attempting to create UNMANAGED instance for class %s.",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader", "%s maps to real class type %s",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Instance of type %s created.", class_type.c_str());
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:  ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Entering determineAvailableClasses()...");
include/pluginlib/class_loader_imp.h:  ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Exiting determineAvailableClasses()...");
include/pluginlib/class_loader_imp.h:    ROS_ERROR_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:    ROS_ERROR_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Class %s has no mapping in classes_available_.",
include/pluginlib/class_loader_imp.h:  ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Class %s maps to library %s in classes_available_.",
include/pluginlib/class_loader_imp.h:  ROS_DEBUG_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Checking path %s ", it->c_str());
include/pluginlib/class_loader_imp.h:      ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Library %s found at explicit path %s.",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Class %s has no mapping in classes_available_.",
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader", "No path could be found to the library containing %s.",
include/pluginlib/class_loader_imp.h:  ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Processing xml file %s...", xml_file.c_str());
include/pluginlib/class_loader_imp.h:      ROS_ERROR_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:      ROS_ERROR_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:        ROS_DEBUG_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:        ROS_DEBUG_NAMED("pluginlib.ClassLoader",
include/pluginlib/class_loader_imp.h:  ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Refreshing declared classes.");
include/pluginlib/class_loader_imp.h:    ROS_DEBUG_NAMED("pluginlib.ClassLoader", "Attempting to unload library %s for class %s",

The main issue with switching this is that console_bridge doesn't have the concept of named loggers the way rosconsole does, so that would be lost, however it seems like there may be some logic within rosconsole_bridge that attempts to recover a prefix embedded in the log string itself:

https://github.com/ros/rosconsole_bridge/blob/8e27f6ef86c84b78d9fd20d883923fca70ba3532/src/bridge.cpp#L56-L75

Would a PR that switches this over be acceptable to the maintainers?

@dirk-thomas
Copy link
Member

Not sure if that effects the decision / implementation. One question will be where REGISTER_ROSCONSOLE_BRIDGE will be called than to ensure that the messages are being delivered to e.g. /rosout.

@mikepurvis
Copy link
Member Author

Interesting— yeah, not having an actual TU to put that in does muddy the waters a bit. However, given the context here, perhaps it would be reasonable to just do it once as a heap-allocated thing when the first ClassLoader is initialized. Anyway, would have to play with it a bit.

@mikaelarguedas
Copy link
Member

My preference here is to wait for ros_comm to be split in multiple repositories. Then pluginlib can be used in various tools without removing the ability for pluginlib users to use rosconsole features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants