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

rviz::MessageFilterDisplay has a hardcoded subscriber queue size #1035 #1113

Closed

Conversation

dkaznacheev
Copy link
Contributor

MessageFilterDisplay base class had a hardcoded queue size for message_filters::Subscriber and tf::MessageFilter fields, so I created a separate IntProperty in MessageFilterDisplay for this purpose.
I am also interested if it's possible to make it an adjustable parameter in rviz global options; if so, I could try to fix it.

@@ -0,0 +1,65 @@
/*
* Copyright (c) 2012, Willow Garage, Inc.
Copy link

@krinkin krinkin Jun 17, 2017

Choose a reason for hiding this comment

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

do you work for Willow Garage???

Copy link
Member

Choose a reason for hiding this comment

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

It would be appropriate, @dkaznacheev, to add your own copyright here.

@wjwwood wjwwood added this to the untargeted milestone Jun 19, 2017
@wjwwood
Copy link
Member

wjwwood commented Jun 19, 2017

Thanks for the pull request. I'll get to reviewing and merging it asap.

@dhood dhood self-assigned this Aug 2, 2017
Copy link
Contributor

@dhood dhood left a comment

Choose a reason for hiding this comment

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

thanks @dkaznacheev for the PR. In general it looks good, there are just some style changes requested and a question about the need to unsubscribe/resubscribe

virtual void updateQueueSize()
{
tf_filter_->setQueueSize( (uint32_t) queue_size_property_->getInt() );
unsubscribe();
Copy link
Contributor

Choose a reason for hiding this comment

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

did you add the unsubscribe and resubscribe calls because you found that they were necessary? most places (e.g. marker display) just call setQueueSize and nothing else, but that might be what's beneath issue #1135.

src/rviz/default_plugin/effort_display.h Show resolved Hide resolved
src/rviz/default_plugin/effort_display.h Show resolved Hide resolved
src/rviz/default_plugin/effort_display.h Show resolved Hide resolved
src/rviz/message_filter_display.h Show resolved Hide resolved
src/rviz/message_filter_display.h Show resolved Hide resolved
src/rviz/message_filter_display.h Show resolved Hide resolved
src/test/queue_size_test.cpp Show resolved Hide resolved
StefanFabian added a commit to StefanFabian/rviz that referenced this pull request Nov 21, 2017
Apparently keeping "Willow Garage, Inc" (ros-visualization#1142) was not the right thing (ros-visualization#1113).
dhood pushed a commit that referenced this pull request Dec 11, 2017
* Added check for invalid quaternions.

* Updated copyright

Apparently keeping "Willow Garage, Inc" (#1142) was not the right thing (#1113).

* Relaxed tolerance as discussed in #1167
@dhood
Copy link
Contributor

dhood commented Apr 11, 2018

@dkaznacheev did you have any time to follow up on the requested changes so this feature can get merged for other users?

@wjwwood wjwwood removed this from the untargeted milestone May 10, 2018
@rhaschke
Copy link
Contributor

Replaced with #1428.

@rhaschke rhaschke closed this Sep 22, 2019
130s pushed a commit to 130s/rviz that referenced this pull request Aug 21, 2024
…tion#1113) (ros-visualization#1129)

Signed-off-by: Yadunund <yadunund@openrobotics.org>
(cherry picked from commit 68d9167758b651d2e4c01f4aeb74a9613110e751)

Co-authored-by: Yadu <yadunund@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants