-
Notifications
You must be signed in to change notification settings - Fork 125
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
Added ros_ign_gazebo for ros2 #80
Conversation
Signed-off-by: ahcorde <ahcorde@gmail.com>
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.
Thanks for tackling this so quickly, @ahcorde ! I tried it with Eloquent and it worked for me. It would be nice to document the supported versions on the README. Also, I'm not sure why Travis wasn't triggered 🧐 I see that it's supposed to install dashing
.
I created the branch ros2 to keep it as the devel branch.
Would that be for eloquent
and foxy
?. Could we target this PR at dashing
? Usually on Ignition we target PRs at the lowest compatible version and merge changes forward (as opposed to how on ROS commits are cherry-picked backwards).
Some other requests that could come in follow-up PRs:
- Could you add the package to
ros_ign/package.xml
? - Can we use the new launch file on
ros_ign_gazebo_demos
? We should still leave the existing one there for a while in case someone is using it, but we shouldn't encourage its use anymore.
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
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.
Everything is working for me, I'd just like to hear your thoughts about:
- not splitting gui / server launch files
- not using a bash script
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
I don't have a strong opinion here. I created two launch files just to keep the same idea that we have in Gazebo and provide a launch file to run ign-gazebo headless mode (without reviewing/knowing the arguments). But Gazebo is a different case where we have two executables in Ignition we only have one. The idea of duplicate the arguments sounds weird and it's going to create issues in the user side. I think is a good idea to handle everything in the same launch file.
the bash script is not adding any value I can remove it |
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
…to ahcorde/add/ros_ign_gazebo
This org hadn't granted permissions. There's a build running now: https://travis-ci.org/github/ignitionrobotics/ros_ign/builds/698585638 |
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
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.
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
This package contains things that make it convenient to integrate ROS with Ignition, such as:
Run ignition Gazebo
Spawn entities
By the way this command is adding the entity to ignition gazebo, but the mesh doesn't appear.
Note: I created the branch
ros2
to keep it as the devel branch.Signed-off-by: ahcorde ahcorde@gmail.com