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

Remap joint states instead of re-publish #192

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

rhaschke
Copy link
Contributor

@rhaschke rhaschke commented Nov 7, 2021

Re-publishing joint_states via joint_state_publisher (a python app) is highly inefficient.
IMO remapping would be the right way to go instead - as long nobody needs the published topics on the namespace
franka_state_controller / franka_gripper.

But, remapping has its drawbacks too:

  • It breaks existing software relying on topics franka_state_controller/joint_states and/or franka_gripper/joint_states
  • joint_states messages from arm and gripper are published both to the same topic, so we cannot easily distinguish them anymore (w/o looking at the message of course)
  • The variable message format (7 vs. 2 joints) might confuse existing software, e.g. rqt_plot.

Note: This PR includes #191.

@rhaschke
Copy link
Contributor Author

@frankaemika, what do you think of this idea? Nobody ever commented on this for 9 months.

@rickstaa
Copy link
Contributor

rickstaa commented Sep 4, 2023

@rhaschke, thank you for your work on improving the franka_ros package's performance! I greatly appreciate the changes made in this pull request and tried to review it to speed up the merge process.

During my review, I encountered some issues that I believe are important to address:

  1. Unrecognized 'remap' Tags: When I attempted to run roslaunch franka_gazebo panda.launch, I received warnings about unrecognized 'remap' tags within the <include> tag.

  2. Simulation Breakage: The simulation is broken as no messages are being published on the /joint_states topic. To temporarily address this, I moved the 'remap' command outside of the <include> tag. However, this results in two separate messages being published sequentially—one for arm joint states and another for gripper joint states.

Considering that using the joint_state_publisher is sub-optimal, perhaps we could explore a custom C++ joint publisher solution. This would involve merging arm and gripper joint states and republishing them. We might take inspiration from tools like relay.cpp in the ROS repository.

@gollth, @@FE-EnricoSartori I think @rhaschke raised a vital performance improvement opportunity. I'm interested to know what you think about this change 🤔.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants