add effort, point, wrench

[k-okada]

add plugin for effort, PointStamped, WrenchStamped

[hershwg]

This looks fantastic, thank you!

[dgossow]

Maybe we'll want to include this into a new hydro branch.

[hershwg]

@k-okada, this is missing point_visual.h and point_visual.cpp. Also, CMakeLists.txt does not seem to include the compilation of point_visual.cpp, wrench_visual.cpp, or effort_visual.cpp.

[hershwg]

@dgossow: we could, but there are already a number of fixes in groovy-devel awaiting release. Since this is strictly an addition I don't see a problem with throwing in into a late-groovy release.

[dgossow]

@hershwg Yeah, you're right. This will not break anything existing. Let's do it like this.

[k-okada]

@hershwg sorry that I forget to add few files. See if this works.

[hershwg]

@k-okada Sorry I lost track of this. Just merged now.

Can you make icons for the new display types? Try to go along with the visual style of the existing icons as much as possible. The icon files go in rviz/icons/classes.

So far I've only been able to see the "effort" display, but that looks fine. I don't have a setup that publishes WrenchStamped or PointStamped just handy. Another nice thing would be if you could add little test scripts to rviz/src/test which publish messages which can be viewed with your new display types. They don't need to be regression tests (though obviously that would be great), they can just be something I could run casually to try it out myself.

Thanks!

[k-okada]

ok, I created test script and icons, see #638
by the way, how do you perform regression tests with this kind of visualization tools. Do you have any good examples?

[hershwg]

I do not have any good examples of regression tests for visualization tools. I have some regression tests for non-visual code, though even they mostly got left behind when I catkin-ized rviz. I know in principle how to do regression tests for the visuals, but it is a bunch more work. You need to set up a test environment with a "headless" X server that doesn't require a graphics card or monitor. Then you open a "window", render stuff into it, read the pixels back and compare them to a known-good image.

It is really nice to have good test coverage like that, it makes you much more confident about making changes, but it is a lot of work on its own, often without any immediate benefit.

[wjwwood]

Ok guys, this code is TOTALLY weird... @k-okada can you explain to me why you basically copied the implementation of the tf/message_filter.h header and then changed it many subtle ways?

https://github.com/ros-visualization/rviz/blob/indigo-devel/src/rviz/default_plugin/effort_display.h#L29-493

Why not fix the upstream version in tf?

https://github.com/ros/geometry/blob/indigo-devel/tf/include/tf/message_filter.h

You are using #include directives at line ~500:

https://github.com/ros-visualization/rviz/blob/indigo-devel/src/rviz/default_plugin/effort_display.h#L496

Which honestly is very bad practice.

I became aware of this because I am having to apply this tf patch to the effort_display.h file:

ros/geometry#23

In order to get rviz to compile... I shouldn't have to fix the same problem in multiple places like this.

Also, I am pretty sure this rviz issue is related to the fact that you are redefining TF macros:

#700

I would argue that this code should be fixed up now or else removed and placed in a separate rviz plugin package.

@k-okada if you can tell me how this class is different from the upstream tf/message_filter.h (the referenced code.ros.org issue is not very useful) then I can try to remove this duplication.

<rant>
You guys should really consider what Technical Debt you are making when you are coding stuff like this, because people like me will end up wasting a morning. Very frustrating.
</rant>

Sorry for the rant, but this is super frustrating... Lets just get this fixed the right way 😄

[wjwwood]

I have put together a temporary fix for Indigo: #763

[k-okada]

the referenced code.ros.org issue is not very useful

I'm sorry that I could not explain more than want I commented on code.ros.org, but effort information (JointState) is published without frame_id, and message_filter throw error when they read such information. I tried to fix this at upstream code (tf/message_fileter.h) but they did not want that idea as described in https://code.ros.org/trac/ros-pkg/ticket/5467.

(https://github.com/ros-visualization/rviz/blob/indigo-devel/src/rviz/default_plugin/effort_display.h#L30)

[wjwwood]

I'm sorry that I could not explain more than want I commented on code.ros.org, but effort information (JointState) is published without frame_id, and message_filter throw error when they read such information.

What do you do differently from the normal message filter in the case that there is no frame_id?

I tried to fix this at upstream code (tf/message_fileter.h) but they did not want that idea as described in https://code.ros.org/trac/ros-pkg/ticket/5467.

I didn't see a patch, was there ever a proposal of how to fix that? Again, none of this tells me how the Message Filter would need to behave in order to meet the use case of the Effort Display. I'm not willing to leave this code the way it is and I cannot fix the problem if I don't understand the problem.

Things I need to know to help:

• Why do joint states not have frame_id's?
• I get that tf's MessageFilter drops messages which do not have a frame_id, but what should it do instead of drop them?
• @tfoote's point from the code.ros.org issue is valid, the MessageFilter in tf returns transformed data, what should it do in the case where there is no frame_id to transform it from?
• Is this a case where the normal message filters (http://wiki.ros.org/message_filters) or no filter should be used?

[mikeferguson]

It would appear the main issue here is that you are really fusing two messages: the joint_states and tf (although, not entirely declaring the tf requirement, it's hidden in the message filter). The visualization depends on the effort value coming from the joint_state message, and then is placed in the view by looking up the frame associated with that joint and doing a tf lookup.

@wjwwood The joint_state message would never have frame_id -- it doesn't belong to any particular frame, only the stamp is filled in within the header.

I'm not sure the right solution here, because this is really unlike any other plugin within RVIZ. I suppose you could do the simple approach and remove the filter altogether and process every joint_states message and let it fail in the tf lookup (but I supposed that might mess with the Status aspects).

[k-okada]

I'm sorry that I could not explain more than want I commented on
code.ros.org, but effort information (JointState) is published without
frame_id, and message_filter throw error when they read such information.

What do you do differently from the normal message filter in the case that
there is no frame_id?

message filter is used to adjust (or synchronize) two different message
queue, but it also checks whether if two messages have frame_id. My code is
to relax error check regarding frame_id.

I tried to fix this at upstream code (tf/message_fileter.h) but they did
not want that idea as described in
https://code.ros.org/trac/ros-pkg/ticket/5467.

I didn't see a patch, was there ever a proposal of how to fix that? Again,
none of this tells me how the Message Filter would need to behave in order
to meet the use case of the Effort Display. I'm not willing to leave this
code the way it is and I cannot fix the problem if I don't understand the
problem.

Things I need to know to help:

• Why do joint states not have frame_id's?

  May be because joint states is not associate with any frame, but at least
  it related to the robot, so I hope it will publish with base frame or
  something, but it did not.

• I get that tf's MessageFilter drops messages which do not have a
  frame_id, but what should it do instead of drop them?

Pass if tf time matches.

• @tfoote https://github.com/tfoote's point from the code.ros.orgissue is valid, the
  MessageFilter in tf returns transformed data, what should it do in the
  case where there is no frame_id to transform it from?

use transform from framd_id in tf

• Is this a case where the normal message filters (
  http://wiki.ros.org/message_filters) or no filter should be used?

normal message filter may work.

[tfoote]

It appears to me that the clean solution to this is that the joint_state should come in and then be processed such that it is associated with the position to render it(position and frame_id). And then that resultant output is passed into a tf::MessageFilter with the appropriate frame_id such that it will be queued until the it is valid to render it.

This will allow the untransformable joint_state datatype to be then be processed as designed by the tf::MessageFilter. There will need to be a datatype designed to hold either all the data to be rendered(my preference), or a reference to all the data to be rendered.