-
Notifications
You must be signed in to change notification settings - Fork 26
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
use drake cmakelists #233
use drake cmakelists #233
Conversation
@gizatt looks like the build is failing due to an issue with |
I'm seeing an Apriltags build error, not VHACD, in the Jenkins build log:
The old use-Drake-CMakelists upgrade had a VHACD failure, so I would expect to see one here (it has to be related to an environment change due to the top-level CMake changes). It's OK to turn it off to get this old branch landed if you can verify the error still happens -- change the default in the top-level CMakelist and change the Jenkins CMake build command in |
The build is now failing with
it looks like apriltags driver is not finding yaml-cpp. @jamiesnape I am guessing this related to this PR? This is in the director build and I guess drake no longer supplies yaml-cpp linker flags. @gizatt yes the original error was VHACD, which I then fixed by disabling it in the jenkins build. This yaml-cpp error is new. |
@jamiesnape so I guess drake still has yaml-cpp,. I am not sure why the apriltags-driver is suddenly having problems linking against yaml-cpp. Do you think it is related to the bug in yaml-cpp mentioned in RobotLocomotion/drake#7602? As a short term fix we can disable apriltags_drivers as no one is using them. |
Looks like the variable ( |
Ok thanks @jamiesnape that makes sense. I will implement a fix upstream in the apriltags driver or disable it. It seems like adding |
Yes:
|
@@ -11,6 +11,8 @@ include_directories(${Eigen3_INCLUDE_DIR}) | |||
find_package(VTK REQUIRED) | |||
include_directories(${VTK_INCLUDE_DIRS}) | |||
|
|||
find_path(yaml-cpp REQUIRED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
find_package
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, typo on my part. Just pushed a fix.
I just tested the IK issue from #234 and it seems to be working fine. I think this is ready to be merged. @peteflorence @gizatt |
Reviewed 1 of 3 files at r1, 4 of 5 files at r2, 1 of 1 files at r3, 1 of 2 files at r4, 1 of 1 files at r5. src/ReachabilityAnalyzer/CMakeLists.txt, line 26 at r5 (raw file):
I think more "correct" would be ${YAML_CPP_LIBRARIES}, but this is probably safer since that variable name hasn't been consistent across system-vs-local Yaml in the recent past... Comments from Reviewable |
@jamiesnape @fbudin69500 I was under the impression that the
and the other in
They have a very different set of lcmtypes. In particular the one in In particular some |
Yes, there was a fork of the |
(you probably want the |
Yeah I am coming to that conclusion. Somehow when I launch files a particular way the |
Via |
Looks like the
I am guessing that |
|
I guess we should be appending rather than prepending, I will PR a change. |
Thanks. Once we do that should we update |
This change is