Skip to content

Change KDL IK plugin joint weight parameters specification#2750

Closed
sea-bass wants to merge 4 commits intomainfrom
fix-kdl-weight-params
Closed

Change KDL IK plugin joint weight parameters specification#2750
sea-bass wants to merge 4 commits intomainfrom
fix-kdl-weight-params

Conversation

@sea-bass
Copy link
Copy Markdown
Contributor

@sea-bass sea-bass commented Mar 18, 2024 •
edited
Loading

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 mentioned this pull request Mar 18, 2024
Open
@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
Copy Markdown
Contributor Author

sea-bass commented Mar 18, 2024 •
edited
Loading

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.

sjahr
sjahr approved these changes Mar 19, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@sjahr sjahr left a comment •
edited
Loading

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
Merged
6 tasks
@sea-bass
Copy link
Copy Markdown
Contributor Author

sea-bass commented Mar 19, 2024

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
Copy Markdown
Contributor

rhaschke commented Mar 19, 2024

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
Copy Markdown
Contributor Author

sea-bass commented Mar 19, 2024

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
Copy Markdown
Contributor

rhaschke commented Mar 19, 2024

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
Copy Markdown
Contributor

Abishalini commented Mar 19, 2024

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 sea-bass mentioned this pull request Mar 19, 2024
Closed
@sea-bass
Copy link
Copy Markdown
Contributor Author

sea-bass commented Mar 19, 2024

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
Copy Markdown
Contributor Author

sea-bass commented Mar 20, 2024

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
Copy Markdown
Member

gavanderhoorn commented May 22, 2024

@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

Reviewers

@henningkayser henningkayser henningkayser approved these changes

@Abishalini Abishalini Abishalini approved these changes

@sjahr sjahr sjahr approved these changes

@pac48 pac48 Awaiting requested review from pac48

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sea-bass @rhaschke @Abishalini @gavanderhoorn @henningkayser @sjahr