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.

Sign up for GitHub

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

Jump to bottom

handle continuous joints in getLowerAndUpperLimits #3153

Merged
merged 1 commit into from
Dec 11, 2024
Merged

handle continuous joints in getLowerAndUpperLimits #3153

merged 1 commit into from
Dec 11, 2024

Conversation

marioprats
Copy link
Contributor

Description

The RevoluteJointModel returns -pi/pi as position joint limits for continuous joints, which are not position bounded.
This PR fixes getLowerAndUpperLimits to check the position_bounded_ flag and return joint limits accordingly.

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

@marioprats marioprats requested a review from sjahr December 10, 2024 13:21
@mergify Mergify
Copy link

mergify bot commented Dec 10, 2024

This pull request is in conflict. Could you fix it @marioprats?

@marioprats marioprats force-pushed the continuous-joints-limits branch from fe617ac to 1c1b818 Compare December 10, 2024 13:27
@codecov-commenter
Copy link

codecov-commenter commented Dec 10, 2024 •
edited
Loading

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 44.09%. Comparing base (f82cdcd) to head (1c1b818).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
moveit_core/robot_model/src/joint_model_group.cpp 0.00% 4 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3153      +/-   ##
==========================================
- Coverage   45.77%   44.09%   -1.67%     
==========================================
  Files         482      698     +216     
  Lines       40432    61537   +21105     
  Branches        0     7458    +7458     
==========================================
+ Hits        18502    27126    +8624     
- Misses      21930    34245   +12315     
- Partials        0      166     +166

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sea-bass sea-bass added backport-humble Mergify label that triggers a PR backport to Humble backport-jazzy Mergify label that triggers a PR backport to Jazzy labels Dec 10, 2024
sea-bass
sea-bass approved these changes Dec 10, 2024
View reviewed changes
Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

LGTM!

Added backport labels to Humble and Jazzy.

@sjahr sjahr added this pull request to the merge queue Dec 10, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Dec 11, 2024
@sea-bass sea-bass added this pull request to the merge queue Dec 11, 2024
@sea-bass
Copy link
Contributor

I just found the issue for why this PR and my last one kept getting kicked out of the merge queue.

The default timeout in the branch settings was set to 60 minutes, which is sometimes not enough for the ccov/ikfast jobs that now actually work.

Increased it to 90 minutes.

Merged via the queue into moveit:main with commit ecd5d30 Dec 11, 2024
9 checks passed
mergify bot pushed a commit that referenced this pull request Dec 11, 2024
@marioprats @mergify
handle continuous joints in getLowerAndUpperLimits (#3153)
f50a319 
(cherry picked from commit ecd5d30)

# Conflicts:
#	moveit_core/robot_model/include/moveit/robot_model/joint_model_group.hpp
#	moveit_core/robot_model/src/joint_model_group.cpp
@mergify mergify bot mentioned this pull request Dec 11, 2024
Closed
6 tasks
mergify bot pushed a commit that referenced this pull request Dec 11, 2024
@marioprats @mergify
handle continuous joints in getLowerAndUpperLimits (#3153)
bd1a13f 
(cherry picked from commit ecd5d30)
@mergify mergify bot mentioned this pull request Dec 11, 2024
Merged
6 tasks
sea-bass pushed a commit that referenced this pull request Dec 11, 2024
@mergify @marioprats
handle continuous joints in getLowerAndUpperLimits (#3153) (#3155)
e94c891 
(cherry picked from commit ecd5d30)

Co-authored-by: Mario Prats <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjahr sjahr sjahr approved these changes

@sea-bass sea-bass sea-bass approved these changes

Assignees
No one assigned
Labels
backport-humble Mergify label that triggers a PR backport to Humble backport-jazzy Mergify label that triggers a PR backport to Jazzy
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@marioprats @codecov-commenter @sea-bass @sjahr