Skip to content

Add new IK/FK virtual methods and const correctness in base interface#297

Open
oguzhanbzglu wants to merge 3 commits into
ros-controls:masterfrom
b-robotized-forks:feat/api-const-changes
Open

Add new IK/FK virtual methods and const correctness in base interface#297
oguzhanbzglu wants to merge 3 commits into
ros-controls:masterfrom
b-robotized-forks:feat/api-const-changes

Conversation

@oguzhanbzglu
Copy link
Copy Markdown

@oguzhanbzglu oguzhanbzglu commented May 15, 2026

Summary

Extends the KinematicsInterface base class with optional IK/FK methods and
const-correctness in calculate_frame_difference.

Changes

kinematics_interface (base class):

  • Add four new optional virtual methods with default return false implementations:
    • convert_cartesian_pose_to_closest_joint_state — IK to closest solution given seed state
    • convert_cartesian_pose_to_joint_state_within_range — IK within joint range constraints
    • convert_cartesian_pose_to_possible_joint_states — IK returning all solutions
    • convert_joint_state_to_cartesian_pose — FK returning Cartesian pose
  • calculate_frame_difference: input parameters x_a and x_b are now const

kinematics_interface_kdl and kinematics_interface_pinocchio (existing interfaces):

  • Update calculate_frame_difference signature to match the corrected base class

Note

  • The new virtual methods have a default return false so existing plugins are not broken.
  • The const update and the virtual methods also can be separated from this MR if this seems necessary.

See also other PRs:

@codecov
Copy link
Copy Markdown

codecov Bot commented May 17, 2026

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.93%. Comparing base (7f7aade) to head (d991e80).

Files with missing lines Patch % Lines
...lude/kinematics_interface/kinematics_interface.hpp 0.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #297      +/-   ##
==========================================
- Coverage   80.11%   78.93%   -1.18%     
==========================================
  Files           7        7              
  Lines         538      546       +8     
  Branches      249      250       +1     
==========================================
  Hits          431      431              
- Misses         39       47       +8     
  Partials       68       68              
Flag Coverage Δ
unittests 78.93% <0.00%> (-1.18%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ics_interface_kdl/src/kinematics_interface_kdl.cpp 74.14% <ø> (ø)
...e_pinocchio/src/kinematics_interface_pinocchio.cpp 75.00% <ø> (ø)
...lude/kinematics_interface/kinematics_interface.hpp 20.00% <0.00%> (-80.00%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

not an expert here, but IMHO this is breaking ABI so we can't backport any of these changes.

/**
* \brief Convert joint state to Cartesian pose using forward kinematics.
*/
virtual bool convert_joint_state_to_cartesian_pose(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't this the same as calculate_link_transform? I don't think that this is compatible with the current API if we don't pass the link_name.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes that is true thanks for pointing. I have removed it.

@oguzhanbzglu
Copy link
Copy Markdown
Author

not an expert here, but IMHO this is breaking ABI so we can't backport any of these changes.

The const changes cause a problem for ABI pipeline. I removed them.

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.

2 participants