Removing ROS

[sdmiller]

As per our [pcl-developers] discussion, this (214 file) commit aims to completely eliminate ROS clashes. It does so by:

• Renaming std_msgs and sensor_msgs to pcl_std_msgs and pcl_sensor_msgs, and changing the directory structure to reflect this
• Prefixing all clashing types (Header, Image, PointCloud2, PointField) with PCL, and changing the header files to reflect this
• Updating the API / Documentation accordingly
• Getting rid of all references to USE_ROS, find_package(ROS ...), and add_ros_links references

I tried my best to make sure I fixed everything without any annoying casualties (e.g., openni_wrapper::Image, comments pertaining to Header files or Image Processing, etc). Let me know if you catch any.

[sdmiller]

Here's the script I used. Since there are some false positives (especially with "Image"), it requires confirmation before modifying files, and lets you edit some by hand for partial mistakes.

remove_ros.sh

#!/bin/bash
#move the files
git mv common/include/sensor_msgs common/include/pcl_sensor_msgs
git mv common/include/pcl_sensor_msgs/Image.h common/include/pcl_sensor_msgs/PCLImage.h
git mv common/include/pcl_sensor_msgs/PointField.h common/include/pcl_sensor_msgs/PCLPointField.h
git mv common/include/pcl_sensor_msgs/PointCloud2.h common/include/pcl_sensor_msgs/PCLPointCloud2.h
git mv common/include/std_msgs common/include/pcl_std_msgs
git mv common/include/pcl_std_msgs/Header.h common/include/pcl_std_msgs/PCLHeader.h

#use sed to rename the classes
PATTERNS="sensor_msgs:pcl_sensor_msgs std_msgs:pcl_std_msgs PointField:PCLPointField PointCloud2:PCLPointCloud2 Image:PCLImage Header:PCLHeader"
for DIR in `echo */ | sed "s/ /\n/g" | grep -v build`; do
  for PATTERN in $PATTERNS; do
    OLD=`echo $PATTERN | sed "s/:.*//g"`
    NEW=`echo $PATTERN | sed "s/.*://g"`
    echo "Going from $OLD to $NEW"
    FILES=`grep -Rl $OLD $DIR`
    for f in ${FILES}; do
      echo "Replacing ${OLD} to ${NEW} in ${f}"
      tmpfile=${f}.tomerge
      if [ ! -e ${tmpfile} ]; then
        cp ${f} ${tmpfile}
      fi
      sed -i "s/${OLD}/${NEW}/g" ${tmpfile}
      sed -i "s/\([a-zA-Z0-9_]\)${NEW}/\1${OLD}/g" ${tmpfile} #fix broken prefixes
      sed -i "s/${NEW}\([a-zA-Z0-9_]\)/${OLD}\1/g" ${tmpfile} #fix broken suffixes
    done
  done
done
#make the user verify
for tmpfile in `find . -name "*.tomerge"`; do
  f=${tmpfile%.tomerge}
  if [ `diff ${f} ${tmpfile} | wc -l` -gt 0 ]; then
    while [ 1 ]; do
      echo git diff --color-words --no-index ${f} ${tmpfile}
      git diff --color-words --no-index ${f} ${tmpfile}
      echo -n "Hit (y) to accept, (n) to reject, (e) to edit, (m) to mark for later: "
      read -n 1 answer
      echo
      if [ "$answer" == "y" ]; then
        mv ${tmpfile} ${f}
        echo MERGED
        break;
      elif [ "$answer" == "n" ]; then
        echo IGNORED
        break;
      elif [ "$answer" == "m" ]; then
        mv ${tmpfile} ${f}.toreview
        echo LEFT IN ${f}.toreview
        break;
      elif [ "$answer" == "e" ]; then
        vim ${tmpfile}
      else
        echo "Not a valid option: " $answer
      fi
    done
  fi
  rm -f ${tmpfile}
done

[jkammerl]

Great job, Stephen!

Do we need to add corresponding copy-constructors to the perception_pcl ROS package in order to support ROS message generation?

[sdmiller]

Ah, yes, we still need to add the functionality to convert between the two. I've never used perception_pcl, so it'd be great if a current ROS user wanted to take charge on that and make sure it's usable. This will be a straightforward copy constructor -- AFAIK the only member that changes is Header.timestamp, which can be converted via ros::Duration::from/toNSec ()

[jspricke]

Great work Stephen. Is there an issue at percepetion_pcl already? If you are working on the code, could you add one and reference this one? Also, we should provide a migration guide, something like our script (maybe without the git commands as we can't assume all projects to use git, although I really like them). I would propose to have everything set up and tested first and then merge all at once.

[jspricke]

What's the status of this? Should we merge it into master? Would be great to have it before tagging pcl-1.7rc1

[sdmiller]

I'm ready whenever everyone else is. I opened an issue in perception_pcl but haven't gotten any response.

[sdmiller]

Just wanted to update everyone: William Woodall has agreed to help us with this from the ROS side, and it looks like we should be ready soon. I'll get to work on a migration tutorial + script, and try to have it all ready for testing this weekend.

[jspricke]

Hi Stephen,

could you change this to move everything into the pcl namespace, as proposed in the other issue? Also, can you rebase the patches against current master?

[jspricke]

Also, should we rename the to/fromROS functions to make it clear that they are not about ROS?

[sdmiller]

Good call -- though to/fromROD are used fairly often, so migration may be
slightly more annoying for users. If we do switch, I'd opt for
"toPointCloud2" or "toBlob".

As for rebasing, etc, I should be able to take a look late tonight (Japan
time). I know better than to hope for a conflict free merge :). I also
updated the migration script with the latest namespaces and removed the git
dep, so that'll be attached as well.

On Tuesday, July 2, 2013, Jochen Sprickerhof wrote:

Also, should we rename the to/fromROS functions to make it clear that they
are not about ROS?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/110#issuecomment-20314296
.

[sdmiller]

Everything should be up to date. I rebased, but it looks like your latest merges are still showing up in the commit. Should I open a new pull request?

[wjwwood]

@wjwwood

If this travis run passes are we good to merge? Or are there outstanding things still?

[sdmiller]

I think we should be. Here's a script we can give people who want to migrate their code over

remove_ros_3rdparty.sh

#!/bin/bash
if [ -z "$1" ]; then
  echo "Usage: $0 path/to/project/directory"
  exit 1
fi
PROJECT_ROOT=$1
DIFF_CMD="diff" #input your favorite diff viewer. Examples:
#DIFF_CMD="colordiff" 
#DIFF_CMD="git diff --color-words --no-index"
EDIT_CMD="vim" #input your favorite editor

# Use sed to find and replace classes and namespaces
PATTERNS="sensor_msgs:pcl std_msgs:pcl PointField:PCLPointField PointCloud2:PCLPointCloud2 Image:PCLImage Header:PCLHeader"

CFILES=`find ${PROJECT_ROOT} -regextype posix-egrep -regex '.*\.h$|.*\.hpp$|.*\.c$|.*\.cpp$'`

for PATTERN in $PATTERNS; do
  OLD=`echo ${PATTERN} | sed "s/:.*//g"`
  NEW=`echo ${PATTERN} | sed "s/.*://g"`_CHANGEDWITHSED
  echo "Going from ${OLD} to ${NEW}"
  FILES=`grep -l ${OLD} ${CFILES} | grep -v build`
  for f in ${FILES}; do
    echo "Replacing ${OLD} to ${NEW} in ${f}"
    tmpfile=${f}.tomerge
    if [ ! -e ${tmpfile} ]; then
      cp ${f} ${tmpfile}
    fi
    sed -i "s/${OLD}/${NEW}/g" ${tmpfile}
    sed -i "s/\([a-zA-Z0-9_]\)${NEW}/\1${OLD}/g" ${tmpfile} #fix broken prefixes
    sed -i "s/${NEW}\([a-zA-Z0-9_]\)/${OLD}\1/g" ${tmpfile} #fix broken suffixes
    sed -i "s/_CHANGEDWITHSED//g" ${tmpfile}
  done
done
# View all proposed changes
for tmpfile in `find ${PROJECT_ROOT} -name "*.tomerge"`; do
  f=${tmpfile%.tomerge}
  if [ `diff ${f} ${tmpfile} | wc -l` -gt 0 ]; then
    while [ 1 ]; do
      echo ${DIFF_CMD} ${f} ${tmpfile}
      ${DIFF_CMD} ${f} ${tmpfile}
      echo -n "Hit (y) to accept, (n) to reject, (e) to edit, (m) to mark for later: "
      read -n 1 answer
      echo
      if [ "$answer" == "y" ]; then
        mv ${tmpfile} ${f}
        echo MERGED
        break;
      elif [ "$answer" == "n" ]; then
        echo IGNORED
        break;
      elif [ "$answer" == "m" ]; then
        mv ${tmpfile} ${f}.toreview
        echo Proposed change saved to ${f}.toreview
        break;
      elif [ "$answer" == "e" ]; then
        ${EDIT_CMD} ${tmpfile}
      else
        echo "Not a valid option: " $answer
      fi
    done
  fi
  rm -f ${tmpfile}
done

[jspricke]

@sdmiller as far as I can see you didn't rebase but added a merge commit (44a5d3e). Because of the time constraint, I will go ahead and make a new commit based on your pull request, hope that's ok.

Regarding to/fromROS: I would propose to mark them as deprecated and add new methods to/fromPointCloud2. If no one objects, I will add them as well.

[jspricke]

closing in favor of #164.