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

Multi-Gazebo Kobukis not updating orientations #38

Closed
jihoonl opened this issue Sep 23, 2014 · 15 comments
Closed

Multi-Gazebo Kobukis not updating orientations #38

jihoonl opened this issue Sep 23, 2014 · 15 comments
Labels

Comments

@jihoonl
Copy link
Collaborator

jihoonl commented Sep 23, 2014

KGP - Kobuki Gazebo Plugin

  • Start two gazebo kobukis (segbot + kobuki is ok)
  • Send commands to cmd_vel of one kobuki
  • KGP catches these and moves around properly in gazebo world
  • KGP does not post rotations to the odom topic (translations are there)
  • KGP does not publish rotation information on base_footprint-odom tf.
@jihoonl jihoonl added the bug label Sep 23, 2014
@jihoonl
Copy link
Collaborator Author

jihoonl commented Sep 23, 2014

@slivingston
Copy link
Contributor

#31 that I opened in May 2014 is also related.

@stonier stonier changed the title tf orientation does not get updated properly when two kobuki models are spawned in one world. Multi-Gazebo Kobukis not updating orientations Nov 17, 2014
@stonier
Copy link
Member

stonier commented Nov 17, 2014

  • Kobuki is manually calculating its differential drive output
  • There is a gazebo diff drive plugin - should we be using that?

@bit-pirate
Copy link
Contributor

There is a gazebo diff drive plugin - should we be using that?

If you do that, then you need to split things up. I.e. develop a second plugin which handles Kobuki's sensor I/O.

@stonier
Copy link
Member

stonier commented Nov 19, 2014

Historical point : we didn't use that diff drive plugin because it wasn't there when the kobuki gazebo plugin was made.

In conclusion: doesn't sound like work worth chasing that gives us any extra benefits.

@slivingston
Copy link
Contributor

@jihoonl , I think the correction is to use tf::Quaternion::setRPY() instead of setEuler(). Try applying the patch below. Note there are many different conventions in use for "Euler angles", which makes the setEuler() function somewhat ambiguous. Quick links to relevant tf API documentation are:

For comparison, the differential drive plugin in the gazebo_ros_pkgs repository uses setRPY. The preceding link is for the tip of indigo-devel branch of gazebo_ros_pkgs at time of writing.

I have tested the below patch for the case of spawning two Kobuki models without using the rocon infrastructure.

diff --git a/kobuki_gazebo_plugins/src/gazebo_ros_kobuki_updates.cpp b/kobuki_gazebo_plugins/src/gazebo_ros_kobuki_updates.cpp
index 81340fc..17ac762 100644
--- a/kobuki_gazebo_plugins/src/gazebo_ros_kobuki_updates.cpp
+++ b/kobuki_gazebo_plugins/src/gazebo_ros_kobuki_updates.cpp
@@ -105,7 +105,7 @@ void GazeboRosKobuki::updateOdometry(common::Time& step_time)
   odom_.pose.pose.position.z = 0;

   tf::Quaternion qt;
-  qt.setEuler(0,0,odom_pose_[2]);
+  qt.setRPY(0,0,odom_pose_[2]);
   odom_.pose.pose.orientation.x = qt.getX();
   odom_.pose.pose.orientation.y = qt.getY();
   odom_.pose.pose.orientation.z = qt.getZ();

@jihoonl
Copy link
Collaborator Author

jihoonl commented Mar 2, 2015

@slivingston Thanks. I will test and get back to the issue.

@slivingston
Copy link
Contributor

Update: The above patch is not sufficient, and indeed, we would not expect it to be because there is apparently correct behavior in the case of a single Kobuki. I have nearly identified the bug and will report soon.

@jihoonl
Copy link
Collaborator Author

jihoonl commented Mar 2, 2015

Yeah I have just tested the patch is not sufficient to fix this issue.

To test this in easy way with concert,

Example Concert : Office sim Solution

  1. gazebo_concert with two turtlebot.
  2. add concert_service_indoor_2d_map_prep/indoor_2d_map_prep service.[The up-to-date source as of now!]
  3. run Concert Make a Map through rocon_remocon

@slivingston
Copy link
Contributor

The bug is due to the different Kobuki plugins using the same ImuSensor. I am creating a patch now.

@jihoonl
Copy link
Collaborator Author

jihoonl commented Mar 2, 2015

Awesome Thanks! :)

@slivingston
Copy link
Contributor

diff --git a/kobuki_gazebo_plugins/src/gazebo_ros_kobuki_loads.cpp b/kobuki_gazebo_plugins/src/gazebo_ros_kobuki_loads.cpp
index 3e0ef50..5bf4ff1 100644
--- a/kobuki_gazebo_plugins/src/gazebo_ros_kobuki_loads.cpp
+++ b/kobuki_gazebo_plugins/src/gazebo_ros_kobuki_loads.cpp
@@ -295,7 +295,7 @@ bool GazeboRosKobuki::prepareIMU()
     return false;
   }
   imu_ = boost::dynamic_pointer_cast<sensors::ImuSensor>(
-            sensors::SensorManager::Instance()->GetSensor(imu_name));
+     sensors::get_sensor(world_->GetName()+"::"+node_name_+"::base_footprint::"+imu_name));
   if (!imu_)
   {
     ROS_ERROR_STREAM("Couldn't find the IMU in the model! [" << node_name_ <<"]");

The basic problem is that the sensor name, as obtained on line 289 of gazebo_ros_kobuki_loads.cpp is not scoped. Thus it is the same when multiple models are spawned using the same URDF file, namely kobuki_gazebo.urdf.xacro, which declares the imu name at line 186. Hence, the sensor obtained by sensors::SensorManager::Instance()->GetSensor() is subject to a race condition. I propose avoiding this by using the above patch, which gets the sensor using a fully scoped name that includes the name of the model associated with the Kobuki Gazebo plugin.

The relevant Gazebo API documentation is gazebo::sensors::get_sensor().

@jihoonl
Copy link
Collaborator Author

jihoonl commented Mar 2, 2015

@slivingston Could you make a pull request?

@slivingston
Copy link
Contributor

I just opened #39

@jihoonl
Copy link
Collaborator Author

jihoonl commented Mar 2, 2015

This has been resolved witih #39

@jihoonl jihoonl closed this as completed Mar 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants