-
Notifications
You must be signed in to change notification settings - Fork 311
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
Make Panda robot Gazebo compatible #126
Conversation
This reverts commit dfe405e.
<xacro:unless value="${not connected_to}"> | ||
<joint name="${arm_id}_joint_${connected_to}" type="fixed"> | ||
<joint name="virtual_joint" type="fixed"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would make sense to add ${arm_id}_
to the virtual_joint, else you can get an error that virtual_joint
is not unique when using multiple robots in the same environment or importing into moveit setup assistant (MSA). The dual arm example works on its own because of namespace, but if you import dual_panda_example.urdf.xacro
into MSA you will get an error.
@simonbogh, are you authorized to merge PRs in this repo? |
Hi, sorry for the late response, and thanks for the contribution! I tested Gazebo on Noetic, and everything worked fine. The robot is moving quite slowly however, which is causing On Melodic, we ran into an issue with |
It probably needs fine-tuning. The idea is to provide a baseline that could be also adapted by the other robots. After merging this PR, "Panda in Gazebo" becomes a dedicated use-case that people can solve in particular.
Yes. moveit/panda_moveit_config#68 is the PR that applies the changes suggested in https://ros-planning.github.io/moveit_tutorials/doc/gazebo_simulation/gazebo_simulation.html. You may want to use that PR or manually apply the changes on a freshly generated |
@tahsinkose Thanks a lot for creating this pull request and the accompanying documentation. When it merges this pull request allows me to switch back from my local forks to the upstream |
@fwalch probably you didn't receive notification on my reply at #126 (comment) since I didn't explicitly mention you. I believe PID tuning in Noetic needs to be separately addressed after enabling native MoveIt support. What do you think? |
@fwalch, this was because I adjusted velocity and acceleration scalars to be 0.1 😆 I made them 1.0 again in moveit/panda_moveit_config@c3eed9c#diff-0b89fdfad571d8e4315bc8654941cd7bed8155ed97b9a71756716619333566efR250. |
…azebo * commit 'ef756fb937df84ea8a0d1df305d01124bf93bd38': (35 commits) FIX: Remove TODO CHANGE: Use default controller configs too avoid code duplication ADD: Default case if joint type is unknown ADD: Example Launch file to franka_gazebo ADD: TODO regarding NE/EE/K Frames FIX: Conditional Build for older Gazebo Versions (kinetic) CHANGE: Lookup equilibrium_pose topic relatively ADD: Lint & Format franka_gazebo FIX: Insert default case for `segment` CHANGE: Lookup contact & collision thresholds similar to franka_hw FIX: Use `eigen_conversions` to convert KDL::Frame -> Eigen::Affine3d FIX: Remove TODOs ADD: Gravity Tests for franka_gazebo::ModelKDL CHANGE: Move math util functions as static functions in header CHANGE: Split initialization of FrankaHWSim into sub-methods ADD: Dependencies to Docker FIX: Review Comments FIX: Remove outdated non-template methods FIX: Naming for private member of ModelKDL::chain CHANGE: Move util methods inside class namespace ...
Hello and thanks for your contribution on this topic. We had a close look at your changes and found them very valuable in simulating the robot with Gazebo. However due to some internal processes we are not able to merge this directly. Rather we integrated your changes together with some other features in a complete Gazebo integration of franka_ros. Next to the necessary URDF changes this includes also:
Hopefully this will allow you to simulate existing ROS controllers now in Gazebo as well. Check out our documentation for more information. Since this feature is now implemented I will close this PR now. Thanks for your effort! |
@gollth Thanks a lot for this feature improvement! It takes many things on my to-do list that I planned to implement for my research (i.e. gravity composition, better collision meshes, etc.). Can I maybe ask you some questions about your implementation?
Demo.launch warnings[ WARN] [1628672207.286446541]: Link panda_leftfinger has visual geometry but no collision geometry. Collision geometry will be left empty. Fix your URDF file by explicitly specifying collision geometry.
[ WARN] [1628672207.286477598]: Link panda_rightfinger has visual geometry but no collision geometry. Collision geometry will be left empty. Fix your URDF file by explicitly specifying collision geometry. |
@gollth Can I ask you one small other question? I was wondering why you are publishing the |
Thanks you @rickstaa for your interest in this topic.
Regarding the |
@gollth Thanks a lot for the fast response and the clear explanation.
|
@gollth I added one last pull request that is related to point 2 of my initial post:
This pull request #154 gives users the ability to switch between a mesh based |
@gollth can I ask you one last question about the I looked through both the libfranka and franka_ros code to see where you used the collision geometries defined in the After this search, I came to the conclusion that the (when working with the real robot) collision properties in the If I understood the above correctly, that means that the motions that are allowed are different when working with the simulated one compared to the real one. This means that if I train a RL algorithm in simulation, it might learn motions that are restricted in the real robot. The only way to solve this discrepancy is to create collision geometries, similar to those used in the FCI, that do not cause earlier mentioned instabilities in the gazebo simulation. |
In that case I think merging #137, #139, #145 and #147 would possibly improve user experience. For #153, it currently only changes the Additionally, you could merge #154 to give users the ability to switch to simple meshes instead of collisions. This however is up to you, as users can also just make these changes themselves if they really need them. I am not sure about #150, #151 and #152 which I created to solve warnings thrown by MoveIt when no collision geometry is found in any of the joints that are controlled. I do not know which solution is the best, as I don't know the exact collision geometries that are used in the FCI. Maybe you can the team can take a look if solving these warning is desired. |
Continues from #52. I've rebased existing work onto
melodic-devel
. Changes can be listed as:.xacro
files. Although one may find this structure slightly complicated, I believe it makes any future additions trivial-ish. Besides overall file lengths dropped slightly.arg
use_cylinder_collision_model
for the.launch
-time specification of collision model. It defaults to true. In MoveIt we need meshes. See usage at https://github.com/tahsinkose/panda_moveit_config/blob/c88412d75651e66dcfecae5b877e6d3c910d2c7c/launch/gazebo.launch#L16-L17.arg
use_gazebo_sim
. There is one major and one minor reason. Major one is that I did not want thexacro
process to waste time on loading irrelevant sections. Minor one is the fact thatmimic
joints are not properly treated by Gazebo. Hence that field needs to be absent when launching robot for Gazebo.