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 leg_detector to OpenCV3 #53

Merged
merged 11 commits into from
Apr 30, 2018
Merged

Conversation

kdhansen
Copy link
Contributor

I have updated the leg_detector to the new structure of OpenCV3. I actually had another PR #48 open but it failed in Travis. Isaac Saito fixed this in his PR #49 which I rebased this code onto, and now it passes on Travis!

Isaac also found that we need to hardcode the location of haarcascade_frontal_face_alt from OpenCV3 (see #52). Between the time he made #49 and now, OpenCV3 was updated to 3.2.0, so I went and updated the filepath.

This is all to say that, merging this branch also includes all the hard work of Isaac in PR #49.

130s and others added 11 commits February 17, 2017 16:17
[face_detector][test common] Fix warning in .launch format.
Fix namespace in testcases.
Increase retry. .bag-based perceptional tests are hard to deterministic, so it makes sense to do this.
Set param use_sim_time.
[face_detector][test] hztest can be used for measureing no publishment. hzerror is not needed in that case.
[face_detector][test] Specify test name. Pass all args.
[face_detector][test] pass_all_args might require all args kept out of include tag.
[face_detector][test] Specify test-name.
[face_detector][test] Disable all tests other than rgbd. I couldn't figure out how to use launches other than face_detector.rgbdlaunch, after hours of trial on local machine. I leave that up to someone else.
[face_detector][test] Detection using rgbd works with this config.
[face_detector][test] Shorter test duration (bag files are less than 7 seconds).
[face_detector][test] Shorter test_duration. We only need a second or so as long as we can confirm the designated topic is being published.
[face_detector][test] rosbag play with clock and loop option.
Since the publish rate of the detected face varies every second because one of the persons in the bag file is moving, [publishtest](http://wiki.ros.org/rostest/Nodes#publishtest) should suit the best in this case.

[face_detector][test common] Enable substitution in rosparam tag.
The cv::ml package has been rewritten. I just changed the code to
accomodate the new api, maybe this does not conform to the way the new
cv::ml should be used...
In opencv3 the predict_prob method was removed from cv::ml::RTrees.
The dependency on the people_msgs package should be enough. This removes
warnings for the leg_detector and people_tracking_filter when building
with catkin_make_isolated and catkin build.
Isaac Saito has been working hard to make the Travis CI work with
Kinetic. Removing his email from ravis.yml, to save his inbox from
errors due to other issues.
Version changed to 3.2.0, unfortunately we need to hardcode the
location for now.
@DLu DLu added the legs label Apr 9, 2017
@mjyc
Copy link

mjyc commented Nov 24, 2017

Hi! Is there a plan for reviewing this PR? If not, is there any way I can help? e.g. code review, comparing performance on real data between this & 14.04, etc.

@dlaz dlaz requested a review from DLu December 20, 2017 16:53
@awesomebytes
Copy link
Member

Bump, any plan on moving this forward?
I can try on a PR2 both the indigo and kinetic versions.

@awesomebytes
Copy link
Member

awesomebytes commented Apr 20, 2018

So, in case it helps on deciding, I gave it a try to run it on the indigo-devel version on a Ubuntu 14.04 and on the kinetic version this PR has on a VM with Ubuntu 16.04.

Both are running with the same laser topic, a real Hokuyo UTM-30LX-01 (from a PR2).

Find a video of their behaviour in Rviz here: https://youtu.be/G-OxB-L14z0
(Left side indigo, right side kinetic).

Find also attached to this comment a rosbag with all the topics:

path:         leg_detector_indigo_vs_kinetic_2018-04-20-19-05-57.bag
version:      2.0
duration:     34.4s
start:        Apr 20 2018 19:05:58.10 (1524215158.10)
end:          Apr 20 2018 19:06:32.47 (1524215192.47)
size:         6.6 MB
messages:     77457
compression:  bz2 [40/40 chunks; 19.23%]
uncompressed: 29.7 MB @ 884.3 KB/s
compressed:    5.7 MB @ 170.1 KB/s (19.23%)
types:        people_msgs/PositionMeasurementArray [59c860d40aa739ec920eb3ad24ae019e]
              sensor_msgs/LaserScan                [90c7ef2dc6895d81024acba2ac42f369]
              visualization_msgs/Marker            [18326976df9d29249efc939e00342cde]
topics:       /base_scan                               738 msgs    : sensor_msgs/LaserScan               
              /leg_tracker_measurements                737 msgs    : people_msgs/PositionMeasurementArray
              /leg_tracker_measurements_kinetic        737 msgs    : people_msgs/PositionMeasurementArray
              /people_tracker_measurements             737 msgs    : people_msgs/PositionMeasurementArray
              /people_tracker_measurements_kinetic     737 msgs    : people_msgs/PositionMeasurementArray
              /visualization_marker                  35598 msgs    : visualization_msgs/Marker           
              /visualization_marker_kinetic          38173 msgs    : visualization_msgs/Marker

leg_detector_indigo_vs_kinetic_2018-04-20-19-05-57.bag.tar.gz

I see even an improvement in kinetic. But generally, they do quite the same job.

This was running (after checking out the repo and doing catkin_make install and sourcing that install):

roslaunch leg_detector leg_detector.launch

With the only modification of changing the fixed_frame to laser, and on the kinetic version, remapping the topics to end in kinetic.

I'd like to have this package back in the ROS repos as it's very good to enable simple demos (like this video) to get people started with ROS and stuff to do with people perception.

Thank you all for your very good work! @DLu @kdhansen

Copy link
Member

@dlaz dlaz left a comment

Choose a reason for hiding this comment

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

Overall LGTM, just a few small comments.

@@ -82,15 +82,15 @@ if(CATKIN_ENABLE_TESTING)
DESTINATION ${CATKIN_DEVEL_PREFIX}/${CATKIN_PACKAGE_SHARE_DESTINATION}/test
MD5 59126117e049e69d577b7ee27251a6f8)

add_rostest(test/wide-stereo_true_rtest.xml)
add_rostest(test/wide-stereo_false_rtest.xml)
# add_rostest(test/wide-stereo_true_rtest.xml)
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer just deleting these instead of commenting them out, here and below

@@ -244,7 +244,8 @@ class LegDetector

int mask_count_;

cv::ml::RTrees forest;
// cv::ml::RTrees forest;
Copy link
Member

Choose a reason for hiding this comment

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

nit: remove

@dlaz
Copy link
Member

dlaz commented Apr 23, 2018

I don't really have things setup to try this out now, so unless there are any objections from @DLu I'm going to go ahead and merge. I'll check back in a few days.

@awesomebytes
Copy link
Member

@dlaz a little friendly bump in case anyone wants to take a final look or you want to merge it.

@dlaz dlaz merged commit 20257a4 into wg-perception:kinetic Apr 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants