spencer-project / spencer_people_tracking

Multi-modal ROS-based people detection and tracking framework for mobile robots developed within the context of the EU FP7 project SPENCER.
http://www.spencer.eu/
660 stars 327 forks source link

Fixes to upper-body detector and reading configs from rosparams + topic remappings via rosparams #59

Closed MFernandezCarmona closed 4 years ago

MFernandezCarmona commented 5 years ago

I have added changes to load Config files as rosparams. Also several changes that solve a nasty segmentation. Finally a couple of minor changes to read topic name from params instead of remap in topics.

tlind commented 4 years ago

This is an extensive pull request that cannot be merged easily. In particular, it re-introduces some GPL code into the upper body detector which conflicts with its existing license.

Leaving it open though since there are fixes to a segmentation fault (possibly related to #10). I believe this affects mainly two files from commit e5ca86b28c99726d4893d:

detection/rgbd_detectors/rwth_upper_body_detector/src/detector.cpp
detection/rgbd_detectors/rwth_upper_body_detector/src/KConnectedComponentLabeler.cpp

There is also an additional config file (for Kinect v2?) and a new upper-body template, which would have to be regression-tested against our existing SPENCER datasets to see if they don't make results worse.

Some of these changes should be manually merged at some point.

MFernandezCarmona commented 4 years ago

I see. If I find some time I'll check what can be done to help. Thanks!

tlind commented 4 years ago

I merged the changes to the upper-body detector and did some manual edits where necessary. I did not include the changes that introduce additional params for topic remapping in several ROS packages; if you want to merge upstream back into your fork, you should subsequently revert commit b922fd8 to retain those changes in your fork.

Note that there are now also separate branches for ROS Melodic and Noetic.

Thanks again for the contribution Manuel!