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

update to use non deprecated pluginlib macro #27

Merged
merged 1 commit into from
Feb 7, 2019

Conversation

mikaelarguedas
Copy link
Contributor

These macros, deprecated for now 8 years, will be removed in the next ROS release (ROS Melodic)

This change will allow the code to keep compiling on future ROS versions

@mkhansenbot
Copy link

This built for me on 18.04 / Melodic. Any chance it can be merged soon?

jarvisschultz added a commit to NU-MSR/turtlebot that referenced this pull request Oct 5, 2018
jarvisschultz added a commit to NU-MSR/turtlebot that referenced this pull request Oct 5, 2018
@4ndr3w
Copy link

4ndr3w commented Nov 11, 2018

+1 for merging this, fixed build for me on Melodic w/ Ubuntu 18.04.

@mkhansenbot
Copy link

@chadrockey @wjwwood @mikeferguson - who's the maintainer for this now? Can someone merge this?

@wjwwood
Copy link
Member

wjwwood commented Nov 13, 2018

I believe that @chadrockey is still the official maintainer. I think it's probably safe to merge and release, but given that this is targeted at the indigo-devel branch there may be some maintainer related work to decide if that's ok or if a new branch called melodic-devel should be created.

I don't have time to do that, but maybe someone else does. If not, maybe @chadrockey would be interested in having help from one of you guys doing this last part needed to get it released.

@4ndr3w
Copy link

4ndr3w commented Nov 14, 2018

Tested and this branch builds inside the ros:indigo Docker container, all tests pass.

@130s
Copy link
Member

130s commented Nov 26, 2018

Confirmed this builds on Melodic Docker container. See below the output.

Also I suggested CI coverage in #34

$ docker images|grep -i melodic
ros                                                                melodic                                         57d0a2450203        6 days ago          1.27GB
$ docker run -it ros:melodic

# source /opt/ros/melodic/setup.bash 
# REPOS_CLONE=https://github.com/ros-perception/depthimage_to_laserscan
# mkdir -p ~/cws_dil/src && cd ~/cws_dil/src
# for repo in $REPOS_CLONE; do git clone $repo; done && cd ~/cws_dil && apt-get update && rosdep update && rosdep install -r -y --from-paths src --ignore-src && apt-get install python-catkin-tools && catkin config --install && catkin build -DCMAKE_BUILD_TYPE=Release

Default branch fails to build as expected.

:
Starting  >>> depthimage_to_laserscan
___________
Errors     << depthimage_to_laserscan:make /root/cws_dil/logs/depthimage_to_laserscan/build.make.000.log 
/root/cws_dil/src/depthimage_to_laserscan/src/DepthImageToLaserScanNodelet.cpp:60:24: error: expected constructor, destructor, or type conversion before ‘(’ token
 PLUGINLIB_DECLARE_CLASS(depthimage_to_laserscan, DepthImageToLaserScanNodelet, depthimage_to_laserscan::DepthImageToLaserScanNodelet, nodelet::Nodelet);
                        ^
make[2]: *** [CMakeFiles/DepthImageToLaserScanNodelet.dir/src/DepthImageToLaserScanNodelet.cpp.o] Error 1
make[1]: *** [CMakeFiles/DepthImageToLaserScanNodelet.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [all] Error 2
cd /root/cws_dil/build/depthimage_to_laserscan; catkin build --get-env depthimage_to_laserscan | catkin env -si  /usr/bin/make --jobserver-fds=6,7 -j; cd -
..
Failed     << depthimage_to_laserscan:make           [ Exited with code 2 ]                                                                                       
Failed    <<< depthimage_to_laserscan                [ 11.6 seconds ]      
[build] Summary: 1 of 2 packages succeeded.                                
[build]   Ignored:   None.                                                 
[build]   Warnings:  None.                                                 
[build]   Abandoned: None.                                                 
[build]   Failed:    1 packages failed.
[build] Runtime: 13.9 seconds total.

# git log -1
commit f135fbd5f60dc7b33aa6d01f9d60c0ec9cb9d787 
Author: Chad Rockey <chadrockey@gmail.com>
Date:   Mon Jun 16 16:00:21 2014 -0700

    1.0.7

Using this PR, I confirm build passes.

# cd ~/cws_dil/src/depthimage_to_laserscan/ &&  git checkout mikaelarguedas-patch-1 && ~/cws_dil && rm -fr build devel logs install && catkin b
:
Starting  >>> depthimage_to_laserscan
Finished  <<< depthimage_to_laserscan                [ 12.9 seconds ]
[build] Summary: All 2 packages succeeded!                           
[build]   Ignored:   None.                                           
[build]   Warnings:  None.                                           
[build]   Abandoned: None.                                           
[build]   Failed:    None.                                           
[build] Runtime: 15.6 seconds total.                                 
[build] Note: Workspace packages have changed, please re-source setup files to use them.

@mikaelarguedas
Copy link
Contributor Author

@chadrockey @wjwwood It looks like the change builds successfully on both Indigo and Melodic.
Is there anything else the community can do to help getting this merged and released in Melodic?

@chadrockey chadrockey merged commit 958c9c5 into indigo-devel Feb 7, 2019
@mikaelarguedas mikaelarguedas deleted the mikaelarguedas-patch-1 branch February 7, 2019 09:15
kingjin94 added a commit to kingjin94/realtime_urdf_filter that referenced this pull request Jul 29, 2019
YBachmann pushed a commit to YBachmann/depthimage_to_laserscan that referenced this pull request Jul 29, 2024
…as-patch-1

update to use non deprecated pluginlib macro
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.

6 participants