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 PoseEstimatorModule, AssetDatabase #508

Merged
merged 15 commits into from
Feb 22, 2019

Conversation

gilwoolee
Copy link
Contributor

@gilwoolee gilwoolee commented Feb 20, 2019

This PR contains the following updates:

  • Update PoseEstimatorModule to follow c++ naming convention
  • Have DELETE marker to be handled earlier (so that it doesn't look for matching assetKey but only checks for existing objectUid in world)
  • Remove objectName from AssetDatabase since this information is already given by the marker (further, it removes the need for pr_assets to have different name for objects of same shape, e.g. cans or food items). Right now all tags either not have objectName or have objectName same as asset key.

Before creating a pull request

  • Document new methods and classes
  • Format code with make format

Before merging a pull request

  • Set version target by selecting a milestone on the right side
  • Summarize this change in CHANGELOG.md
  • Add unit test(s) for this change

…TE marker to be handled earlier, remove objectName from AssetDatabase since this information is already given by the marker (further, it removes the need for pr_assets to have different name for objects of same shape, e.g. cans or food items).
Copy link
Contributor

@egordon egordon left a comment

Choose a reason for hiding this comment

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

One substantial change, a couple of comment nits.

include/aikido/perception/DetectedObject.hpp Outdated Show resolved Hide resolved
include/aikido/perception/DetectedObject.hpp Show resolved Hide resolved
include/aikido/perception/DetectedObject.hpp Outdated Show resolved Hide resolved
src/perception/DetectedObject.cpp Show resolved Hide resolved
@gilwoolee gilwoolee added this to the Aikido 0.3.0 milestone Feb 20, 2019
src/perception/PoseEstimatorModule.cpp Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Feb 20, 2019

Codecov Report

Merging #508 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #508   +/-   ##
=======================================
  Coverage   77.69%   77.69%           
=======================================
  Files         262      262           
  Lines        6792     6792           
=======================================
  Hits         5277     5277           
  Misses       1515     1515

brianhou
brianhou previously approved these changes Feb 21, 2019
Copy link
Contributor

@brianhou brianhou left a comment

Choose a reason for hiding this comment

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

Left some nit comments, but generally looks good to me.

egordon
egordon previously approved these changes Feb 21, 2019
Co-Authored-By: gilwoolee <gilwoo301@gmail.com>
@gilwoolee gilwoolee dismissed stale reviews from egordon and brianhou via bdca00a February 21, 2019 10:15
Copy link
Contributor

@brianhou brianhou left a comment

Choose a reason for hiding this comment

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

One last nit about DetectedObject-impl.hpp.

include/aikido/perception/detail/DetectedObject-impl.hpp Outdated Show resolved Hide resolved
egordon
egordon previously approved these changes Feb 21, 2019
@brianhou brianhou merged commit c830058 into master Feb 22, 2019
@brianhou brianhou deleted the enhancement/perception_detectObject branch February 22, 2019 08:42
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