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

tf expiration param #1

Merged
merged 16 commits into from Apr 15, 2013

Conversation

@dawonn
Copy link
Contributor

dawonn commented Mar 24, 2013

The tf expiration was future-dated and then ignored.
I added a param for the setting, rather than the existing hard-coded solution.

@tfoote

This comment has been minimized.

Copy link

tfoote commented Mar 24, 2013

Diff looks good +1

@hershwg

This comment has been minimized.

Copy link
Member

hershwg commented Mar 25, 2013

The tf_delay_ member variable is never initialized, so if the rosparam is not set, it's value could be anything. I suppose the default should be 0, since the offset was previously ignored, otherwise you'll change behavior.

Also, would you mind starting a new pull request with just this one commit? This pull request has 14 empty commits and it's kind of distracting.

Thanks

@hershwg

This comment has been minimized.

Copy link
Member

hershwg commented Apr 15, 2013

I see you actually did provide a default, which looks fine. Never mind about cleaning up the commit history, I can't be bothered myself.

hershwg added a commit that referenced this pull request Apr 15, 2013
@hershwg hershwg merged commit 35505e2 into ros-perception:master Apr 15, 2013
130s pushed a commit to 130s/slam_gmapping that referenced this pull request Jun 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.