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

Refactor Robot Model Factory #116

Conversation

ArthurH91
Copy link
Collaborator

Hello,

in this PR, the refactoring of the robot model factory. This has to be merged before the OCP class PR.

I ditched a lot of stuff to keep the essential: a robot wrapper with armature. Please, say if you need / want some other stuff in it.

I decided to get rid of the story of "complete' collision/visual models" because in my opinion it doesn't make much sense. Either it's mesh or simple convex shapes, but there is no reduction. If you're against, again, let's discuss it in the comments.

There is a function to directly parse the urdf and from sphere cylinder, obtain a collision model with capsules instead of sphere cylinder.

That's about it, it still needs unit testing, i'm working on it right now.

Copy link
Contributor

@Kotochleb Kotochleb left a comment

Choose a reason for hiding this comment

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

There is missing option to load model from string, which is crucial for ROS integration

@ArthurH91 ArthurH91 marked this pull request as ready for review January 10, 2025 13:06
@ArthurH91
Copy link
Collaborator Author

ArthurH91 commented Jan 10, 2025

I let the XML comment for a future work, because i'm not sure how to load the collision model with XML #118

Copy link
Contributor

@Kotochleb Kotochleb left a comment

Choose a reason for hiding this comment

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

@MaximilienNaveau shouldn't this class be moved outside of the factory folder?

agimus_controller/agimus_controller/factory/robot_model.py Outdated Show resolved Hide resolved
agimus_controller/agimus_controller/factory/robot_model.py Outdated Show resolved Hide resolved
agimus_controller/agimus_controller/factory/robot_model.py Outdated Show resolved Hide resolved
agimus_controller/tests/test_robot_models.py Outdated Show resolved Hide resolved
agimus_controller/tests/test_robot_models.py Outdated Show resolved Hide resolved
agimus_controller/tests/test_robot_models.py Outdated Show resolved Hide resolved
agimus_controller/tests/test_robot_models.py Outdated Show resolved Hide resolved
agimus_controller/tests/test_robot_models.py Show resolved Hide resolved
@MaximilienNaveau
Copy link
Collaborator

@MaximilienNaveau shouldn't this class be moved outside of the factory folder?

Yes I believe it should.

@MaximilienNaveau MaximilienNaveau merged commit 3d4f5dc into agimus-project:topic/humble-devel/refactor Jan 11, 2025
2 of 4 checks passed
@ArthurH91 ArthurH91 mentioned this pull request Jan 17, 2025
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.

4 participants