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

fix: apply upstream changes to 'setup_assistant_tutorial' tutorial #678

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rickstaa
Copy link
Contributor

@rickstaa rickstaa commented Nov 10, 2021

This pull request makes sure that all the upstream changes in the moveit MSA and the franka_ros package are applied to the tutorial.

TODOS

  • Update section order to new order of the MSA.
  • Update text to adhere to franka_ros v0.8.1.
  • Update screenshots (using the ready pose).
  • Update simulation section if Add MSA write gazebo urdf feature moveit#2960 is merged.
  • Discuss the coarse and detailed collision models.

This commit makes sure that all the upstream changes in the [moveit MSA](https://github.com/ros-planning/moveit/)
and the [franka_ros](https://github.com/frankaemika/franka_ros) package
are applied to the tutorial.
Also add different screenshots that more clearly display the hand
of the robot, which is kind of hidden in the startup pose.
Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the suggestion in frankaemika/franka_ros#194 (comment), we should add both arm planning groups, panda_arm and manipulator.
In my commit, I added a corresponding section. However, note that the screenshot will slightly change again.
We should also shortly discuss the coarse and detailed collision models.
Finally, I don't like the default robot pose of the Panda, which very much hides the hand.
I suggest taking screenshots using the ready pose instead - as soon as it is defined.

Comment on lines 52 to 59
* Click on the browse button and navigate to the *panda_arm.urdf.xacro* file
installed when you installed the Franka package above. (This file gets installed in
``/opt/ros/noetic/share/franka_description/robots/panda_arm.urdf.xacro`` on Ubuntu
with ROS Noetic) Choose that file.

* Now add **hand:=true** as the *optional xacro arguments and then click *Load Files*.
The Setup Assistant will load the files (this might take a few seconds) and present
you with this screen:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to start with a more generic formulation here and then become more specific. See my commit.

@rickstaa
Copy link
Contributor Author

I reviewed your changes and the text is now more clear. I will re-run the moveit_setup_assistant to check if all the MSA changes work.

Finally, I don't like the default robot pose of the Panda, which very much hides the hand.
I suggest taking screenshots using the ready pose instead - as soon as it is defined.

I also noticed that but I included the screenshots like this since this is what the user sees when he walks through the process. I however can easily change them using the ready pose.

@rhaschke
Copy link
Contributor

That's true. But, I think for a tutorial it is confusing if the hand is never visible but the tutorial talks about it.
When interacting with the rviz display in MSA, the user can easily make the hand visible also in the default pose.

Showing such a view, as I did, will also provide an additional cue to the user, that she is able to change that view herself. Maybe, an explicit comment would be helpful anyway.

@rickstaa rickstaa force-pushed the update_noetic_moveit_setup_assistant branch from 68fac6b to 81f2f8b Compare November 11, 2021 08:10
@rickstaa
Copy link
Contributor Author

I updated some of the screenshots. I will do the rest when frankaemika/franka_ros#194 (comment) is fixed.

We can use https://github.com/rickstaa/msa_panda_config_output to get back to the MSA that was used.

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