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

documentation mismatch in kinematic_constraint.h #1793

Merged
merged 1 commit into from
Nov 29, 2019
Merged

documentation mismatch in kinematic_constraint.h #1793

merged 1 commit into from
Nov 29, 2019

Conversation

JeroenDM
Copy link
Contributor

Description

There is a mismatch between the documentation and the implementation.
Implementation uses XYZ Euler angles, documentation says ZXZ.

The implementation can be seen in kinematic_constraint.cpp and in default_constraint_samplers.cpp.

I don't think the checklist below is relevant for such a small change (only a single line).
Is such a small change even worth a pull request?

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

There was a mismatch between the documentation and the implementation.
Implementation uses XYZ Euler angles, documentation says ZXZ.
@welcome
Copy link

welcome bot commented Nov 29, 2019

Thanks for helping in improving MoveIt and open source robotics!

@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1793   +/-   ##
=======================================
  Coverage   47.53%   47.53%           
=======================================
  Files         290      290           
  Lines       22932    22932           
=======================================
  Hits        10901    10901           
  Misses      12031    12031
Impacted Files Coverage Δ
...oveit/kinematic_constraints/kinematic_constraint.h 89.65% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update acead73...8497c93. Read the comment docs.

Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

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

Nice catch!

@rhaschke rhaschke merged commit 9305824 into moveit:master Nov 29, 2019
@welcome
Copy link

welcome bot commented Nov 29, 2019

Congrats on getting your first MoveIt pull request merged and improving open source robotics!

v4hn pushed a commit to v4hn/moveit that referenced this pull request Mar 30, 2020
Implementation uses XYZ Euler angles (RPY), but documentation said ZXZ.
v4hn pushed a commit to v4hn/moveit that referenced this pull request Mar 31, 2020
Implementation uses XYZ Euler angles (RPY), but documentation said ZXZ.
v4hn pushed a commit to v4hn/moveit that referenced this pull request Mar 31, 2020
Implementation uses XYZ Euler angles (RPY), but documentation said ZXZ.
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.

None yet

3 participants