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

Reorganizing DART library composition #652

Merged
merged 33 commits into from
Apr 16, 2016
Merged

Reorganizing DART library composition #652

merged 33 commits into from
Apr 16, 2016

Conversation

jslee02
Copy link
Member

@jslee02 jslee02 commented Mar 29, 2016

Similarly to #587, this RP changes the library composition of DART from {dart-core, dart} to {dart, optional components depending on the 3rd party library dependencies).

This pull request includes #651. See this to see only the changes made after #651.

Componentizing collision is saved for later until we figure out how to handle dynamic library linking of dart to the components. For example, dart should be compiled without dart-collision-bullet, but also dart should be able to load dart-collision-bullet in runtime to use the bullet collision detection in the constraint solver.

jslee02 added 4 commits March 28, 2016 22:18
…nt libraries depending on dependency 3rd party library requirements
Conflicts:
	CMakeLists.txt
	appveyor.yml
	ci/script_linux.sh
	ci/script_osx.sh
	dart/gui/osg/examples/CMakeLists.txt
	tutorials/CMakeLists.txt
@costashatz
Copy link
Contributor

How far are you from merging this branch?

Also, you should remove the opengl dependency since as far as I have seen you removed the draw functions from dynamics..

Thanks for all the work..

@jslee02
Copy link
Member Author

jslee02 commented Apr 14, 2016

How far are you from merging this branch?

Probably this PR will be merged today.

set(dart_gui_hdrs ${dart_gui_hdrs} ${hdrs})
set(dart_gui_srcs ${dart_gui_srcs} ${srcs})
set(dart_gui_hdrs ${dart_gui_hdrs} ${hdrs} ${dart_renderer_hdrs})
set(dart_gui_srcs ${dart_gui_srcs} ${srcs} ${dart_renderer_srcs})

# Library
dart_add_library(dart-gui ${srcs} ${hdrs})
Copy link
Contributor

Choose a reason for hiding this comment

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

The errors come from the fact that we need to add the dart_renderer_hdrs and dart_renderer_srcs in the dart_add_library command too..

Copy link
Member Author

Choose a reason for hiding this comment

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

You're correct. This was resolved as renderer was merged into gui as planned.

jslee02 added 6 commits April 14, 2016 10:03
Here is a quick summary of the dependencies.

- mandatories for core library: eigen, ccd, fcl, assimp, boost
- optional for component libraries:
  - dart-optimizer: nlopt, ipopt
  - dart-collision: bullet
  - dart-planning: flann
  - dart-utils: tinyxml, tinyxml2, urdfdom
  - dart-gui: opengl, glut, openscenegraph
…n 0.4) still returns incorrect contact points for primitive shapes
@jslee02
Copy link
Member Author

jslee02 commented Apr 14, 2016

In the recent commits:

I believe this PR is ready to merge once all the ci tests are passed and if no one has an objection.

@jslee02
Copy link
Member Author

jslee02 commented Apr 14, 2016

@mxgrey Could you take a look at this and merge if it looks good to you?

I merged @costashatz's pull request and added more changes for setting PointMass's collision status.

@mxgrey
Copy link
Member

mxgrey commented Apr 15, 2016

Can we make planning a component and make flann an optional dependency? I'll attempt to do this myself and make a pull request targeted at this branch to make it easy to review my changes.

After looking more closely, it seems that this is already the case. I was thrown off by this line which seems to imply that flann is a dependency of DART itself. I don't know if that variable is important or not, but perhaps it should be updated.

Adding collision sets to CollisionResult
@jslee02
Copy link
Member Author

jslee02 commented Apr 16, 2016

I don't know if that variable is important or not, but perhaps it should be updated.

It's used for generating dart.pc, which is a pkg-config file. In most case, DART will be found by DARTConfig.cmake so it wouldn't be used that much. But, yeah, it should be updated. However, I honestly don't know how to set it up for DART components.

@@ -1035,7 +1035,7 @@ class BodyNode :
UniqueProperties mBodyP;

/// Whether the node is currently in collision with another node.
DEPRECATED(6.0)
/// \deprecated DEPRECATED(6.0) See #670 for more detail.
Copy link
Member Author

@jslee02 jslee02 Apr 16, 2016

Choose a reason for hiding this comment

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

Removed the used of DEPRECATED(6.0) for member variable because there is no way to suppress warnings for this. Instead, I put DEPRECATED(6.0) into comment so that we can't miss this when searching deprecated API using this keyword.

@mxgrey
Copy link
Member

mxgrey commented Apr 16, 2016

I'm no build system expert, but everything here looks reasonable to me. If @jslee02 is finished with this, I see no problem with merging. 👍

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