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

Added ROS parameter instead of gflag to ros_gz_sim::create #463

Closed
wants to merge 2 commits into from

Conversation

yaswanth1701
Copy link

@yaswanth1701 yaswanth1701 commented Nov 16, 2023

🎉 New feature

Closes #459

Summary

Replaced all gflag arguments in ros_gz_sim::create with ROS parameters.

Test it

can try running following command
To launch gazebo sim:
ros2 launch ros_gz_sim gz_sim.launch.py
To spawn a model:
ros2 run ros_gz_sim create --ros-args -p world:=”default” -p file:='https://fuel.ignitionrobotics.org/1.0/openrobotics/models/Gazebo'

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@yaswanth1701 yaswanth1701 changed the title Ros2 rosparm Added ROS parameter instead of gflag to ros_gz_sim::create Nov 16, 2023
Copy link
Collaborator

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

Do you mind to sign the commit ?

Comment on lines 45 to 46
// declaring ros param

ros2_node->declare_parameter("world","");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// declaring ros param
ros2_node->declare_parameter("world","");
// declaring ros param
ros2_node->declare_parameter("world","");

Copy link
Author

Choose a reason for hiding this comment

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

Yeah I can do that, sorry this was my first time and forgot to do that before. I will make the changes and will push again.

Comment on lines 41 to 43
std::string world_,file_,param_,string_,topic_,name_;
bool allow_renaming_;
double x_,y_,z_,R_,P_,Y_;
Copy link
Collaborator

Choose a reason for hiding this comment

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

In general we use underscore at the end when the variables are attributes of class, do you mind to remove the undercore at the end of tha variable name ?

Suggested change
std::string world_,file_,param_,string_,topic_,name_;
bool allow_renaming_;
double x_,y_,z_,R_,P_,Y_;
std::string world_, file_, param_, string_, topic_, name_;
bool allow_renaming_;
double x_, y_, z_, R_, P_, Y_;

Copy link
Author

Choose a reason for hiding this comment

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

Hi @ahcorde , I have made the changes could you please review again. Thank you.

Signed-off-by: Yaswanth <[email protected]>
@azeey
Copy link
Contributor

azeey commented Dec 21, 2023

We have a PR similar to this in #465. That PR does not remove the gflags processing, so I'd be in favor of closing this and reviewing that.

@yaswanth1701
Copy link
Author

Hi @azeey yes, you can close this PR. There are few issues with my PC , So didn't got chance to work on this again, sorry for the trouble.

@azeey
Copy link
Contributor

azeey commented Dec 21, 2023

Okay, thanks for your contribution though!

@azeey azeey closed this Dec 21, 2023
@yaswanth1701 yaswanth1701 deleted the ros2_rosparm branch December 21, 2023 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Make ros_gz_sim::create use ROS parameters
3 participants