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

Change KDL IK plugin joint weight parameters specification #2750

Closed
wants to merge 4 commits into from

Conversation

sea-bass
Copy link
Contributor

@sea-bass sea-bass commented Mar 18, 2024

Description

This is a potential fix for #2749.

It changes the KDL parameters introduced in #1671 from:

panda_arm:
  kinematics_solver: "kdl_kinematics_plugin/KDLKinematicsPlugin"
  kinematics_solver_search_resolution: 0.005
  kinematics_solver_timeout: 0.05

  joints: ["joint1", "joint2", "joint3", "joint4", "joint5", "joint6"]
  # Default weight for joints is 1.0 
  joint1:
    weight: 2.0
  joint5:
    weight: 1.5

to

panda_arm:
  kinematics_solver: "kdl_kinematics_plugin/KDLKinematicsPlugin"
  kinematics_solver_search_resolution: 0.005
  kinematics_solver_timeout: 0.05

  joints: ["joint1", "joint2", "joint3", "joint4", "joint5", "joint6"]
  # Default weight for joints is 1.0 
  weights:
    joint1:
      value: 2.0
    joint5:
      value: 1.5

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

@sea-bass sea-bass requested review from Abishalini and pac48 March 18, 2024 20:29
@sea-bass sea-bass changed the title Fix KDL IK plugin parameters specification Fix KDL IK plugin joint weight parameters specification Mar 18, 2024
@sea-bass sea-bass changed the title Fix KDL IK plugin joint weight parameters specification Change KDL IK plugin joint weight parameters specification Mar 18, 2024
@sea-bass
Copy link
Contributor Author

sea-bass commented Mar 18, 2024

Ugh, seems like MoveIt Setup Assistant has some failures due to the addition of the nonzero velocity here: moveit/moveit_resources#198.

This is fixed by #2751, which I temporarily included in this branch as well to see if Humble CI passes.

Copy link
Contributor

@sjahr sjahr left a comment

Choose a reason for hiding this comment

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

Do we need to update some moveit_resources packages?

@sjahr sjahr mentioned this pull request Mar 19, 2024
6 tasks
@sea-bass
Copy link
Contributor Author

Do we need to update some moveit_resources packages?

Actually, no, because none of the example configs even sets joint weights in the first place!

But ler's not merge this in yet, as we could also opt to fix generate_parameter_library

@rhaschke
Copy link
Contributor

Why don't you simply use a map from joint name to weight (instead of an additional indirection layer):

  weights:
    joint1: 2.0
    joint5: 1.5

@sea-bass
Copy link
Contributor Author

Why don't you simply use a map from joint name to weight (instead of an additional indirection layer):

  weights:
    joint1: 2.0
    joint5: 1.5

That does sound good, but I'm not sure if generate_parameter_library supports doing this at the moment. Looking at the examples: https://github.com/PickNikRobotics/generate_parameter_library?tab=readme-ov-file#mapped-parameters, seems like there has to be a field underneath the mapped parameters.

@rhaschke
Copy link
Contributor

I'm not sure if generate_parameter_library supports doing this at the moment.

If it doesn't, you probably should fix that library. A library implementation shouldn't impose constraints on the kind of parameter structures available.

@Abishalini
Copy link
Contributor

If humble CI jobs pass, would it make sense to merge this while a fix to generate_param_library is being worked on?
This would unblock other PRs in the repo

@sea-bass sea-bass added backport-humble Mergify label that triggers a PR backport to Humble backport-iron Mergify label that triggers a PR backport to Iron labels Mar 19, 2024
@sea-bass
Copy link
Contributor Author

Seems there is interest in fixing CI for now, so will open up #2756 and we'll make sure we get that in for the next MoveIt 2 release.

@sea-bass
Copy link
Contributor Author

Update: I think I fixed generate_parameter_library to be able to keep the old syntax: PickNikRobotics/generate_parameter_library#185

@sea-bass sea-bass closed this Mar 27, 2024
@gavanderhoorn
Copy link
Member

@sea-bass: could the branch be deleted?

@sea-bass sea-bass deleted the fix-kdl-weight-params branch May 22, 2024 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-humble Mergify label that triggers a PR backport to Humble backport-iron Mergify label that triggers a PR backport to Iron
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants