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

Add a tf_buffer_cache_time_ns to tf_wrapper #792

Merged
merged 3 commits into from
Nov 3, 2021

Conversation

ivanpauno
Copy link
Member

Alternative to #780

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno added the enhancement New feature or request label Nov 3, 2021
@ivanpauno ivanpauno self-assigned this Nov 3, 2021
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

Passing a parameter override doesn't seem to be working for me. I'm trying:

rviz2 --ros-args -p tf_buffer_cache_time_msg:=7

however the parameter appears unchanged:

$ ros2 param get /rviz tf_buffer_cache_time_ms  
Integer value is: 10000

Also, if there's a display already added in my rviz config, I see the following error on startup:

[ERROR] [1635957603.774472100] [rviz2]: Could not load display config: parameter 'tf_buffer_cache_time_ms' has already been declared

I've tried with the CameraDisplay and OdometryDisplay, both cause the error.

@ivanpauno
Copy link
Member Author

@jacobperron try:

rviz2 --ros-args -p tf_buffer_cache_time_ms:=50000
ros2 param get /rviz tf_buffer_cache_time_ms

that works for me (you seem to have written _msg instead of _ms.

Also, if there's a display already added in my rviz config, I see the following error on startup:

Nice, I will take a look

@jacobperron
Copy link
Member

you seem to have written _msg instead of _ms

🙃 yeah, that was it. Working now...

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno
Copy link
Member Author

Also, if there's a display already added in my rviz config, I see the following error on startup:

b0001fa fixed the issue to me

Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

LGTM with green CI


Here's another fun one I just noticed: if you have the "add display" dialog open it must block the executor thread since ros2 param get hangs until the dialog is closed.

I don't think this needs to be fixed; certainly not for this PR.

@clalancette
Copy link
Contributor

Here's another fun one I just noticed: if you have the "add display" dialog open it must block the executor thread since ros2 param get hangs until the dialog is closed.

Yuck. Agreed that we don't have to solve it here, but do you mind opening a separate issue to track that?

@ivanpauno
Copy link
Member Author

Here's another fun one I just noticed: if you have the "add display" dialog open it must block the executor thread since ros2 param get hangs until the dialog is closed.

Interesting ...
Yes, that seems unrelated.

@jacobperron
Copy link
Member

jacobperron commented Nov 3, 2021

Yeah, I can open an issue. I suppose it's affecting subscriptions too (e.g. I guess displays do not update if we have a dialog open). Maybe this is desired? I don't know what the behavior is in ROS 1.

@jacobperron
Copy link
Member

Here's a ticket: #793

@ivanpauno
Copy link
Member Author

CI:

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

@ivanpauno ivanpauno merged commit 0423b7b into ros2 Nov 3, 2021
@ivanpauno ivanpauno deleted the ivanpauno/tf-buffer-cache branch November 3, 2021 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants