Generic Analyser does not process starts_with and remove_prefix on the same name

[mitchellwills]

The Generic Analyser processes starts_with before slashes are replaced with spaces and remove_prefix after the slashes are replaced. This causes issues with the find_and_remove_prefix parameter as it is matched using the slash, but then the prefix is not removed because the slash does not match the space.

For example if a node is in a namespace and has the name /ns/mynode and it generates a status message with the full name ns/mynode: myname

Then you would expect using the following to match and remove the prefix, but it does not

find_and_remove_prefix: 'ns/mynode: '

instead you must use

startswith: 'ns/mynode: '
remove_prefix: 'ns mynode: '

This can be demonstrated by either viewing the /diagnostics_agg topic or using the rqt_gui plugin

[trainman419]

If you can provide a pull request that implements this correctly, I'll gladly merge it.

[mitchellwills]

Hmm I have been thinking about implementing this, but i'm not quite sure what the correct solution is. Which behavior should be considered correct handling the check before or after replacing the slash? If find_and_remove_prefix is changed then should the startswith or remove_prefix (whichever implements it wrong also be changed).

Also just wondering what the reason was for replacing the slash with a space? For me it was confusing that the slash was missing when I first started using the diagnostic_aggregator.

[mitchellwills]

@trainman419 Any thoughts on my questions

[trainman419]

I wasn't around for the initial design of the diagnostics stack, so I'm not sure what the reasons were for removing slashes from diagnostics. I suspect the slashes confuse downstream tools like rqt_robot_monitor, which expect them to delimit namespaces.

I think that remove_prefix is broken for operating on a modified version of the input instead of operating on the raw input. If you look at the code which implements find_and_remove_prefix, it's doing the same thing as remove_prefix, so I think fixing one will implicitly fix the other.

I've gone though all of the packages on the ROS wiki which depend on diagnostic_aggregator, and confirmed that none of them are using a configuration that would be affected by this fix.

[trainman419]

The fix for this is kind of a mess; internally, the diagnostic aggregator converts names to output format (slashes replaced with spaces) before they're ever processed by the aggregator, so the fix for this is to convert the remove_prefix argument to output format before trying to use it.

I've confirmed the fix for this on my test system. Please confirm that it works for you as well, and then I will release the new version.

[mitchellwills]

Works great!
Thanks

[trainman419]

Released to hydro: ros/rosdistro#9105 and indigo: ros/rosdistro#9106