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

Publish odom twist in child_frame_id #28

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mkoval
Copy link

@mkoval mkoval commented Aug 12, 2015

This node currently publishes the twist component in the world frame, instead of the body frame. This differs from the nav_msg/Odometry message description, which says:

The twist in this message should be specified in the coordinate frame given by the child_frame_id

This pull request fixes this issue by making segway_rmp publish twists in the child_frame_id. It also fixes a bug that caused yaw rate to be reported as z-down. We had to make both of these changes to successfully use the output of segway_rmp as an input to the robot_localization package (see cra-ros-pkg/robot_localization#221).

@wjwwood
Copy link
Member

wjwwood commented Aug 13, 2015

Perhaps this is the change makes the code more correct, but it is also a very breaking behavioral change. Some people may be relying on this behavior, wrong as it is. Since I don't use this anymore, I'll defer to the other maintainers which use it.

@acarrillo
Copy link

I also bumped into this for my own project about a year ago, and it left me
headdesking pretty hard. I ended up making my own filter function that
properly mutated the message fields before passing it onwards to
robot_localization. Doing this right might be one of those band-aids where
the sooner it's ripped off, the better! If others have run into this issue
before in their projects, they probably remember it, since it's rather
insidious, so the question is whether there's a loud, clear way to
communicate the change to the right parties should someone decide to run
their code against the latest segway_rmp package. Even after hacking to
make the system work, I would welcome a change that requires me to un-do my
hack.

Just my $0.02!

Best,
-- Alex C.

On Thu, Aug 13, 2015 at 1:55 PM, William Woodall notifications@github.com
wrote:

Perhaps this is the change makes the code more correct, but it is also a
very breaking behavioral change. Some people may be relying on this
behavior, wrong as it is. Since I don't use this anymore, I'll defer to the
other maintainers which use it.


Reply to this email directly or view it on GitHub
#28 (comment).

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.

4 participants