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

Add tpelib::Shape::GetSize() and tpelib::Model::GetCanonicalLink() #45

Merged
merged 2 commits into from
Apr 30, 2020

Conversation

claireyywang
Copy link
Contributor

tpelib::BoxShape::GetSize() was declared but not implemented in previous PR and caused windows CI failure in tpe_plugin PR

tpelib::Model::GetCanonicalLink() assumes the first link of each model is the canonical link, thus enabling ign-gazebo to find the link entity https://github.com/ignitionrobotics/ign-gazebo/blob/2259f027cd3a244f2c841b62c968088eace1b338/src/systems/physics/Physics.cc#L1233 and update world pose accordingly.

This PR needs to be merged before #32

@claireyywang claireyywang self-assigned this Apr 29, 2020
@@ -47,6 +47,8 @@ class IGNITION_PHYSICS_TPELIB_VISIBLE Model : public Entity
/// \return Newly created Link
public: Entity &AddLink();

public: Entity &GetCanonicalLink();
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add doxygen comment for this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -84,6 +84,12 @@ void BoxShape::SetSize(const math::Vector3d &_size)
this->dirty = true;
}

//////////////////////////////////////////////////
math::Vector3d BoxShape::GetSize()
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a simple check here in Shape_TEST.cc to test this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@claireyywang claireyywang requested a review from iche033 April 29, 2020 20:56
@claireyywang claireyywang merged commit fa418b3 into ign-physics2 Apr 30, 2020
@claireyywang claireyywang deleted the claire/tpelib_add_fns branch May 13, 2020 21:08
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.

2 participants