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

[bug] tf2_ros::Buffer:transform always assumes timeout #589

Open
Aposhian opened this issue Mar 15, 2023 · 4 comments
Open

[bug] tf2_ros::Buffer:transform always assumes timeout #589

Aposhian opened this issue Mar 15, 2023 · 4 comments
Labels
help wanted Extra attention is needed

Comments

@Aposhian
Copy link

Aposhian commented Mar 15, 2023

Bug report

Required Info:

  • Operating System:
    • Ubuntu 22.04
  • Installation type:
    • binaries
  • Version or commit hash:
    • humble
  • DDS implementation:
    • n/a
  • Client library (if applicable):
    • rclcpp

Steps to reproduce issue

  • don't set setUsingDedicatedThread(true)
  • try to call transform() on a tf2_ros::Buffer
  • see the tf2_ros::threading_error

Expected behavior

I just want to call transform using the latest transform in the tf buffer.

Actual behavior

lookupTransform allows for this, but transform always calls an overloaded version of lookupTransform that takes a timeout, which then checks for a dedicated thread. This means I get spam ERROR logs about not using a dedicated thread.

Additional details

This could be fixed two ways:

  • removing timeout as default argument on transform and making a separate overload that takes an explicit timeout arg
  • on canTransform, only check for threading if the timeout is non-zero.
@Aposhian Aposhian changed the title [bug [bug] tf2_ros::Buffer:transform always assumes timeout Mar 15, 2023
@clalancette clalancette added the help wanted Extra attention is needed label Mar 23, 2023
@clalancette
Copy link
Contributor

This seems like a reasonable feature request. We'd be happy to review a PR that implements this.

@vineet131
Copy link
Contributor

Hi @clalancette
I can help out with this. Does using approach 2 in buffer.cpp look right to you? Something like:

if (!checkAndErrorDedicatedThreadPresent(errstr) && timeout != tf2::Duration(0.0)) {
    return false;
  }

@Aposhian
Copy link
Author

You will want to switch the order of those checks, since checkAndErrorDedicatedThreadPresent logs if it fails the check. If you check for timeout first then it will shortcut and not evaluate the check.

@vineet131
Copy link
Contributor

You will want to switch the order of those checks, since checkAndErrorDedicatedThreadPresent logs if it fails the check. If you check for timeout first then it will shortcut and not evaluate the check.

You're right about that, that's my bad. I preferred approach 2 to approach 1 because those who have implemented this code should be able to transform as is without having to pass another kind of signature. I'll write up a PR for this.

ahcorde pushed a commit that referenced this issue Jul 17, 2023
When the timeout is default (0.0), it should NOT error.
When the timeout is NOT default, it should raise an error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants