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

Allow choosing between geometry_msgs/Twist and #22

Closed
wants to merge 1 commit into from

Conversation

corot
Copy link
Contributor

@corot corot commented May 17, 2015

geometry_msgs/TwistStamped (issue #21)

// **** Are velocity input messages stamped?
// if false, will subscribe to Twist msgs on /vel
// if true, will subscribe to TwistStamped msgs on /vel
if (!nh_private_.getParam ("stamped_vel", stamped_vel_))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where do we tell the Parameter Server that stamped_vel_ parameter is true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a parameter in my launch file. As by default it expects stamped Twist messages, nobody will notice the change unless adding stamped_vel as true in the launch file

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, so the launch file is what the users of this package makes by themselves, right? If that's the case, this needs documented somewhere (maybe adding to README or equivalent?) otherwise no one notices this feature even if this feature might be valuable to them.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I added a parameter in demo.launch to raise awareness of how to activate the proposed feature. https://github.com/ccny-ros-pkg/scan_tools/pull/35/files#diff-da85c9e3d31b840300d30555c2129fc8R6

@130s
Copy link
Collaborator

130s commented Nov 10, 2015

travis config is now added. Triggering it by reopening.

@130s 130s closed this Nov 10, 2015
@130s 130s reopened this Nov 10, 2015
@130s 130s mentioned this pull request Nov 10, 2015
@130s
Copy link
Collaborator

130s commented Nov 10, 2015

I re-opened this in #35 to adjust to the current default branch, as well as utilize travis. Thanks!

@130s 130s closed this Nov 10, 2015
130s added a commit that referenced this pull request Nov 10, 2015
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.

None yet

3 participants