Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect frame_id values for Temperature messages? #315

Closed
gavanderhoorn opened this issue Jun 18, 2019 · 5 comments
Closed

Incorrect frame_id values for Temperature messages? #315

gavanderhoorn opened this issue Jun 18, 2019 · 5 comments
Assignees
Labels
bug kinetic Issues with the refactor in Kinetic wrid19 World ROS-Industrial Day 2019

Comments

@gavanderhoorn
Copy link
Member

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.

@gavanderhoorn gavanderhoorn added bug kinetic Issues with the refactor in Kinetic labels Jun 18, 2019
@gavanderhoorn gavanderhoorn added the wrid19 World ROS-Industrial Day 2019 label Jun 28, 2019
@fmauch
Copy link

fmauch commented Jul 2, 2019

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

@gavanderhoorn
Copy link
Member Author

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

Related: #288.

@miguelprada
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Member Author

Fixed with the merge of #320.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug kinetic Issues with the refactor in Kinetic wrid19 World ROS-Industrial Day 2019
Projects
None yet
Development

No branches or pull requests

3 participants