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 python issue #50 #51

Merged
merged 2 commits into from
Oct 13, 2019
Merged

Conversation

olivier-stasse
Copy link
Member

This PR adds a method to bindings/python/robots/robot-wrapper.hpp
to read a JointModelFreeFlyer python type and call the appropriate constructor of the RobotWrapper object.
It might be needed to extend this if someone wants to use another joint than a FreeFlyer as a root (but it looks unlikely to happen).

Copy link

@MaximilienNaveau MaximilienNaveau left a comment

Choose a reason for hiding this comment

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

I am not sure I am the one to review this as I am no expert in pyhton-bindings and never used tsid...
Though I had some minor comment on the code.

bp::object & bpObject,
bool verbose)
{
std::cout << "Before extraction " << std::endl;

Choose a reason for hiding this comment

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

Is this cout needed? Should it be protected with the verbose boolean?

std::cout << "Before extraction " << std::endl;
pinocchio::JointModelFreeFlyer root_joint =
bp::extract<pinocchio::JointModelFreeFlyer>(bpObject)();
std::cout << "After extraction " << std::endl;

Choose a reason for hiding this comment

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

idem

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunate oversight. Thanks for catching it

Thx to mnaveau for picking it.
@olivier-stasse
Copy link
Member Author

The new commit is fixing the std::cout issue.

@MaximilienNaveau
Copy link

I would say this PR is ready for merging. Thanks for the dev!

@andreadelprete
Copy link
Collaborator

Thanks for this PR @olivier-stasse !

@andreadelprete andreadelprete merged commit 761194b into stack-of-tasks:master Oct 13, 2019
NoelieRamuzat pushed a commit to NoelieRamuzat/tsid that referenced this pull request Feb 18, 2020
@nim65s nim65s mentioned this pull request Mar 1, 2020
nim65s added a commit that referenced this pull request Mar 1, 2020
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.

None yet

3 participants