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

rosgraph_msgs Log message type. #50

Closed
mlautman opened this issue Nov 14, 2018 · 8 comments
Closed

rosgraph_msgs Log message type. #50

mlautman opened this issue Nov 14, 2018 · 8 comments
Labels
enhancement New feature or request

Comments

@mlautman
Copy link

I am working on a port of rqt_console with @dirk-thomas which uses the rosgraph_msgs/Log message. I see that rosgraph_msg/Log hasn't been ported to ros2 yet and I was wondering if I should implement the Log msg or if there was a specific reason for not porting Log in rosgraph_msgs. As this is more of a design decision, I am happy to move the conversation to discourse if you believe that this should be a more general discussion.

@tfoote
Copy link
Contributor

tfoote commented Nov 14, 2018

I don't believe that there's any reason that it hasn't been ported except that it hasn't been needed yet.

I would suggest coordinating with @nburek

He's working on several PRs right now to start publishing console outputs:
ros2/rclcpp#582
ros2/rcl#327
ros2/rcutils#127

@nburek
Copy link
Contributor

nburek commented Nov 14, 2018

Hey Mike,

I am currently working on adding support for logging to a "rosout" topic and as part of that have ported over the Log message from rosgraph_msg. The message I'm working with looks mostly the same except that I've adjusted the level constants to match the enum values from the rcl code and I've removed the list of topics in the message as it didn't make sense to me that you would want that list with every log message.

I was also looking at moving the message from "rosgraph_msg" into "common_interfaces/diagnostic_msgs" as logs seemed better grouped as a diagnostics tool than a rosgraph tool.

I'll start working on getting a pull request created for my Log message port (it may take a couple of days to go through my company's open source approval process before I can officially make the pull request).

I've updated the discourse topic discussing ROS2 logging with this information: https://discourse.ros.org/t/ros2-logging/6469

@dirk-thomas
Copy link
Member

Since the message should be used by rcl please add the Log message to rcl_interfaces package. That one is a dependency of rcl already. Putting the message in common_interfaces would add another dependency to rcl and require common_interfaces to be released before rcl which isn't desired.

@nburek
Copy link
Contributor

nburek commented Nov 14, 2018

Ok, makes sense. I'll add it to rcl_interfaces instead.

@tfoote
Copy link
Contributor

tfoote commented Nov 14, 2018

For consistency I'd suggest keeping the same package + name too.

@dirk-thomas
Copy link
Member

For consistency I'd suggest keeping the same package + name too.

Imo rosgraph_msgs doesn't make any sense for the Log message. So I don't think keeping that package name is a good idea.

@clalancette clalancette added the enhancement New feature or request label Nov 15, 2018
@nburek
Copy link
Contributor

nburek commented Nov 15, 2018

Pull request for the Log msg file: #53

@brawner
Copy link
Contributor

brawner commented Feb 6, 2020

@mlautman @nburek This issue can be closed with #53 merged, yah?

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

No branches or pull requests

6 participants