Revise inheritance between HDL and VPL grabbers.

[SergioRAgostinho]

Arose during the review of #1738. Both classes need a cleanup.

[UnaNancyOwen]

Add new base class VelodyneGrabber, and derive HDLGrabber and VLPGrabber from base class?

[SergioRAgostinho]

Exactly. I've started to work on this an it's currently a slow burner.

[UnaNancyOwen]

Thanks 👍

[UnaNancyOwen]

Velodyne has released a new LiDAR (VLP-32C). and They will release a new LiDAR (VLS-128) in 2018 years. We may need to support these LiDAR in the future.

[SergioRAgostinho]

Thanks for the update. If you're interested in helping, I can publish what I currently have in my fork and one of these weekends we can make a sprint for it.

[aryasenna]

Hi @SergioRAgostinho and @UnaNancyOwen it's been 2 years and I have just noticed that so far the VLP grabber class is still limited to VLP16.

When you feed the class with VLP 32 packets, the grabber still displays the point cloud, albeit incorrectly and truncated to 16 layers.

Does this indicate a relatively simple fix?

I noticed that VLP classes differs from its parent HDL class basically due to vlp16_vertical_corrections[] and VLP_MAX_NUM_LASERS constant.

I am tempted to change VLP_MAX_NUM_LASERS to 32 and fill hdl32_vertical_corrections[], but I'm quite sure the latter won't work well since it is physics-dependent.

So I have access to this sensor, is there any way I can help?

@SergioRAgostinho in your fork repo, I don't see any VLP32 support, do you have it locally?

[aryasenna]

Aha, VeloView has some info about vertical corrections: https://github.com/Kitware/VeloView/blob/cbdbe68cede38caf5d5dbf2d14dd9519a51a6024/share/VLP-32c.xml

Anyone know how to interpret this to PCL's vlp32_vertical_corrections[]?

Does the value inside <vertCorrection_></vertCorrection_> tags correspond to PCL's vertical correction, unit and dimensionallity-wise?

[stale[bot]]

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.

[kunaltyagi]

@SergioRAgostinho would it be possible to create a writeup so we can mark this as a todo for someone with more time (and hardware) with them?

[stale[bot]]

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.

[lincy27]

@aryasenna, were you able to utilize the VLP grabber with VLP 32 by modifying certain parameters? Is there any workaround for using VLP grabber class with data captured from VLP 32? While using VLP grabber with VLP 32 data, were you noticing reflections (mirroring effect) in the visualizer?

[aryasenna]

Hi @lincy27

Sorry for late reply, Yes that is the case, I derived the class from HDL grabber, and created VLP32 grabber, there is some basic parameters like v_res and h_res, but there is specific Velodyne vertical correction that we just have type out (see above).

While using VLP grabber with VLP 32 data, were you noticing reflections (mirroring effect) in the visualizer?

You mean using VLP grabber as-is without modifying parameters? if yes, then yeah there are a lot of strange artifact (not sure if mirroring is one of them). The VLP grabber is only for Velodyne VLP-16.

With my own VLP 32 grabber I don't see anything bad (yet?).

I can maybe make a branch and a pull request, but it depends if PCL maintainers want to have it or not (ping @SergioRAgostinho & @UnaNancyOwen).

[stale[bot]]

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.

[poornimajd]

@aryasenna can you please share the code of your vlp32c grabber?

[aryasenna]

@poornimajd hi, If you can wait till weekend sure. ;) Seems the other 2 maintainers are not responding, what about @kunaltyagi, do you guys want pull request?

[poornimajd]

@aryasenna ,could you please share the code for VLP-32C grabber?

[aryasenna]

hi @poornimajd sorry, last week was a bit busy, the code is here: https://github.com/PointCloudLibrary/pcl/pull/4519

[poornimajd]

@aryasenna thanks for the code,will check it out!

[poornimajd]

@aryasenna I checked out the code ,and after minute changes ,without errors ,it was compiled and tested with vlp32 pcap file ,but it still gives a weird output.as shown: When viewed in veloview,it is actually different, Have you tried viewing some vlp32c file ,I am not sure if it is the problem in the code or is it the way I am writing the code to view it. Any suggestion is appreciated!

[aryasenna]

@poornimajd how did you use it in your code? Minimum working example can make things clearer.

[poornimajd]

@aryasenna ,thanks for the reply! Here is the code which I used to get the above output.

`#include <iostream>
#include <string>
#include <vector>

#include <pcl/point_cloud.h>
#include <pcl/point_types.h>
#include <pcl/io/vlp_grabber.h>
#include <pcl/console/parse.h>
#include <pcl/visualization/pcl_visualizer.h>

// Point Type
// pcl::PointXYZ, pcl::PointXYZI, pcl::PointXYZRGBA
typedef pcl::PointXYZI PointType;

int main( int argc, char *argv[] )
{
    // Command-Line Argument Parsing
    if( pcl::console::find_switch( argc, argv, "-help" ) ){
        std::cout << "usage: " << argv[0]
            << " [-ipaddress <192.168.1.70>]"
            << " [-port <2368>]"
            << " [-pcap <*.pcap>]"
            << " [-help]"
            << std::endl;
        return 0;
    }

std::string ipaddress( "192.168.1.70" );
std::string port( "2368" );
std::string pcap;

pcl::console::parse_argument( argc, argv, "-ipaddress", ipaddress );
pcl::console::parse_argument( argc, argv, "-port", port );
pcl::console::parse_argument( argc, argv, "-pcap", pcap );

std::cout << "-ipadress : " << ipaddress << std::endl;
std::cout << "-port : " << port << std::endl;
std::cout << "-pcap : " << pcap << std::endl;

// Point Cloud
pcl::PointCloud<PointType>::ConstPtr cloud;

// PCL Visualizer
boost::shared_ptr<pcl::visualization::PCLVisualizer> viewer( new pcl::visualization::PCLVisualizer( "Velodyne Viewer" ) );
viewer->addCoordinateSystem( 3.0, "coordinate" );
viewer->setBackgroundColor( 0.0, 0.0, 0.0, 0 );
viewer->initCameraParameters();
viewer->setCameraPosition( 0.0, 0.0, 30.0, 0.0, 1.0, 0.0, 0 );

// Point Cloud Color Hndler
pcl::visualization::PointCloudColorHandler<PointType>::Ptr handler;
const std::type_info& type = typeid( PointType );
if( type == typeid( pcl::PointXYZ ) ){
    std::vector<double> color = { 255.0, 255.0, 255.0 };
    boost::shared_ptr<pcl::visualization::PointCloudColorHandlerCustom<PointType>> color_handler( new pcl::visualization::PointCloudColorHandlerCustom<PointType>( color[0], color[1], color[2] ) );
    handler = color_handler;
}
else if( type == typeid( pcl::PointXYZI ) ){
    boost::shared_ptr<pcl::visualization::PointCloudColorHandlerGenericField<PointType>> color_handler( new pcl::visualization::PointCloudColorHandlerGenericField<PointType>( "intensity" ) );
    handler = color_handler;
}
else if( type == typeid( pcl::PointXYZRGBA ) ){
    boost::shared_ptr<pcl::visualization::PointCloudColorHandlerRGBField<PointType>> color_handler( new pcl::visualization::PointCloudColorHandlerRGBField<PointType>() );
    handler = color_handler;
}
else{
    throw std::runtime_error( "This PointType is unsupported." );
}

// Retrieved Point Cloud Callback Function
boost::mutex mutex;
boost::function<void( const pcl::PointCloud<PointType>::ConstPtr& )> function =
    [ &cloud, &mutex ]( const pcl::PointCloud<PointType>::ConstPtr& ptr ){
        boost::mutex::scoped_lock lock( mutex );

        /* Point Cloud Processing */

        cloud = ptr;
    };

// VLP Grabber
boost::shared_ptr<pcl::VLPGrabber> grabber;
if( !pcap.empty() ){
    std::cout << "Capture from PCAP..." << std::endl;
    grabber = boost::shared_ptr<pcl::VLPGrabber>( new pcl::VLPGrabber( pcap ) );
}
else if( !ipaddress.empty() && !port.empty() ){
    std::cout << "Capture from Sensor..." << std::endl;
    grabber = boost::shared_ptr<pcl::VLPGrabber>( new pcl::VLPGrabber( boost::asio::ip::address::from_string( ipaddress ), boost::lexical_cast<unsigned short>( port ) ) );
}

// Register Callback Function
boost::signals2::connection connection = grabber->registerCallback( function );

// Start Grabber
grabber->start();

while( !viewer->wasStopped() ){
    // Update Viewer
    viewer->spinOnce();

    boost::mutex::scoped_try_lock lock( mutex );
    if( lock.owns_lock() && cloud ){
        // Update Point Cloud
        handler->setInputCloud( cloud );
        if( !viewer->updatePointCloud( cloud, *handler, "cloud" ) ){
            viewer->addPointCloud( cloud, *handler, "cloud" );
        }
    }
}

// Stop Grabber
grabber->stop();

// Disconnect Callback Function
if( connection.connected() ){
    connection.disconnect();
}

return 0;

}`

[aryasenna]

@poornimajd well, you really have to look what I changed in my pull request and not just expect sample code would work ;)

I created a new class, called VLP32Grabber, interface is exactly the same as VLPGrabber.

Basically:

• you would #include <pcl/io/vlp32_grabber.h> instead of vlp_grabber.h
• replace all instance of pcl::VLPGrabber with pcl::VLP32Grabber in your code.

[poornimajd]

I have changed the VLP32_grabber.h to vlp_grabber.h and so that I do not need to replace anything,and I got the above shown output.

[aryasenna]

I have changed the VLP32_grabber.h to vlp_grabber.h and so that I do not need to replace anything,and I got the above shown output.

the class name is still pcl::VLP32Grabber, renaming the file is partial.

But I don't get it, doing what you did should not make the code even compile.

Did you really recompile PCL from source before trying?

[poornimajd]

the class name is still pcl::VLP32Grabber, renaming the file is partial.

I have changed the class name too and all the VLP32Grabber to VLPGrabber.

Did you really recompile PCL from source before trying?

Yes I did.