search

xqms / rosmon

ROS node launcher & monitoring daemon
Other
180 stars 47 forks source link

Added option to prepend node name to stdout when using --disable-ui. #140

Closed stevegolton closed 3 years ago

stevegolton commented 3 years ago

Added a new option --stdout-print-source which when used in combination with --disable-ui prepends the source (i.e. node name) in parentheses to log lines printed to stdout.

Addresses #139

Background: I use rosmon with --disable-ui and --flush-stdout to run launch files from a systemd service. This is a great combination as the logs can be managed via journalctl. However, the node name is not included in the log line so it makes diagnostics difficult when reviewing the logs. The standard rosmon TUI prints the node name in a coloured box at the start of each log line, so I figured it wouldn't be too far-fetched to do something similar when --disable-ui is used.

Perhaps a format string might be a more flexible option for the future, but this simple switch fixed my issue for now.

xqms commented 3 years ago

Hey, thanks for the contribution!

I think we can make that behavior the default instead of requiring an additional option - I can't see a downside. I think it's highly unlikely that there are people around who parse rosmon's output with --disable-ui...

xqms commented 3 years ago

... and maybe do something similar to the TUI to figure out the maximum node name width, and then print the name in a padded manner? Something to that effect:

fmt::print("{:>{}}: ", name, max_width);
stevegolton commented 3 years ago

Hi @xqms,

Thanks for the feedback!

I think we can make that behavior the default instead of requiring an additional option - I can't see a downside. I think it's highly unlikely that there are people around who parse rosmon's output with --disable-ui...

This sounds good to me. I was a little wary about changing the default output format for parsing reasons as you mentioned, but if you're happy then I'm happy. I'll remove the option.

.. and maybe do something similar to the TUI to figure out the maximum node name width, and then print the name in a padded manner?

Good idea, I'll push these two changes shortly.

stevegolton commented 3 years ago

I wonder if there should be an arbitrary max value on this padding width. It could be quite inconvenient if a user has deeply nested nodes and has to keep scrolling over to the right? I see the TUI uses a quarter of the terminal window as a limit but I guess the --disable-ui option should not rely on a terminal.

xqms commented 3 years ago

I guess we could put down a maximum of 40 characters (half the "default" 80 character terminal width) or something like that... But I'm also fine with merging this as it is and then addressing that if someone speaks up about it ;) What do you think?

stevegolton commented 3 years ago

Sorry about the delay. Yeah, that sounds good to me! Thanks :)