UbiquityRobotics / fiducials

Simultaneous localization and mapping using fiducial markers.
BSD 3-Clause "New" or "Revised" License
265 stars 135 forks source link

Remove unused parts from fiducial_slam #244

Closed agutenkunst closed 3 years ago

agutenkunst commented 3 years ago

These seem to be obsolete.

ros::Subscriber verticesSub;
ros::Subscriber cameraInfoSub;

are not used.


if (doPoseEstimation) {
        double fiducialLen, errorThreshold;
        nh.param<double>("fiducial_len", fiducialLen, 0.14);
        nh.param<double>("pose_error_theshold", errorThreshold, 1.0);

        ftPub = ros::Publisher(
            nh.advertise<fiducial_msgs::FiducialTransformArray>("/fiducial_transforms", 1));
    }

looks like an artifact. Maybe aruco_detect was used with copy+paste at the beginning? Also do_pose_estimation is not documented at http://wiki.ros.org/fiducial_slam

JanezCim commented 3 years ago

Hey, thanks for your contribution :)

I agree with these changes,

ros::Subscriber verticesSub;
ros::Subscriber cameraInfoSub;

arent used anywhere in the fiducial_slam.cpp

and doPoseEstimation part does not make sense in the fiducial_slam.cpp, really seems like its an artifact from an earlier aruco_detect. @jim-v do you agree?

Ive tested this in Gazebo and it does not seem to break anything

jim-v commented 3 years ago

The code is left over from an optional mode that called solvePnP on all the fidcual vertices in an image inside fiducial_slam to get a single pose estimate. It was never fully developed and there should be no problem with removing it. It is arguably more correct than the way pose estimates are combined, but no one had time to work on it to finish it off.