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

feat(build): handle installation and linking of dependencies for pinocchio collision support #161

Merged
merged 7 commits into from
Mar 12, 2024

Conversation

domire8
Copy link
Member

@domire8 domire8 commented Mar 12, 2024

Description

This PR solves the issue by installing all required dependencies and add compile definitions for the collision support with pinocchio.

Review guidelines

Estimated Time of Review: 5 minutes

Checklist before merging:

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

@domire8 domire8 linked an issue Mar 12, 2024 that may be closed by this pull request
@domire8
Copy link
Member Author

domire8 commented Mar 12, 2024

The tests fail since the base docker image used to run the tests on doesn't have any of our new dependencies installed..i'd say we run the tests ourselves until we figure out the new CI (#159)

RUN --mount=type=cache,target=${HOME}/.cache,id=pip-${TARGETPLATFORM}-${CACHEID},uid=1000 \
python3 -m pip install --prefix=/tmp/python /python
RUN mv /tmp/python/local /tmp/python-usr

FROM cpp-test as python-test

Choose a reason for hiding this comment

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

Seems like this could be part of test? Is there a specific time we might want to run just part of the tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

Most of time in development, we develop in cpp first and then we write the python bindings. So we want to write and test cpp first before we continue with python.

Choose a reason for hiding this comment

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

OK, just seems like it could stay the same stage with all the tests. As long as you keep the C++ tests running first it would be a similar experience. Anyway, not a blocker.

Copy link
Member Author

@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.

@yrh012 does that fix your issue?

@domire8 domire8 force-pushed the feat/pinocchio-deps branch from d581742 to bb9dce0 Compare March 12, 2024 14:22
@yrh012
Copy link

yrh012 commented Mar 12, 2024

@yrh012 does that fix your issue?

All tested and looks good now!

@domire8 domire8 requested a review from LouisBrunner March 12, 2024 15:21
@domire8 domire8 merged commit 5156bb7 into develop Mar 12, 2024
5 of 7 checks passed
@domire8 domire8 deleted the feat/pinocchio-deps branch March 12, 2024 18:42
@github-actions github-actions bot locked and limited conversation to collaborators Mar 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add FCL and Octamap dependencies for collsion detection Support
3 participants