Add rosconsole echo

[efernandez]

This PR adds a new command to rosconsole that allows to print the logger messages from the /rosout or /rosout_agg topic using only the CLI.

This new command is echo, so we can simply call rosconsole echo to see all the logger messages. The command takes multiple optional arguments that modify its behaviour, as shown below:

$ rosconsole echo -h
usage: rosconsole echo [-h] [-1] [--topic TOPIC] [--nocolor] [-d]
                       [FILTER] [LEVEL]

Print logger messages

positional arguments:
  FILTER         regular expression to filter the logger name (default: .*)
  LEVEL          minimum logger level to print (default: warn)

optional arguments:
  -h, --help     show this help message and exit
  -1, --once     prints one logger message and exits
  --topic TOPIC  topic to read the logger messages from (default: /rosout)
  --nocolor      output without color
  -d, --detail   print full logger details

This video shows how it works using two test nodes from https://github.com/efernandez/rosconsole_echo_test:

The two positional arguments supports auto-complete hints thanks to ros/ros#168.

This is slightly inspired on @guillaumeautran 's rosdiagnostic tool. I tried to make it closer to how the other rosconsole commands are though, and also auto-complete friendly.

[efernandez]

FYI @mikepurvis @dirk-thomas

[dirk-thomas]

The CI failure is unrelated to this patch and has been addressed by #1325. Please rebase this branch to the latest commit of the targeted branch so that CI can pass.

[dirk-thomas]

It seems it would make sense to use argparse for the whole script. But I wouldn't couple that with the feature you are adding here. It is certainly good to use it for the new echo verb.

[efernandez]

Do you mean I should make it local to the function where it's being used?

I honestly don't think it makes a big difference, but I can do that if that's what you're requesting here.

[dirk-thomas]

Sorry, for not being clearer. I wasn't referring to the location of the import statement. Instead I was trying to point out that the first command line argument of rosconsole is inspected "manually" by looking at sys.argv and then argparse is only used for the new echo verb. It would be nice if argparse would be used for all arguments including the decision/selection based on the verb.

But that is clearly outside the scope of this PR.

[efernandez]

Gotcha! Yes, that makes a lot of sense, but definitely not a change for this PR.

[dirk-thomas]

Often such an option is called -v / --verbose.

[efernandez]

Changed.

[dirk-thomas]

Deleting the local variable doesn't do anything. Since the script is exited after this function call anywhere there is nothing which needs to be done. As an alternative - if the goal is to also be able to call this function without exiting right after it - the RosConsoleEcho class could have a deregister method which stops the subscription.

[efernandez]

Removed.

[dirk-thomas]

Since both positional arguments filter and level are optional it is not very clear how to pass e.g. only a level. Maybe consider to change either of them into -- option.

[efernandez]

The main reason I made them positional arguments is because I didn't know how to provide auto-complete hints with rosbash when I have something like -l (or --level).

I initially had --filter and --level. If I only have --level, do you if it's possible to have auto-complete hints in ros/ros#168?

[dirk-thomas]

I think it is possible. Manually implemented completion is usually a 🤐 In newer code I usually rely on argcomplete but that would be a drastic change.

I will leave it up to you if you want to change the positional arguments to options.

[efernandez]

I've had a quick look at bash completion and it seems it's definitely possible. I don't know how easy and clean it'd be though.

Since I really like the idea of having --level, so we can change the level only, I'd give it a try.
I'll come back to you soon, after updating both PRs.

[efernandez]

Changed to -l (--level) and update the rosbash auto-complete function to support it in ros/ros#168

[dirk-thomas]

You could avoid the definition of STRING_LEVEL with the following code:

self._level = getattr(Log, options.level.upper())

[efernandez]

Cool! Done.

[dirk-thomas]

In order to reduce the two variables LEVEL_STRING_NO_COLOR and LEVEL_STRING_COLOR you could use the following more compact logic (just written in the browser):

LEVEL_COLOR = {
    'DEBUG': 92,
    'INFO': 97,
    'WARN': 93,
    'ERROR': 91,
    'FATAL': 95,
}
self._level_string_map = {getattr(Log, level): level.ljust(5) for level in LEVEL_COLOR.keys()}
if not options.nocolor:
    [
        self._level_string_map[getattr(Log, level)] = '\033[' + str(color) + 'm' + self._level_string_map[getattr(Log, level)] + '\033[0m'
        for level, color i LEVEL_COLOR.items()
    ]

In the (unlikely) event of another level being added in the future the lines to update would be much lower.

[efernandez]

Makes sense.

I've done a different implementation though. Let me know if it looks good to you.

And some minor fixes related to the order of the levels when printed by argparse when the user introduces a wrong level.

[efernandez]

@dirk-thomas I feel I should change this to string = level.ljust(max([len(level) for level in LEVEL_COLOR.keys()]))

Sounds good?

[efernandez]

Done.

Also rebased onto lunar-devel, although the tests were already passing before that.

[efernandez]

Let me know if there is something else to do, or otherwise I'll squash all commits into one before you merge this.

[dirk-thomas]

Thank you!