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.

Sign up for GitHub

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

Jump to bottom

Do not timeout when waiting for transforms #146

Merged
merged 3 commits into from
Aug 7, 2019
Merged

Do not timeout when waiting for transforms #146

merged 3 commits into from
Aug 7, 2019

Conversation

jacobperron
Copy link
Member

@jacobperron jacobperron commented Aug 6, 2019 •
edited
Loading

Previously, a value of zero was causing timeouts to happen immediately.

Resolves #145

@jacobperron
Do not timeout when waiting for transforms
c114b9b 
Previously, a value of zero was causing timeouts to happen immediately.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron mentioned this pull request Aug 6, 2019
Merged
@jacobperron
Copy link
Member Author

This fix should resolve an issue with the RViz message filter display ros2/rviz#375 (comment)

@jacobperron jacobperron mentioned this pull request Aug 6, 2019
Closed
@jacobperron
Copy link
Member Author

As discussed earlier, and also mentioned here, maybe it makes sense to expose the timeout as a parameter to MessageFilter.

@wjwwood @tfoote Thoughts?

@jacobperron jacobperron added the in review Waiting for review (Kanban column) label Aug 6, 2019
wjwwood
wjwwood approved these changes Aug 6, 2019
View reviewed changes
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

lgtm as a default

As discussed earlier, and also mentioned here, maybe it makes sense to expose the timeout as a parameter to MessageFilter.

I would say so. You never really want an unconfigurable timeout.

@jacobperron
Make buffer lookup timeout a parameter to MessageFilter
2a4e1f9 
The parameter defaults to std::chrono::duration::max().

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Add test for construction/destruction of MessageFilter objects
0c317aa 
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Copy link
Member Author

I've parameterized the buffer lookup timeout via the constructor (2a4e1f9) and added tests (0c317aa). @wjwwood PTAL.

@jacobperron
Copy link
Member Author

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@jacobperron jacobperron merged commit 2e69278 into ros2 Aug 7, 2019
@delete-merged-branch delete-merged-branch bot deleted the jacob/message_filter_no_timeout branch August 7, 2019 02:08
@crdelsey crdelsey mentioned this pull request Aug 7, 2019
Closed
@mikaelarguedas mikaelarguedas mentioned this pull request Aug 7, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wjwwood wjwwood wjwwood approved these changes

Assignees
No one assigned
Labels
in review Waiting for review (Kanban column)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Default waitForTransform timeout of 0ns in message filter results in 100% transform timeout
2 participants
@jacobperron @wjwwood