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

Apriltags #151

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

Apriltags #151

wants to merge 22 commits into from

Conversation

rhololkeolke
Copy link
Contributor

Adds a new optional apriltag robot detector plugin. See the README updates for more information about how to compile, run, and configure this detector.

@rhololkeolke
Copy link
Contributor Author

Can we merge this in before master diverges too much?

Other than it being a bit slow for high res images, everything seems to work for those that have tried it. And the speed-ups for high res images can be merged in a different request.

Copy link
Member

@g3force g3force left a comment

Choose a reason for hiding this comment

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

Found some minor issues.

I'd like to have an automatic build with circleci to verify that everything builds fine with april tags from scratch. I can have a look at this.

There are some unrelated reformattings that bloads up the diff a bit, but it looks ok so far.

@@ -126,14 +126,14 @@ ProcessResult PluginDetectRobots::process(FrameData * data, RenderOptions * opti
color_id=color_id_blue;
team=global_team_selector_blue->getSelectedTeam();
num_robots=global_team_selector_blue->getNumberRobots();
detection_frame->clear_robots_blue();
// detection_frame->clear_robots_blue();
Copy link
Member

Choose a reason for hiding this comment

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

Why is this commented out? Can this be removed safely?

robot_detection = detection_frame->add_robots_yellow();
}
robot_detection->set_confidence(
det->decision_margin); // TODO(dschwab): What value should I put here?
Copy link
Member

Choose a reason for hiding this comment

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

is this still a TODO? Can this be resolved?

@@ -181,7 +181,7 @@ TeamDetector::~TeamDetector()
void TeamDetector::update(::google::protobuf::RepeatedPtrField< ::SSL_DetectionRobot >* robots, int team_color_id, int max_robots, const Image<raw8> * image, CMVision::ColorRegionList * colorlist, CMVision::RegionTree & reg_tree) {
color_id_team=team_color_id;
_max_robots=max_robots;
robots->Clear();
// robots->Clear();
Copy link
Member

Choose a reason for hiding this comment

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

same here: Can this be removed?

@g3force
Copy link
Member

g3force commented Jan 18, 2020

Automatic build for apriltag added to circleci config. It checks out the master branch of the apriltag library, so the build is not reproducible, but we will find issues with updates in the apriltag lib earlier.

@rdesc
Copy link

rdesc commented Aug 5, 2020

The updated README.md should explicity say the april tags repo needs to be cloned and installed. Also I had to update my LD_LIBRARY_PATH env variable to include the path where the libapriltag.so files were installed, might be worth mentioning in the docs as well.

Descriptions of the different AprilTag params I feel should also be added.

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.

3 participants