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

Adding bearing angle image to pcl #198

Merged
merged 1 commit into from
Jan 21, 2014

Conversation

ZhuangYanDLUT
Copy link
Contributor

To represent a 3D laser point cloud in a more simplified way, a Bearing Angle(BA) model was employed to transform a 3D point cloud into a 2D image in pcl/bearing_angle, so that image processing methods can be used in many aspects.

Bearing Angle is defined as the angle between the laser beam and the segment connecting two consecutive measurement points (see Fig. 1). Grey values for a Bearing Angle image are proportional to the reflected laser light energy, which depends not only on how far away the measured object is from the scanner but also on the surface characteristics of the object.
bearing angle
Fig. 1 The Bearing Angle is calculated along the diagonal direction

@jspricke
Copy link
Member

Could you please reuse pull requests, instead of opening up a new one every time? Just force push into the same branch.


/** \Based on the theta, calculate the gray value of every pixel point */
unsigned char
pcl::BearingAngle::getGray (double theta)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need this as a separate method, it's just one line.

@jspricke
Copy link
Member

  • Your test is not a unit test. Please read other tests and documentation on how to do it.
  • Please move your classes into the common/range_image.

@ZhuangYanDLUT
Copy link
Contributor Author

Please help me check them.

Cheers
Yan

@ZhuangYanDLUT
Copy link
Contributor Author

Any questions?

/** \brief Members: float x, y, z, unsigned char gray
* \ingroup common
*/
struct PointWithGray
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this to point_types.h or better use one of the structs defined there?

@ZhuangYanDLUT
Copy link
Contributor Author

I have moved "struct PointWithGray" to point_types.h, please help me check them.

Cheers
Yan

@@ -162,6 +162,11 @@
*/
struct PointXYZINormal;

/** \brief Members: float x, y, z, unsigned char gray
Copy link
Member

Choose a reason for hiding this comment

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

Can you add more explanation what gray should be? unit, use case..

@ZhuangYanDLUT
Copy link
Contributor Author

Please help me check them. Thanks.

Cheers
Yan

@ZhuangYanDLUT
Copy link
Contributor Author

Any questions?


/** \transform 3D point cloud into a 2D Bearing Angle(BA) image */
void
BearingAngleImage::generateBAImage (std::vector< std::vector<pcl::PointXYZ> > &point_cloud, int cloud_width, int cloud_height)
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it make sense to create these image for other point clouds than PointXYZ? I would propose to use a template here. Also, the typical type in PCL is a PointCloud, not a std::vector ;). You could check if the point cloud is dense, so you don't have to pass in the width and height.

@ZhuangYanDLUT
Copy link
Contributor Author

Cheers
Yan

@jspricke
Copy link
Member

HI Yan, your patch failed on Travis, could you please fix this? Thanks!

@ZhuangYanDLUT
Copy link
Contributor Author

According to your guide, I have fixed the patch.
However, it ran timeout.
Please help me to fix it.

Thanks!
Yan

@jspricke
Copy link
Member

Timeouts are ok, that's a limitation of the setup. But as you can see here in the log [1], your unit test didn't worked.

[1] https://s3.amazonaws.com/archive.travis-ci.org/jobs/13194638/log.txt

@ZhuangYanDLUT
Copy link
Contributor Author

The clang was finished successfully, but the gcc interrupted again because of timeout.

@ZhuangYanDLUT
Copy link
Contributor Author

Any questions?

points.clear ();
}

/** \calculate the angle between the laser beam and the segment joining two consecutive measurement points */
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to repeat documentation comments inside source files. doxygen will pick them up from header files.

@ZhuangYanDLUT
Copy link
Contributor Author

Cheers
Yan

@ZhuangYanDLUT
Copy link
Contributor Author

Any questions ?

@jspricke
Copy link
Member

jspricke commented Dec 9, 2013

Hi,

Please don't bump issues on the tracker. It's in the list and we will
come back.

Cheers Jochen

@ZhuangYanDLUT
Copy link
Contributor Author

I'm very sorry for my incomprehension to it.

Cheers
Yan

@rbrusu
Copy link
Member

rbrusu commented Dec 16, 2013

I'm not entirely sure we need a new global point type, if this is only used by this algorithm. Perhaps we can definite it only in the class where it gets used?

@ZhuangYanDLUT
Copy link
Contributor Author

Yes, I agree with it and had defined it in my class before jspricke commented (Please refer to the above part.)

I will fix it.

Thank you.
Yan

@jspricke
Copy link
Member

I would rather propose to export it to PointXYZRGBA in that way we would be compatible with the other tools in PCL.

@ZhuangYanDLUT
Copy link
Contributor Author

Do you mean I should use PointXYZRGBA in point_types.h instead of PointWithGray I have defined ?

@jspricke
Copy link
Member

Yes, as you are defining a color anyhow, you can just set all channels to the same value.

@jspricke
Copy link
Member

Hi wnl, can you please attribute the commit with your real name? Also, can you remove the commented lines? Otherwise I'm fine with this patch.

@ZhuangYanDLUT
Copy link
Contributor Author

How to remove all commented lines?

a = p1.squaredNorm ();
b = (p1 - p2).squaredNorm ();
c = p2.squaredNorm ();
// a = sqrt (point1.x * point1.x + point1.y * point1.y + point1.z * point1.z);
Copy link
Member

Choose a reason for hiding this comment

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

I was referring to these lines.

@ZhuangYanDLUT
Copy link
Contributor Author

Any questions?

jspricke added a commit that referenced this pull request Jan 21, 2014
Adding bearing angle image to pcl
@jspricke jspricke merged commit 376d3d4 into PointCloudLibrary:master Jan 21, 2014
@taketwo
Copy link
Member

taketwo commented Jan 25, 2014

I guess we were to fast with merging this, there are complaints in pcl-users already: http://www.pcl-users.org/Problem-building-from-trunk-pcl-commons-td4031835.html.

@ZhuangYanDLUT
Copy link
Contributor Author

Oh, I'm so sorry for my carelessness.

Thanks for your selfless help and work.

Cheers
Yan

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

Successfully merging this pull request may close these issues.

4 participants