search

ros-industrial / ur_modern_driver

(deprecated) ROS 1 driver for CB1 and CB2 controllers with UR5 or UR10 robots from Universal Robots
Apache License 2.0
302 stars 340 forks source link

Incorrect frame_id values for Temperature messages? #315

Closed gavanderhoorn closed 5 years ago

gavanderhoorn commented 5 years ago

The current implementation (kinetic-devel) uses the joint names as values for the header.frame_id when publishing Temperature messages for the joint (motor) temperatures:

https://github.com/ros-industrial/ur_modern_driver/blob/6b818fa149bcd070131d6b04e5d97e2f67d28684/src/ros/rt_publisher.cpp#L94-L107

joint_names_ is populated here:

https://github.com/ros-industrial/ur_modern_driver/blob/6b818fa149bcd070131d6b04e5d97e2f67d28684/include/ur_modern_driver/ros/rt_publisher.h#L72-L75

with the defaults coming from here:

https://github.com/ros-industrial/ur_modern_driver/blob/6b818fa149bcd070131d6b04e5d97e2f67d28684/include/ur_modern_driver/ros/rt_publisher.h#L37-L38

frame_ids can never be set to names of joints, only to link names, as only the latter will correspond to TF frames.

The intent of the publisher is of course to publish joint temperatures, so using joint_names_ seems like the logical thing to do, but header.frame_id just cannot be used that way, as it violates convention and thus potentially breaks applications.

fmauch commented 5 years ago

I'll use the link names of the frames that actually sit inside the joints.

gavanderhoorn commented 5 years ago

@fmauch just mentioned that the temperature publisher in kinetic-devel appears to be dropping messages.

Related: #288.

miguelprada commented 5 years ago

Thinking about this, I'm not sure if using link names just because frame_id isn't meant for joints is a good idea either. These are joint (motor) temperatures after all, not link temperatures.

I personally prefer to misuse the frame_id field rather than reporting meaningless "link temperatures".

Alternatively, a new message type could be introduced.

gavanderhoorn commented 5 years ago

We discussed this here as well: the idea is that the origins of the links are coincident with the origins of the joints.

We don't know where the temperature sensors are located exactly, but a good assumption would be the joints.

With this in mind it would seem acceptable to use the names of the links for frame_id.

A completely proper solution would be to introduce new links, but right now I would place those at the origins of the links, so that wouldn't really change anything.

gavanderhoorn commented 5 years ago

Fixed with the merge of #320.