ROS Melodic and the Broken API on Master

[JWhitleyWork]

Several months ago, I merged the PR #214 into master. This was an excellent, very efficient PR but what I didn't realize at the time was that it broke the API from the Melodic release of the velodyne packages. On the plus (and minus) side, I have yet to release a new Melodic version of this repository so hell has been kept from breaking loose. Since Melodic is an LTS ROS release and there are likely a lot of people who wouldn't be very happy with a package update breaking their downstream things, the following is my new plan of action:

1. Go back to the commit before b4ad0ae introduced the breaking changes and create a melodic branch from there.
2. Do my best to cherry-pick changes that were made after that point onto the melodic branch without breaking the API.
3. Have someone test the melodic branch.
4. Release the melodic branch into the build farm as a Melodic release.
5. Consider master to be targeted at Noetic.

Not only does this remove the need for a hack like #335, it keeps from breaking the Melodic API. Noetic will then receive the new, no-PCL API (with the PointCloud type renamed to include the new fields).

I would appreciate any feedback from @spuetz, @beetleskin, and @YoshuaNava (and anyone else that wishes to chime in).

If I don't hear any dissenting opinions, I'll get started on this work sometime in the next 2 weeks and hopefully we'll have a shiny, new Melodic release soon thereafter.

[spuetz]

I think adding a bit of code should fix that issue. But also marking the code maintaining backwards compatibility as deprecated.

[JWhitleyWork]

@spuetz Could you explain a little more? I don't quite understand your answer.

[spuetz]

@JWhitleyWork Yes of course, sry for the short answer above. I think we could add the necessary files like the point_types.h and the corresponding container, which is maintaining the old point cloud fields. We add these to the velodyne_pointcloud package and we add the pcl dependency again. Then we mark these files and usage as deprecated with a link to the velodyne_pcl package. What do you think?

[JWhitleyWork]

@spuetz Assuming that you mean we do the second step for Noetic only, then yes, that would also work. Could you possibly create the PR for this proposal? I'm sorry but I'm not as familiar with the changes you made to remove PCL as I should be.

[spuetz]

@JWhitleyWork I'll create a PR in the next week.

[JWhitleyWork]

@JWhitleyWork I'll create a PR in the next week.

Thank you! Sorry I haven't been able to give time to this lately.

[YoshuaNava]

Please keep me updated about this. I can help testing with a VLP-16.

Also, @JWhitleyWork can you write a short list of tests that should be done before a Release? I understand that you don't have a sensor at hand now, but writing a solid procedure will guarantee that your really cool work on this driver will be used and extended by others with great results.

[JWhitleyWork]

@YoshuaNava Here's a list off the top of my head:

1. Connect sensor and test that launching the driver with an improper configuration either causes an error or some indication that no data are received.
2. Connect sensor and test that launching the driver with a proper configuration does not cause any errors.
3. Verify that the velodyne_points topic can be visualized in RViz.
4. Verify that changes to the target_frame parameter changes the frame in the header of the velodyne_points topic (you can use rostopic echo velodyne_points | grep frame to filter out the raw data).
5. Verify that changes to the frame_id parameter changes the frame in the header of the velodyne_packets topic.

Don't bother messing with the fixed_frame right now as it's messed up right now (see #342 for explanation). @spuetz Also check out my reply in #342 as there is definitely something wrong there.

[JWhitleyWork]

@spuetz Any progress? I haven't seen a PR yet.

[spuetz]

@JWhitleyWork I'm doing it today. We should have an official branch melodic-devel and the master without pcl for noetic. Or how do you plan the release?