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

remove turtlesim velocity and use Twist msg #5

Merged
merged 4 commits into from
Feb 5, 2013
Merged

remove turtlesim velocity and use Twist msg #5

merged 4 commits into from
Feb 5, 2013

Conversation

jihoonl
Copy link
Contributor

@jihoonl jihoonl commented Feb 4, 2013

Turtlesim with Twist message

@chadrockey
Copy link
Member

Is geometry_msgs missing from the package.xml and CMakeLists? Otherwise, each substitution looks good. Will the tutorial pages need updated?

+1 to changing "turtle1/command_velocity", etc to "turtle1/cmd_vel"

Does everyone think that allowing the 'Pose' message to continue is fine? Otherwise, we would have to introduce quaternion logic at this point... and pull in tf to do getYawFromQuaternion and getQuaternionFromYaw, etc.

@jihoonl
Copy link
Contributor Author

jihoonl commented Feb 4, 2013

I was unsure whether I should use geometry_msgs/Pose. Because Turtlesim/pose was providing both positions and velocity.

@chadrockey
Copy link
Member

The standard for Pose + Velocities is nav_msgs/Odometry:
http://www.ros.org/doc/api/nav_msgs/html/msg/Odometry.html

However, like I said, orientation is given as a quaternion, so I'm not sure if it's worth including in the tutorials. On the other hand, users have to get used to quaternions eventually. :)

@dirk-thomas
Copy link
Member

How is odometry related to that?

@chadrockey
Copy link
Member

It's the most standard message we have for a robot reporting its position, orientation, linear velocities, and angular velocities. The turtlesim/Pose message is a simplified nav_msgs/Odometry.

@jihoonl
Copy link
Contributor Author

jihoonl commented Feb 5, 2013

@dirk-thomas Both Odometry and turtlesim/Pose are providing position and velocity.
@chadrockey I think it is better to leave turtlesim/Pose as it is. Since turtlesim is for very newbies, Odometry may introduce another difficulty to them. So I think it would be better to keep turtlesim simple but lead to next level. I think Odometry can be introduced in the intermediate level tutorial.

@dirk-thomas
Copy link
Member

This issue is about the velocity message and topic name. Not about the pose and we should keep it that way.

@dirk-thomas
Copy link
Member

Code changes look fine. But turtlesim does not depend on package which provides the Twist msg.

@jihoonl
Copy link
Contributor Author

jihoonl commented Feb 5, 2013

package.xml and Cmakelist updated. I wonder why it was compiling without this dependency though.

@jihoonl
Copy link
Contributor Author

jihoonl commented Feb 5, 2013

it was not alphabetic order...

dirk-thomas added a commit that referenced this pull request Feb 5, 2013
remove turtlesim velocity and use Twist msg
@dirk-thomas dirk-thomas merged commit fc19df4 into ros:groovy-devel Feb 5, 2013
dirk-thomas added a commit that referenced this pull request Feb 15, 2013
wjwwood added a commit that referenced this pull request Feb 26, 2013
@wjwwood wjwwood mentioned this pull request Feb 26, 2013
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.

3 participants