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

Add node name to the output in rosout.log #918

Closed
wants to merge 1 commit into from
Closed

Add node name to the output in rosout.log #918

wants to merge 1 commit into from

Conversation

patrickcjh
Copy link
Contributor

@patrickcjh patrickcjh commented Oct 28, 2016

Fixes #912.

@dirk-thomas dirk-thomas changed the title Add node name to the output in rosout.log. Fix #912. Add node name to the output in rosout.log Oct 28, 2016
@dirk-thomas
Copy link
Member

@ros-pull-request-builder retest this please

@IanTheEngineer
Copy link
Contributor

@commaster90 can you give us a sample of what the text output was before, and a sample of what your change will produce?

@@ -141,7 +141,8 @@ class Rosout
ss << msg->level << " ";
}

ss << "[" << msg->file << ":" << msg->line << "(" << msg->function << ") ";
ss << msg->name << " ";
ss << "[" << msg->file << ":" << msg->line << "(" << msg->function << ")] ";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make this change more visible. In this line the missing ] has been added. Without the change the brackets are unbalanced.

@patrickcjh
Copy link
Contributor Author

This is an example output in ~/.ros/latest/rosout.log before the change.

1477675270.252137195 INFO [/tmp/binarydeb/ros-indigo-roscpp-tutorials-0.5.5/talker/talker.cpp:111(main) [topics: /rosout, /chatter] hello world 2
1477675270.351831526 INFO [/tmp/binarydeb/ros-indigo-roscpp-tutorials-0.5.5/talker/talker.cpp:111(main) [topics: /rosout, /chatter] hello world 3
1477675270.451983685 INFO [/tmp/binarydeb/ros-indigo-roscpp-tutorials-0.5.5/talker/talker.cpp:111(main) [topics: /rosout, /chatter] hello world 4
1477675270.551896309 INFO [/tmp/binarydeb/ros-indigo-roscpp-tutorials-0.5.5/talker/talker.cpp:111(main) [topics: /rosout, /chatter] hello world 5
1477675270.651884679 INFO [/tmp/binarydeb/ros-indigo-roscpp-tutorials-0.5.5/talker/talker.cpp:111(main) [topics: /rosout, /chatter] hello world 6

And this will be the output in ~/.ros/latest/rosout.log after the change.

1477674754.711808141 INFO /talker [/tmp/binarydeb/ros-indigo-roscpp-tutorials-0.5.5/talker/talker.cpp:111(main)] [topics: /rosout, /chatter] hello world 2
1477674754.811829213 INFO /talker [/tmp/binarydeb/ros-indigo-roscpp-tutorials-0.5.5/talker/talker.cpp:111(main)] [topics: /rosout, /chatter] hello world 3
1477674754.911385469 INFO /talker [/tmp/binarydeb/ros-indigo-roscpp-tutorials-0.5.5/talker/talker.cpp:111(main)] [topics: /rosout, /chatter] hello world 4
1477674755.011573405 INFO /talker [/tmp/binarydeb/ros-indigo-roscpp-tutorials-0.5.5/talker/talker.cpp:111(main)] [topics: /rosout, /chatter] hello world 5
1477674755.110618167 INFO /talker [/tmp/binarydeb/ros-indigo-roscpp-tutorials-0.5.5/talker/talker.cpp:111(main)] [topics: /rosout, /chatter] hello world 6

@IanTheEngineer
Copy link
Contributor

That makes sense, but I think I'm a bit confused. I thought the output format was dictated by the rosconsole config file:
http://wiki.ros.org/rosconsole#Console_Output_Formatting
It appears that it was never used in rosout.log at all. Your change seems like a sensible default, but could break any code that attempts to parse this file. I've written a script like this in the past, but I'm not particularly bothered, as it would be easy to fix. However, if we obeyed the console output formatting in the rosout log, then users could easily configure to add / remove console output formatting fields as they desire.

@patrickcjh
Copy link
Contributor Author

@IanTheEngineer It is my concern too that this might break other code that parse rosout.log. Therefore, I would like to seek opinion here.

I notice that the rosconsole config file you mentioned affects only the output shown on the console, and the config can be different for every node. It however does not log any output to file. It is also a bit confusing because for Python nodes, individual log files for each nodes are generated in the ~/.ros/latest folder (whereas C++ nodes do not have individual logs). Meanwhile, rosout is running within roscore, and it aggregates the log from every nodes and write it to a single rosout.log

@IanTheEngineer
Copy link
Contributor

Ah, that is nearly true. The rosconsole var only affects the stdout if you are running with rosrun, or with a roslaunch node set to output=console. But if you are setting roslaunch output=log, then this rosconsole config environment var dictates the output fields of each line in the *-stdout.log.
Anyway, that's getting into the weeds a bit. I have observed that the rosout.log ignores this configuration regardless, as is now obvious by looking at this file that you updated. I am not opposed to your change, but think we should open a ticket to address this inconsistency between log formatting.

@dirk-thomas
Copy link
Member

I have cherry-picked your patch to the newly created lunar-devel branch: bc9222a

Thank you!

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

Successfully merging this pull request may close these issues.

3 participants