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

Adding the drakeSimulator class and a working example of drake_walking.ipynb #4

Merged
merged 8 commits into from
Jun 13, 2024

Conversation

akhilsathuluri
Copy link
Contributor

@akhilsathuluri akhilsathuluri commented Nov 30, 2023

This is a draft PR for the newly added class which are bindings of CoMoDo with drake and an example showing how it works.
The class follows almost exactly the same design of the mujocoSimulator class.
I also added a helper class that bridges between any one coming from a different platform to make their URDFs compatible with drake and simplifies loading. However, this needs the odio_urdf from my repo: https://github.com/akhilsathuluri/odio_urdf and some additional dependencies, which needs to be handled seperately.

For the sake of a common structure, I added the example in Jupyter too, but it is not comfortable to maintain it (especially when the JSON fails due to git merge issues). An alternative is: https://github.com/mwouts/jupytext

In the next push, I will have modified include files to add the angular_momentum and contact_force_regularisation tasks sclaed by robots mass.
(I changed the includes for faster prototyping.)
I have observed the same things as you mentioned today, that the robot falls down after a couple of steps in mujoco too.

@akhilsathuluri akhilsathuluri self-assigned this Nov 30, 2023
@traversaro
Copy link
Contributor

However, this needs the odio_urdf from my repo: https://github.com/akhilsathuluri/odio_urdf and some additional dependencies, which needs to be handled seperately.

As odio_urdf mantainer are not replying, could we do a friendly fork of it called amo_urdf (the italian joke, "odio" means "I hate", "amo" I love) so that we can depend on it without problems?

@akhilsathuluri akhilsathuluri changed the title Draft PR with drakeSimulator class and a working example of drake_walking.ipynb Adding the drakeSimulator class and a working example of drake_walking.ipynb Nov 30, 2023
@akhilsathuluri
Copy link
Contributor Author

However, this needs the odio_urdf from my repo: https://github.com/akhilsathuluri/odio_urdf and some additional dependencies, which needs to be handled seperately.

As odio_urdf mantainer are not replying, could we do a friendly fork of it called amo_urdf (the italian joke, "odio" means "I hate", "amo" I love) so that we can depend on it without problems?

@traversaro, please let me know what to do about this.

Copy link
Collaborator

@CarlottaSartore CarlottaSartore left a comment

Choose a reason for hiding this comment

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

I tested also on my machine and everything works fine ! Thank you really much @akhilsathuluri for the PR !
Few comments needs to be addressed before merging, I would also ask you if you can generally clean up the code from comments that do not document the code, and open issues if there is anything you would like to implement in the future or features you would like to have in comodo ! Thanks

@traversaro
Copy link
Contributor

However, this needs the odio_urdf from my repo: https://github.com/akhilsathuluri/odio_urdf and some additional dependencies, which needs to be handled seperately.

As odio_urdf mantainer are not replying, could we do a friendly fork of it called amo_urdf (the italian joke, "odio" means "I hate", "amo" I love) so that we can depend on it without problems?

Ok, in the end we did this, currently private repo available at https://github.com/ami-iit/amo-urdf .

fyi @akhilsathuluri @DanielePucci

@akhilsathuluri
Copy link
Contributor Author

@CarlottaSartore : I modified some of the methods based on the comments. Do have a look and let me know.

@CarlottaSartore
Copy link
Collaborator

Ciao @akhilsathuluri , Thank you, and sorry for the delay.

Most of the comments have been implemented! I would kindly ask you to address this comment on the hardcoded feet collision, before merging!
If you can add some reference also for the other hardcoded values it will be great! Thanks!

akhilsathuluri and others added 2 commits June 11, 2024 11:32
@akhilsathuluri
Copy link
Contributor Author

Hello @CarlottaSartore, here are the changes:

  • removed hardcoded feet collisions (x and y values for the boxes)
  • added argument names for collision properties (foot/ground dissipation and stiffness)
  • Added comments for other hardcoded numbers (RGBA values for visual and collision meshes for the ground plane)

Please let me know if you think more changes are necessary. Also, apologies for the delay 😅

@akhilsathuluri akhilsathuluri merged commit e3adc9c into ami-iit:main Jun 13, 2024
@CarlottaSartore
Copy link
Collaborator

Everything looks in place, thanks for your contribution @akhilsathuluri :)

CarlottaSartore pushed a commit that referenced this pull request Jul 4, 2024
Merging upstream changes with local
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.

3 participants