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 Semantics checks to Transform class #62

Merged
merged 5 commits into from
Jul 12, 2015

Conversation

nunoguedelha
Copy link
Contributor

This change is a further step in the implementation of feature #29, with a complete support of semantics in the Transform class.

Refactored Transform class:

  • Transform is now only composed of Position and Rotation class objects, without duplicated semantics elements (every Transform semantics computation relies on Position and Rotation semantics operation methods)
  • Transform support of semantics covers transformation of Position, Rotation and Transform. Semantics on Twist and Wrench will come in the next days.

Note: These changes are merged with implementation of Spatial Momentum, Spatial Acceleration and Spatial Inertia (#59).

- Transform is now composed of Position and Rotation class objects (pos & rot)
- Transform semantics elements are only links to pos and rot semantics
- every semantics computation relies on Position and Rotation semantics methods
(- cleanup old friend statements
 - protected copy-assignment method of TransformSemantics)
(Spatial Inertia, Spatial Acceleration , Velocity , Force and Momentum)
@traversaro
Copy link
Member

LGTM, merging.

traversaro added a commit that referenced this pull request Jul 12, 2015
…formClass

Add Semantics checks to Transform class
@traversaro traversaro merged commit 01c7333 into master Jul 12, 2015
@traversaro
Copy link
Member

Actually I just noted a possible problem: TransformSemantics class expose this two methods:

  • bool setRotationSemantics(const RotationSemantics & rotation);
  • bool setPositionSemantics(const PositionSemantics & position);

One can call them on a transform using transform.getSemantics().setRotationSemantics()... However if they do like this they can have a inconsisteny between the sematics stored in TransformSemantics and the one store in the Rotation object stored in Transform.

@nunoguedelha
Copy link
Contributor Author

Both methods from TransformSemantics ...

  • TransformSemantics::setRotationSemantics()
  • TransformSemantics::setPositionSemantics()

... as well as those from Transform...

  • Transform::setPosition()
  • Transform::setRotation()

... they all call TransformSemantics::check_position2rotationConsistency() which checks the consistency of semantics.
On top of that, TransformSemantics::PositionSemantics and TransformSemantics::PositionSemantics are references to the components semantics (pos and rot). These references are initialised once and for all by the constructors at Transform instantiation.

If I missed something, any comment is most welcome.

@traversaro
Copy link
Member

Ah yes, I forgot about the references in TransformSemantics (even if we already discussed them!)
Sorry for the noise.

@traversaro traversaro deleted the feature/29/AddSemChecksToTransformClass branch July 21, 2015 07:56
RiccardoGrieco pushed a commit to RiccardoGrieco/idyntree-hde-fork that referenced this pull request Apr 14, 2022
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