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

Package to run andino_isaac #9

Merged
merged 12 commits into from
Dec 6, 2023
Merged

Package to run andino_isaac #9

merged 12 commits into from
Dec 6, 2023

Conversation

JCarosella
Copy link
Collaborator

@JCarosella JCarosella commented Dec 5, 2023

Closes #3

This converts the repository into a ROS2 package that has the ability to run isaac with a given world and robot via a launchfile.

  • Isaac worlds should be placed in the "isaac_worlds" package directory
  • Robot should be placed in the "andino_isaac_description" package directory
  • The launch command to run is: ros2 launch andino_isaac andino_isaac.launch.py --world_name plain_world.usda --robot_name just_andino.usda (this will load the default arguments)

@JCarosella JCarosella self-assigned this Dec 5, 2023
Copy link
Collaborator

@BarceloChristian BarceloChristian left a comment

Choose a reason for hiding this comment

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

Added a low LOGAF comment, otherwise it looks cool.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd ignore .thumbs

@@ -0,0 +1,62 @@
import argparse
Copy link
Collaborator

Choose a reason for hiding this comment

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

Indentation looks weird, please use spaces and check the number of spaces you're using.
Also, would be good to be able to select headless (True/False), and renderer (RayTracedLighting, PathTracing) but that can be added to a separate ticket.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#13

Copy link
Collaborator

@francocipollone francocipollone left a comment

Choose a reason for hiding this comment

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

Greeat! Some minor comments

# Paths to places
pkg_andino_isaac_path = get_package_share_directory('andino_isaac')
user_home_path = os.path.expanduser("~")
isaac_install_path = os.path.join(user_home_path, ".local/share/ov/pkg/isaac_sim-2023.1.0")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is user dependant. Create an issue to find this automatically. (Same thing for hte LD Libraty Path)
Alternatively we could have a launch argument to provide the path to the isaac installation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can go a little further, checking if the ~/.local/share/ov/pkg/ exists, getting all the folders starting with isaac_sim-, sort by name, and use latest (In fact, you're not using 2023.1.0-hotfix, so this wouldn't work on most users computers since the version you're using is discouraged). Alternatively, if the path mentioned above does not exist, we should check /isaac-sim which is install folder in Nvidia's container.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#14

Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the difference between just_andino and andino_isaac?
Can we rename the files to be more descriptive? Also information in the readme about the available usda might super useful

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will be added in #11

@JCarosella
Copy link
Collaborator Author

@BarceloChristian @francocipollone Comments addresesd. I'll create the requested issues once this is merged

Copy link
Collaborator

@francocipollone francocipollone left a comment

Choose a reason for hiding this comment

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

Go for it 🚀

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.

Launch Isaac via ROS 2 launch file
3 participants