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

Cleanup for fiducial_slam #175

Merged
merged 6 commits into from
Jun 2, 2019
Merged

Cleanup for fiducial_slam #175

merged 6 commits into from
Jun 2, 2019

Conversation

rohbotics
Copy link
Member

@rohbotics rohbotics commented Jun 1, 2019

This fixes #173

I also found a bug in AutoInit which caused it to not actually use the closest fiducial, and instead use the first one every time.

I would recommend reviewing this commit by commit, the diffs are going to be quite large.

These are generally considered dangerous, due to the potential
for namespace collisions and unclarity when using types.
Ranged for provides an easier way to interate over every
element in a containter.

NOTE: This commit changes what I consider to be buggy behavior
in the autoInit and findClosestObs code, which was not properly
iterating over every elemnent, using obs[0] every time.
@rohbotics rohbotics requested a review from jim-v June 1, 2019 16:37
@rohbotics rohbotics self-assigned this Jun 1, 2019
@rohbotics rohbotics marked this pull request as ready for review June 1, 2019 17:20
Copy link
Contributor

@jim-v jim-v left a comment

Choose a reason for hiding this comment

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

It all looks good to me. Thanks for doing that.

@rohbotics rohbotics merged commit b043991 into kinetic-devel Jun 2, 2019
@rohbotics rohbotics deleted the fix-#173 branch June 2, 2019 16:26
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.

Code Cleanup Notes
2 participants