Allow disabling rosout file logging (to rosout.log) #1381
Conversation
|
I still see rosout.log with only something like It is probably because the ROS parameter server isn't immediately ready when rosout is being used. |
|
As you have noticed you can't rely on the parameter server to be available that "early" in the process. So this will need a different approach to configure. |
|
@dirk-thomas I chose this approach because there is another ros parameter it is using, |
That would be fine. It needs to be documented well though in order to be discoverable. |
|
How critical is it to not log at all from startup? IMO using a parameter is reasonable and still suppresses the log-to-file behaviour after only a short period. |
|
I expected the use case to be "don't want to log because don't have a writable fs". We had similar comments before (don't find the reference right now though). |
|
Okay, fair— I know there's been chatter on ROS Answers in the past about redirecting all the output to At least in our case, the scenario is that we already have rosout recorded in bags; recording it a second time is just more files to rotate and manage. |
|
The rosout node is launched by roslaunch internally and the |
|
Wait. I found the reason why I got that first log. That's from rosout itself. Let me redo it. |
|
Actually the ROS parameter is ready when Rosout is constructed. So I am using ros parameter again now, but at earlier stage. In this way, it won't create rosout.log file at all. |
|
Ping reviewers |
| if (!handle_) | ||
| { | ||
| return; | ||
| } |
There was a problem hiding this comment.
It'd be nice if this could handled as a RAII thing when it goes out of scope, but appreciating that you're just trying to patch in a small fix and not rewrite the whole thing, LGTM.
|
Looks good to me. 👍 |
|
@mikepurvis Thanks. I don't have the write access. Can you merger it for me? Also I need to cherry-pick it to lunar-edge. Should that be another pull request? |
|
Let's wait on a verdict from @dirk-thomas as well. |
|
ping @dirk-thomas |
LGTM
Before a new release is being made into older ROS distros all patches in the newer ROS distro are being considered. A PR is being made which contains all backported patches but mentions all considered patches. Feature requests have in general a smaller chance of being backported though. @yli-cpr Please also add documentation about the new parameter to the wiki with a note that it will be available as of Melodic (for now). Please also post a link to the diff here for future readers to find the documentation. |
|
Wiki page updated: http://wiki.ros.org/rosout. But I don't know how to make it melodic specific. |
|
@yli-cpr Please see http://wiki.ros.org/action/info/rosout?action=diff&rev2=52&rev1=51 how to use the |
|
Thanks!
…On Tue, May 29, 2018 at 1:15 PM, Dirk Thomas ***@***.***> wrote:
@yli-cpr <https://github.com/yli-cpr> Please see
http://wiki.ros.org/action/info/rosout?action=diff&rev2=52&rev1=51 how to
use the Version macro for this.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1381 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AdFyBxREwVQyUjsq8YAwSMstcC97aOCDks5t3YI8gaJpZM4TuBmI>
.
--
Yong Li
Senior Software Developer
1425 Strasburg Rd. Suite 2A
Kitchener, ON N2R 1H2, Canada
yli@clearpath.ai
www.clearpathrobotics.com
|
See issue #1380.