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

[fix] missing const in arguments of RobotWrapper.set_rotor_inertia and RobotWrapper.set_gear_ratios python bindings #245

Merged
merged 6 commits into from
Nov 21, 2024

Conversation

domrachev03
Copy link

This MR adds missing const identifier to the arguments of two methods of RobotWrapper: set_rotor_inertia and set_gear_ratios in python bingings. This problem practically disallowed to use motor inertia in python bindings.

Right now, this functionality does not work, and an attempt to run the simplest example:

from copy import deepcopy

import numpy as np
import pinocchio as pin
import tsid
from robot_descriptions.iiwa7_description import URDF_PATH

pin_model = pin.buildModelFromUrdf(URDF_PATH)
robot = tsid.RobotWrapper(pin_model, False)

robot.set_gear_ratios(np.ones(12))
robot.set_rotor_inertias(np.ones(12))

print(robot.rotor_inertias)
print(robot.gear_ratios)

results in:

Boost.Python.ArgumentError: Python argument types in
    RobotWrapper.set_gear_ratios(RobotWrapper, numpy.ndarray)
did not match C++ signature:
    set_gear_ratios(tsid::robots::RobotWrapper {lvalue}, Eigen::Matrix<double, -1, 1, 0, -1, 1> {lvalue} gear ratio vector)

@nim65s
Copy link
Contributor

nim65s commented Nov 20, 2024

Thanks !
Could you add a regression test, to avoid this issue to happen again ?

@domrachev03
Copy link
Author

Done! I've modified a tests/python/test_RobotWrapper test to add a motor's inertias and check that it's computed correctly.

I am not 100% sure I correctly understood what do you mean by regression test, and if you want me to implement smth else, I would be glad to do that!

Copy link
Contributor

@nim65s nim65s left a comment

Choose a reason for hiding this comment

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

You did exactly what was expected, thanks !

@domrachev03
Copy link
Author

Glad to hear that! Since the pipeline is failing in the devel as well, I guess we are good to merge those changes? Or something else is missing?

@nim65s
Copy link
Contributor

nim65s commented Nov 21, 2024

Yes, both Gitlab & ROS CI are fine, so we can merge :)

@nim65s nim65s merged commit 0d62ba2 into stack-of-tasks:devel Nov 21, 2024
4 of 8 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.

2 participants