-
Notifications
You must be signed in to change notification settings - Fork 523
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
Fix loggers to publish to /rosout #2376
Comments
Another thing we should consider changing is enabling the logging service to enable changing the log level via ros service. |
When investigating if there is a way we can send logs to rosout that are not associated with nodes I found this quote in a Discourse thread from 2018:
Source: https://discourse.ros.org/t/ros2-logging/6469?page=2 |
To fix the easy things first and explore the patterns we want to use, I'm starting making PRs from main functions. I'm working on the warehouse package in moveit_ros and will submit PRs. Here is the list of PRs with notes. Only a main:
main with some free functions:
Adding a parameter to a method (do we backport changes like this?): |
Here is a new approach: #2445 |
Here we go with more complexities. In humble child loggers
We are left with a decision about how we should move forward and best support users on humble, rolling, and iron with moveit. We currently do not use node loggers at all so none of our logs go to Option 1: Save the namespaces, no /rosout on humblePreserving the logging namespaces we currently have using child loggers will mean that on Humble converting to node loggers will largely have no useful affect (most logs will still not go out to Maybe this is ok because we can show that newer versions of ROS have useful features and encourage users to upgrade to Rolling (and later J-turtle) for the Option 2: Abandon namespaces for nowWe could not use namespacing and just put all logs under the node, this will make logging to Option 3: Create nodes everywhereAs stated here a workaround for humble is to create nodes for every namespace. This is the maximum compatibility approach. I'll prototype something to do this. |
The approach in #2482 for child logging has some problems. In Humble when you call
So what?Understanding this we can't go creating a bunch of node objects just to get logging working in Humble. The solution is, for humble, to just use the node logger and not make child loggers. This looses namespacing in humble. There are two user stories for namespacing.
Where did the log line come from?To answer the first one, we can use source location information. Every log message in
export RCUTILS_CONSOLE_OUTPUT_FORMAT="[{severity} {time}] [{name}] [{function_name}]: {message} ({file_name}:{line_number})" Configure log levels per-componentCurrently, without node logging, this is a feature we just don't have. In Rolling and Iron with the child loggers one will be able to configure each logger using the |
@tylerjw thank you for investigating this so thoroughly. I think it's fine if Humble doesn't get rosout support, this really seems like a limitation that we shouldn't have to work around with running redundant nodes. From the issue linked in my original description, I've not expected that Humble would work out of the box anyway. Fixing this in MoveIt for Iron/Rolling is very much acceptable imo. As far as I can tell, the current approach doesn't come with any downsides, either. I'm for merging this approach and encourage users to switch to the latest distros. /rosout support is mostly a developer feature, anyway. |
Is this complete and should be closed? |
This is not finished. |
Is your feature request related to a problem? Please describe.
Currently, basically none of our loggers are associated with a ros node. The pattern we decided here will create detached loggers for which no rosout publishers are initiated. See ros2/rclcpp#1694 (comment)
Describe the solution you'd like
I'd like to initialize all loggers from rclcpp nodes, possibly implement child nodes wherever we have child logger namespaces.
If we do that, we'll have the issue that rclcpp::Node needs to be passed to all classes and functions that require logging, requiring many additional API changes and extensions. Question is, can we find a solution that allows us to hide this dependency (possibly like console_bridge), and even pick different logger backends?
The text was updated successfully, but these errors were encountered: