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

Add Isaac Sim support for the Agilus series #47

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

szandara
Copy link

@szandara szandara commented Jan 15, 2024

Before submitting this PR into master please make sure:

If you added a new robot model:

  • you extended the table of verified data in README.md with the new model
  • you extended the CMakeLists.txt of the appropriate moveit configuration package with the new model
  • you added a test_<robot_model>.launch.py and after launching the robot was visible in rviz
  • you added a <robot_model>_joint_limits.yaml file in the config directory (to provide moveit support)

If you modified an already existing robot model:

  • [ x ] you checked and optionally updated the table of verified data in README.md with the changes
  • [ x ] you have run the test_<robot_model>.launch.py and the robot was visible in rviz

Short description of the change

I have simulated the Kuka robot on an Isaac Sim environment. In order to do that I had to add a new TopicBased planning that is consumed by Isaac. I took inspiration from the Pandas robot where they have added this support.

https://github.com/ros-planning/moveit_resources/blob/humble/panda_moveit_config/config/panda.ros2_control.xacro#L7

I have added support only for the Agilus series for now.
If a user sets use_fake_hardware, they can optionally set fake_hardware_type = isaac, which will change the planner to deal with TopicBasedSystem controls.

@szandara szandara requested a review from Svastits as a code owner January 15, 2024 17:58
@szandara szandara changed the title Zandara/isaac Add Isaac Sim support for the Agilus series Jan 15, 2024
@Svastits
Copy link
Member

I approved your PR, thanks for the contribution :)

@gavanderhoorn
Copy link
Contributor

gavanderhoorn commented Jan 16, 2024

If Isaac Sim is like Gazebo (ie: surrogate hw), should it be treated as such?

Having to switch to fake hardware in order to use a simulator is not how it's typically done in ros2_control and/or ROS in general.

Additionally: rather than expansion, composition has worked well in the past. The latter would keep Isaac dependencies out of the main support packages, while the former would need updates to all 'supported' xacro:macros for each simulator/environment you'd like to support. That doesn't really scale.

The UR driver/packages have recently partly made the switch to composition for this reason: the description package(s) were starting to depend on MoveIt, ros2_control and a bunch of Gazebo stuff. For users who "just" want the description, that started to become really bothersome.

Copy link
Member

@Svastits Svastits left a comment

Choose a reason for hiding this comment

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

Blocked until comment from @gavanderhoorn is resolved

@szandara
Copy link
Author

I agree, this does not scale well and that's why I stopped at the Agilus, however the code is not very abstract at the moment. I did not see an easier way without major changes in your code.

I am happy to change it composition but I am not too familiar with xacro. Would you mind pointing me to the UR driver example? I have taken a look but I don't quite understand what you mean. I find on their side a very similar pattern at least judging from here. https://github.com/UniversalRobots/Universal_Robots_ROS2_Driver/blob/main/ur_robot_driver/urdf/ur.ros2_control.xacro

One way I could think of is to move the joint controls into something like this

      <xacro:ur_joint_control_description
        tf_prefix="${tf_prefix}"
        initial_positions="${initial_positions}"
      />

and then I would import this into my own ros control xacro that contains the Isaac bits ?

Regarding the semantic, I am ok in changing this into something like "ros2_controller_type" that leaves fake_hardware as mock_system.

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.

3 participants