gazebo_ros_api_plugin cleanup #1137
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove an unused overload of publishSimTime and add doxygen for the remaining publishSimTime function. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
The /clock topic is advertised in both loadGazeboRosApiPlugin and advertiseServices. This removes the code from advertiseServices and moves it earlier in loadGazeboRosApiPlugin. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
clalancette
approved these changes
Jul 22, 2020
| @@ -193,7 +193,6 @@ void GazeboRosApiPlugin::loadGazeboRosApiPlugin(std::string world_name) | |||
|
|
|||
| gazebonode_ = gazebo::transport::NodePtr(new gazebo::transport::Node()); | |||
| gazebonode_->Init(world_name); | |||
| //stat_sub_ = gazebonode_->Subscribe("~/world_stats", &GazeboRosApiPlugin::publishSimTime, this); // TODO: does not work in server plugin? | |||
There was a problem hiding this comment.
Since this is commented out, we also don't need the void GazeboRosApiPlugin::publishSimTime(const boost::shared_ptr<gazebo::msgs::WorldStatistics const> &msg) signature below, so it makes sense to remove both.
Comment on lines
-252
to
-254
| // publish clock for simulated ros time | ||
| pub_clock_ = nh_->advertise<rosgraph_msgs::Clock>("/clock",10); | ||
|
|
There was a problem hiding this comment.
And now this code is duplicate because we already do this in the main loadGazeboRosApiPlugin
Comment on lines
206
to
208
| // Manage clock for simulated ros time | ||
| pub_clock_ = nh_->advertise<rosgraph_msgs::Clock>("/clock",10); | ||
|
|
There was a problem hiding this comment.
By moving this above advertiseServices, we get exactly the same behavior as before, but without the code duplication.
Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
|
@osrf-jenkins run tests please |
|
@ahcorde I merged your suggestion |
ahcorde
approved these changes
Aug 17, 2020
scpeters
added a commit
to scpeters/gazebo_ros_pkgs
that referenced
this pull request
Aug 27, 2020
Remove an unused overload of publishSimTime and add doxygen for the remaining publishSimTime function. * Remove duplicate code for /clock advertisement The /clock topic is advertised in both loadGazeboRosApiPlugin and advertiseServices. This removes the code from advertiseServices and moves it earlier in loadGazeboRosApiPlugin. Signed-off-by: Steve Peters <scpeters@openrobotics.org> Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
scpeters
added a commit
that referenced
this pull request
Sep 2, 2020
Remove an unused overload of publishSimTime and add doxygen for the remaining publishSimTime function. * Remove duplicate code for /clock advertisement The /clock topic is advertised in both loadGazeboRosApiPlugin and advertiseServices. This removes the code from advertiseServices and moves it earlier in loadGazeboRosApiPlugin. Signed-off-by: Steve Peters <scpeters@openrobotics.org> Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed with @clalancette that
gazebo_ros_api_pluginadvertises the/clocktopic and sets some variables in multiple places (loadGazeboRosApiPlugin() and advertiseServices()). This removes that duplicate code fromadvertiseServices()and makes sure is called beforeadvertiseServices().There's also an unused private function that is an overload of
publishSimTime(), so I've removed it here and added doxygen for the remainingpublishSimTime().