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

hiding small wrenches should be optional #1196

Conversation

jgueldenstein
Copy link
Contributor

The default behavior for wrench messages was to hide the visualization if the length of the vector was smaller than the width. This pull request adds a boolean property to enable/disable this feature as discussed in #1081.

I also renamed the function updateColorAndAlpha() to updateProperties(). The name made no sense anymore (even before) because scale and width are also updated here. The new name clarifies that several properties are updated.

@v4hn

@v4hn
Copy link
Contributor

v4hn commented Mar 27, 2018

@wjwwood friendly ping. This has been discussed in the issue, CI succeeded and it works perfectly in our setup. Would be great to see this merged soon.

@dhood
Copy link
Contributor

dhood commented Apr 6, 2018

@jgueldenstein, @v4hn thank you for the PR addressing #1081

This change will break ABI (because it adds a member to the WrenchDisplay class). This means we won't be able to release it into indigo/kinetic/lunar, so will have to hold it for when we release rviz into melodic.

@dhood dhood added this to the first melodic release milestone Apr 6, 2018
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.

Looks good, thank you. Will wait until a separate branch is created for melodic to merge though.

@v4hn
Copy link
Contributor

v4hn commented Apr 7, 2018 via email

@dhood dhood changed the base branch from kinetic-devel to melodic-devel April 30, 2018 17:47
@wjwwood wjwwood added the review label May 10, 2018
@wjwwood
Copy link
Member

wjwwood commented May 10, 2018

Melodic is only failing due to a (now fixed) deprecation warning unrelated to this pr. So I'll merge.

@wjwwood wjwwood merged commit 9061807 into ros-visualization:melodic-devel May 10, 2018
@ghost ghost removed the review label May 10, 2018
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.

4 participants