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.

Sign up for GitHub

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

Jump to bottom

generic IK controller #12

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

generic IK controller #12

wants to merge 12 commits into from

Conversation

Simon-Steinmann
Copy link
Contributor

No description provided.

@Simon-Steinmann Simon-Steinmann requested a review from a team as a code owner August 18, 2020 14:28
@omichel
Copy link
Member

omichel commented Aug 18, 2020

Beware, you committed a binary pyc file.

@omichel
Copy link
Member

omichel commented Aug 18, 2020

Also, it would be nice to include a README.md to explain the purpose of this controller and how it is supposed to work (with an example of robot). Ideally you should add a demo world file as well.

@Simon-Steinmann
Copy link
Contributor Author

.pyc is in gitignore... ohhh because of manual upload. And yes, I can add that

@Simon-Steinmann
Copy link
Contributor Author

Should be done now. I seperated the generic controller from the ik_module and the target_spawn module. This way, people can very easily use this ik_module in their own code with these 3 lines:

from ik_module import inverseKinematics
supervisor = Supervisor()
ik = inverseKinematics(supervisor)
ikResults = ik.get_ik(target_pos, target_rot=None)

So it get's decoupled, from where the target_pos and optionally target_rot comes from

@Simon-Steinmann
Copy link
Contributor Author

now it's pretty clean, well commented and error robust (at least in my testing).
Please test it yourself, and also see if you understand the code. There might be a few operations that are not clear and should be split up into multiple with additional comments

@Simon-Steinmann
Copy link
Contributor Author

I found a weird issue. when running the generic controller for the first time, it crashes. Resetting the simulation fixes it. And after that it doesnt occur again

@Simon-Steinmann
Copy link
Contributor Author

Also there is issues with robotic arms that have grippers included. We'll have to find a way to, ideally, automatically exclude those, or at least detect and prompt the user to manually define which links to use

alxy
alxy reviewed Aug 20, 2020
View reviewed changes
default/controllers/generic_inverse_kinematic/ik_module.py Outdated
active_links[i] = link_name in self.motor_names or link_name in self.sensor_names
if active_links[i]:
# ikpy includes the bounds as valid, In Webots they have to be less than the limit
new_lower = self.armChain.links[i].bounds[0] + 0.0000001
Copy link
Contributor

Choose a reason for hiding this comment

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

If bounds are None, that throws an error

@DavidMansolino DavidMansolino mentioned this pull request Aug 21, 2020
Open
@Simon-Steinmann
Copy link
Contributor Author

I fixed the None bounds issue. However, there is still an issue on the ikpy side, when having a joint with torque or velocity limits, but no position limits. I made a PR on their side, hopefully it get's implemented soon.
Phylliade/ikpy#94

@omichel
Copy link
Member

omichel commented Sep 6, 2021

This PR is very old. Shall we review it again now that Phylliade/ikpy#94 is merged or shall we close it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alxy alxy alxy left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Simon-Steinmann @omichel @alxy