Skip to content
This repository has been archived by the owner on Feb 3, 2025. It is now read-only.

Joint::GetAngle returns Angle, which is confusing for non-angular joints #553

Closed
osrf-migration opened this issue Mar 4, 2013 · 14 comments
Labels
all bug Something isn't working major

Comments

@osrf-migration
Copy link

Original report (archived issue) by John Hsu (Bitbucket: hsu, GitHub: hsu).


Typical usage right now expects user to do:

   double sliderPosition = myJoint->GetAngle().Radian()

even for prismatic joints, or joints with non-angular positions. This is potentially confusing.

@osrf-migration
Copy link
Author

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


  • changed state from "new" to "on hold"

Until after June, 2013.

@osrf-migration
Copy link
Author

Original comment by Ian Chen (Bitbucket: Ian Chen, GitHub: iche033).


  • changed state from "on hold" to "open"

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


related question

Maybe we could deprecate math::Angle Joint::GetAngle in favor of double Joint::GetPosition and create GetAngle functions in the relevant joint specific headers (like RevoluteJoint.hh, UniversalJoint.hh, etc)?

@osrf-migration
Copy link
Author

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


Sounds good.

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


It'll take a bit more work than I thought because we also use math::Angle as the argument for joint limits.

@osrf-migration
Copy link
Author

Original comment by Louise Poubel (Bitbucket: chapulina, GitHub: chapulina).


Related issue #1086

@osrf-migration
Copy link
Author

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


  • set version to "all"

@osrf-migration
Copy link
Author

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


  • changed state from "open" to "new"

@osrf-migration
Copy link
Author

Original comment by Louise Poubel (Bitbucket: chapulina, GitHub: chapulina).


@nkoenig , something to consider while migrating to ignition math

@osrf-migration
Copy link
Author

Original comment by Louise Poubel (Bitbucket: chapulina, GitHub: chapulina).


  • set assignee_account_id to "557058:6ff86fcb-b7ab-44a5-b8a6-f6d9cae8b6e8"
  • set assignee to "chapulina (Bitbucket: chapulina, GitHub: chapulina)"

@osrf-migration
Copy link
Author

Original comment by Louise Poubel (Bitbucket: chapulina, GitHub: chapulina).


I'm planning on tackling this while migrating to ignition math. The plan:

  1. Deprecate everything which mentions "angle" from the base joint class:

    • bool SetHighStop(unsigned int _index, const math::Angle &_angle)

    • void SetUpperLimit(unsigned int _index, math::Angle _limit)

    • bool SetLowStop(unsigned int _index, const math::Angle &_angle)

    • void SetLowerLimit(unsigned int _index, math::Angle _limit)

    • math::Angle GetHighStop(unsigned int _index)

    • math::Angle GetUpperLimit(unsigned int _index) const

    • math::Angle GetLowStop(unsigned int _index)

    • math::Angle GetLowerLimit(unsigned int _index) const

    • math::Angle GetAngle(unsigned int _index) const

    • virtual unsigned int GetAngleCount() const

  2. Substitute with equivalents that return/take double. Also make the return axis 0 by default. Note that the "*Stop" will disappear completely.

    • void SetUpperLimit(const unsigned int _index, const double _limit)

    • void SetLowerLimit(const unsigned int _index, const double _limit)

    • double UpperLimit(const unsigned int _index = 0) const

    • double LowerLimit(const unsigned int _index = 0) const

    • double Position(const unsigned int _index = 0) const

    • unsigned int DOF() const

  3. Add functions that return angles for each rotational joint.

@osrf-migration
Copy link
Author

Original comment by Louise Poubel (Bitbucket: chapulina, GitHub: chapulina).


Implemented numbers 1 and 2 in pull request #2568.

I believe we can add number 3 later without breaking ABI/API.

@osrf-migration
Copy link
Author

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


  • changed state from "new" to "resolved"

Resolved in pull request #2568

@osrf-migration
Copy link
Author

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


  • changed state from "resolved" to "closed"

@osrf-migration osrf-migration added major bug Something isn't working all labels Apr 19, 2020
qaz6906 pushed a commit to qaz6906/gazebo-classic_Hexa_vtol that referenced this issue Feb 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
all bug Something isn't working major
Projects
None yet
Development

No branches or pull requests

1 participant