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

Replaced single value joint_limit_margin with list of joint_limit_margin #2576

Merged
merged 5 commits into from
Dec 12, 2023

Conversation

Nils-ChristianIseke
Copy link
Contributor

Description

Related Issue: #2570
Enabling individual joint_limit_margins for each joint. This is useful, for example, when different types of joints are used. One value for all margins (as before) is not necessarily suitable for all joints. E.g. when revolute and prismatic joints exist.

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference -> Have not found a tutorial in which margin is shown directly
  • Document API changes relevant to the user in the MIGRATION.md notes -> Not sure if i need to add the changes?

Copy link

codecov bot commented Dec 5, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (b2a3b16) 50.85% compared to head (bf8f11d) 50.86%.

Files Patch % Lines
moveit_ros/moveit_servo/src/servo.cpp 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2576      +/-   ##
==========================================
+ Coverage   50.85%   50.86%   +0.02%     
==========================================
  Files         388      388              
  Lines       32360    32363       +3     
==========================================
+ Hits        16453    16458       +5     
+ Misses      15907    15905       -2     

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

@Nils-ChristianIseke Nils-ChristianIseke changed the title Replaced joint_limits Replaced single value joint_limit_margin with list of joint_limit_margin Dec 5, 2023
Copy link
Member

@AndyZe AndyZe left a comment

Choose a reason for hiding this comment

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

No big deal, but just a hint for the future: rename the branch before you open a PR. I would've called it joint_limit_list or something like that. This will help if you have lots of PR's open at once.

moveit_ros/moveit_servo/config/panda_simulated_config.yaml Outdated Show resolved Hide resolved
moveit_ros/moveit_servo/config/test_config_panda.yaml Outdated Show resolved Hide resolved
moveit_ros/moveit_servo/src/utils/common.cpp Show resolved Hide resolved
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.

Looks good -- just some formatting suggestions

moveit_ros/moveit_servo/config/panda_simulated_config.yaml Outdated Show resolved Hide resolved
moveit_ros/moveit_servo/config/test_config_panda.yaml Outdated Show resolved Hide resolved
… Enabling setting individual margins for each joint.
…o.cpp to give a clear error message if the size of joint_limit_margis does not match the number of joints of the move_group
Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>
@AndyZe AndyZe merged commit 754d919 into moveit:main Dec 12, 2023
12 checks passed
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