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

feat: integrate collision detection feature into robot model #163

Merged
merged 57 commits into from
Apr 3, 2024
Merged

feat: integrate collision detection feature into robot model #163

merged 57 commits into from
Apr 3, 2024

Conversation

yrh012
Copy link

@yrh012 yrh012 commented Mar 13, 2024 •
edited
Loading

Description

This PR solves the issue by implementing 3 new attributes to the robot::Model class, namely check_collision and get_number_of_collision_pairs and is_geometry_model_initialized. These new attributes along with the default new parameter in the constructor allows for self-collision checking based on the URDF and geometries of the robot.

Accordingly by providing paths to the URDF and it's corresponding collision meshes when initializing/creating robot::Model class, a user can allow for collision checking given a certain joint positions configuration.

One possible improvement yet to consider is the paths of the meshes inside the URDFs used in the test.

Review guidelines

Estimated Time of Review: 15 minutes

Checklist before merging:

  • Confirm that the relevant changelog(s) are up-to-date in case of any user-facing changes

@yrh012 yrh012 self-assigned this Mar 13, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 13, 2024 •
edited
Loading

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@yrh012 yrh012 linked an issue Mar 13, 2024 that may be closed by this pull request
Integrate Collision Detection into robot_model::Model Class #164
Closed
@yrh012
Copy link
Author

yrh012 commented Mar 13, 2024

I have read and understood the Contributor License Agreement and hereby sign it

@domire8
Copy link
Member

domire8 commented Mar 13, 2024

recheck

1 similar comment
@yrh012
Copy link
Author

yrh012 commented Mar 13, 2024

recheck

github-actions bot added a commit that referenced this pull request Mar 13, 2024
@github-actions
@yrh012 has signed the CLA in #163
6ac03d9
domire8
domire8 reviewed Mar 13, 2024
View reviewed changes
source/robot_model/include/robot_model/Model.hpp Outdated Show resolved Hide resolved
source/robot_model/include/robot_model/Model.hpp Outdated Show resolved Hide resolved
source/robot_model/include/robot_model/Model.hpp Outdated Show resolved Hide resolved
source/robot_model/include/robot_model/Model.hpp Outdated Show resolved Hide resolved
source/robot_model/src/Model.cpp Outdated Show resolved Hide resolved
source/robot_model/src/Model.cpp Outdated Show resolved Hide resolved
source/robot_model/test/fixtures/ur5e.urdf Outdated Show resolved Hide resolved
source/robot_model/test/tests/test_collisions.cpp Outdated Show resolved Hide resolved
source/robot_model/include/robot_model/Model.hpp Show resolved Hide resolved
yrh012 and others added 3 commits March 13, 2024 17:12
@yrh012 @domire8
Update source/robot_model/include/robot_model/Model.hpp
044e891 
Co-authored-by: Dominic Reber <71256590+domire8@users.noreply.github.com>
@yrh012
fix: apply review changes
584a2df
@yrh012
Merge branch 'feat/collision-detection-robot' of github.com:aica-tech…
ae1f05f 
…nology/control-libraries into feat/collision-detection-robot
domire8
domire8 reviewed Mar 14, 2024
View reviewed changes
Copy link
Member

@domire8 domire8 left a comment

Choose a reason for hiding this comment

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

Looks better, would like to leave that here for more reviews

python/source/robot_model/bind_model.cpp Outdated Show resolved Hide resolved
source/robot_model/include/robot_model/Model.hpp Outdated Show resolved Hide resolved
@domire8 domire8 mentioned this pull request Mar 25, 2024
Closed
domire8
domire8 reviewed Mar 27, 2024
View reviewed changes
Copy link
Member

@domire8 domire8 left a comment

Choose a reason for hiding this comment

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

Hold on a sec, how do we use these new constructors now?
The existing one is unchanged, clear to me. The new one has an std::optional function, so what if you want to load the collision but you already know that the URDF has correct absolute paths? Do you do provide a nullptr to the constructor?

source/robot_model/src/Model.cpp Show resolved Hide resolved
yrh012 and others added 3 commits March 27, 2024 08:38
@yrh012 @eeberhard
fix: apply nitpicks from code review
4b312ca 
Co-authored-by: Enrico Eberhard <32450951+eeberhard@users.noreply.github.com>
@yrh012
fix: add flag for copy constructor
0dbb1ab
@yrh012
feat: add default flag
d119712
domire8
domire8 reviewed Mar 27, 2024
View reviewed changes
Copy link
Member

@domire8 domire8 left a comment

Choose a reason for hiding this comment

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

Almost good to go, thanks for the patience here!

source/robot_model/include/robot_model/Model.hpp Outdated Show resolved Hide resolved
source/robot_model/src/Model.cpp Outdated Show resolved Hide resolved
source/robot_model/src/Model.cpp Outdated Show resolved Hide resolved
source/robot_model/src/Model.cpp Outdated Show resolved Hide resolved
source/robot_model/src/Model.cpp Outdated Show resolved Hide resolved
source/robot_model/src/Model.cpp Show resolved Hide resolved
yrh012 and others added 4 commits March 28, 2024 10:00
@yrh012 @domire8
fix: suggestions from code review
72df3be 
Co-authored-by: Dominic Reber <71256590+domire8@users.noreply.github.com>
@yrh012
fix: last nitpicks
b983c92
@yrh012
fix: spaces
553727d
@domire8
formatting
2d52b88
@eeberhard eeberhard changed the title feat: integrating collision detection feature into robot model feat: integrate collision detection feature into robot model Mar 28, 2024
yrh012 and others added 3 commits March 28, 2024 15:19
@yrh012 @domire8
fix: space between namings
ab4e04a 
Co-authored-by: Dominic Reber <71256590+domire8@users.noreply.github.com>
@yrh012
Merge branch 'develop' into feat/collision-detection-robot
0d2d7f6
@github-actions
7.3.7 -> 7.3.8
439e0dd
domire8
domire8 previously approved these changes Mar 28, 2024
View reviewed changes
Copy link
Member

@domire8 domire8 left a comment

Choose a reason for hiding this comment

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

Great piece of work

domire8
domire8 reviewed Apr 2, 2024
View reviewed changes
Copy link
Member

@domire8 domire8 left a comment

Choose a reason for hiding this comment

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

You need to set version 7.3.10 manually now

./update_version --version 7.3.10 --commit

python/setup.py Outdated Show resolved Hide resolved
@yrh012 @domire8
fix: remove space
e8ab208 
Co-authored-by: Dominic Reber <71256590+domire8@users.noreply.github.com>
@domire8
Copy link
Member

domire8 commented Apr 2, 2024

You need to set version 7.3.10 manually now

./update_version --version 7.3.10 --commit

This still

@yrh012
Copy link
Author

yrh012 commented Apr 2, 2024

You need to set version 7.3.10 manually now

./update_version --version 7.3.10 --commit

This still

done!

eeberhard
eeberhard approved these changes Apr 3, 2024
View reviewed changes
Copy link
Member

@eeberhard eeberhard left a comment

Choose a reason for hiding this comment

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

Amazing work and thanks for the patience!

@yrh012 yrh012 merged commit 852f164 into develop Apr 3, 2024
4 checks passed
@yrh012 yrh012 deleted the feat/collision-detection-robot branch April 3, 2024 09:05
@github-actions github-actions bot locked and limited conversation to collaborators Apr 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@domire8 domire8 domire8 approved these changes

@eeberhard eeberhard eeberhard approved these changes

@bpapaspyros bpapaspyros bpapaspyros approved these changes

@LouisBrunner LouisBrunner Awaiting requested review from LouisBrunner

Assignees

@yrh012 yrh012

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Integrate Collision Detection into robot_model::Model Class
4 participants
@yrh012 @domire8 @bpapaspyros @eeberhard