Use RViz node clock in tools

[hidmic]

Precisely what the title says. Without this patch, a default clock (i.e. a clock hooked up to system time) is always used, not abiding to use_sim_time.

CI up to rviz_default_plugins:

• Linux

[wjwwood]

lgtm, it would be good to mention this in the release notes and to try it out locally if you haven't done that already.

[hidmic]

it would be good to mention this in the release notes

Which release notes? Foxy's at index.ros.org? I plan to backport this to Eloquent (at least) too, should we also update those release notes?

try it out locally if you haven't done that already.

Timestamps look like they should locally 👍

[wjwwood]

Yes the foxy release notes.

This changes ABI, so I wouldn’t push it to eloquent myself, why does it need that?

[hidmic]

This changes ABI

Argh, yes, I changed a member field.

I'm using Eloquent elsewhere and having RViz tools working with Gazebo is practical at least. I can build from source though.

[clalancette]

Argh, yes, I changed a member field.

As an ABI-compatible alternative, maybe you can not store clock_ and instead just always do raw_node_->get_clock() where you need it?

[hidmic]

Good idea, I'll open another PR after merging this one that does that. Unless @wjwwood you'd rather have the non-ABI breaking change everywhere (despite extra indirections).

[wjwwood]

Good idea, I'll open another PR after merging this one that does that. Unless @wjwwood you'd rather have the non-ABI breaking change everywhere (despite extra indirections).

I don't have a strong preference. I guess having to fetch it each time might be a performance issue, but I don't know that for sure.

[hidmic]

Ok, two patches then.

[hidmic]

Release notes PR: ros2/ros2_documentation#568

[hidmic]

I'll go ahead and merge, as CI on other platforms won't reveal much (for this particular case).