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

LaserScan does not get display in 1.10.15 #762

Closed
jihoonl opened this issue May 7, 2014 · 16 comments
Closed

LaserScan does not get display in 1.10.15 #762

jihoonl opened this issue May 7, 2014 · 16 comments

Comments

@jihoonl
Copy link

jihoonl commented May 7, 2014

I confirmed that 1.10.14 displays laser scan properly. It seems like transform is not applied.

Version 1.10.14
14
Version 1.10.15
15

@wjwwood
Copy link
Member

wjwwood commented May 7, 2014

@jihoonl can you provide a bag file for reproducing this?

@wjwwood
Copy link
Member

wjwwood commented May 7, 2014

@tfoote please hold the sync of hydro if you haven't already done it.

@wjwwood
Copy link
Member

wjwwood commented May 7, 2014

This change seems to be the most likely candidate: 9d5c5a8

@vrabaud have you noticed this or have you seen laser scans working with your patch?

@mikeferguson
Copy link
Contributor

Just checked out latest source to check this out -- and found that if you set color transformer to Intensity, the points don't get transformed -- they just end up at the origin (set the points to very large, and you can see them sitting there). If you select flat or axis color, things work for me.

Tested on a recent robot, and also on this bagfile which you can use: http://fergy.me/slam/2010-10-29-armadillo-cci.bag

@mikeferguson
Copy link
Contributor

And by latest source, I mean 1.10.15 (I had a whole different game of fun when I forgot to switch to the hydro branch, cause the indigo branch builds but doesn't run under hydro)

@jihoonl
Copy link
Author

jihoonl commented May 7, 2014

I can also see that correct points if I set color transformer as other than intensity.

@wjwwood
Copy link
Member

wjwwood commented May 7, 2014

I can confirm that this is due to 9d5c5a8:

screen shot 2014-05-07 at 3 31 06 pm

The top left rviz has 9d5c5a8 reverted and the bottom right one does not.

@wjwwood
Copy link
Member

wjwwood commented May 7, 2014

This seems like a candidate:

1779817#diff-406fe6df120988d84a9f6657c12573daR853

@wjwwood
Copy link
Member

wjwwood commented May 7, 2014

Yeah, that is the problem, it seems like there is an issue with the calls to PointCloudTransformer::transform and the new fallback behavior is being used.

If I disable the check of the res, and therefore make it so that the corresponding code block never gets called, the problem is fixed.

@vrabaud should we just remove the if (!res) case or should we fix it? Maybe we just need another check for arbitrary types, i.e. not just position and color? What is the purpose of that check?

@wjwwood
Copy link
Member

wjwwood commented May 7, 2014

Since this only affects visualization of intensity for lasers, and presumably non position or color fields in point clouds I am going to wait to iterate with @vrabaud rather than reverting the commit and releasing immediately.

Does that sound ok @jihoonl @mikeferguson?

@mikeferguson
Copy link
Contributor

That strategy is fine for my use cases (I don't use intensity) but obviously can't speak for everyone.

@wjwwood
Copy link
Member

wjwwood commented May 7, 2014

Well I should also point out that this is not in public debians for hydro yet either, indigo it is, but I imagine that affects less people at the moment.

@jihoonl
Copy link
Author

jihoonl commented May 7, 2014

As long as it does not go into public, I am good too!

vrabaud added a commit to vrabaud/rviz that referenced this issue May 11, 2014
@vrabaud
Copy link
Contributor

vrabaud commented May 11, 2014

ok, I made a PR to fix it: the original intent was to only do a resize without a default value if possible (I believe it is faster).

wjwwood added a commit that referenced this issue May 11, 2014
@wjwwood
Copy link
Member

wjwwood commented May 11, 2014

I am going to backport #764 to hydro and then could you guys check to make sure that it addresses you problem? I am confident it will though because I tested it locally.

Thanks again to you guys for testing it all out, and thanks to @vrabaud for fixing it up.

vrabaud added a commit that referenced this issue May 12, 2014
@wjwwood
Copy link
Member

wjwwood commented May 12, 2014

hydro back port: #766

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants