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

Merge master branch into #531 #591

Merged
merged 114 commits into from
Jan 22, 2016
Merged

Merge master branch into #531 #591

merged 114 commits into from
Jan 22, 2016

Conversation

jslee02
Copy link
Member

@jslee02 jslee02 commented Jan 21, 2016

This pull request merges master branch into grey/addons for #531.

mxgrey and others added 30 commits October 24, 2015 23:49
Delete copy constructors for non-copyable classes (round 2) -- Patch for 5.1
Delete copy constructors for non-copyable classes (round 2)
Fix const correctness of BodyNode::getMomentOfInertia()
- DART now is able to detect if installed Assimp is missing C++ symbols so we don't need to restrict DART to depend on the version of Assimp that is missing the symbols.
For backward compatibility, we have to call deprecated functions in Function.cpp, and compilers complain for these calls. This commit suppresses the warnings only for those calls. See also #544.

Note that the suppression code couldn't be made as macros. It needs to use compiler dependent #pragma directives but directives is not allowed to be decleared in macros.
Conditional build depending on missing symbols of Assimp package
Add cmake option DART_MSVC_DEFAULT_OPTIONS for Visual Studio build
donnyward and others added 15 commits January 2, 2016 11:40
'int': forcing value to bool 'true' or 'false'. Visual Studio 2015 won't
allow the conversion from int to bool.
Preliminary implementation for servo motor
Fix Visual Studio 2015 build errors
Updating the constraint namespace to C++11
Fixed segmentation fault for null meshes
… it's not being called anywhere, and compile ConstrainedGroup::containConstraint() only in debug mode
Implement missing implementation in ConstrainedGroup
# Resolved conflicts:
#	dart/dynamics/MultiDofJoint.h
@jslee02 jslee02 added this to the DART 6.0.0 milestone Jan 21, 2016
@mxgrey
Copy link
Member

mxgrey commented Jan 21, 2016

It probably wouldn't make much sense for me to review this pull request in detail since it's coming from master. But were there any significant merge conflicts that I should look at?

@jslee02
Copy link
Member Author

jslee02 commented Jan 21, 2016

There was one conflict on dart/dynamics/MultiDofJoint.h, and I resolved it simply. You would like to check the last commit for sure.

Edit: Here is the conflict in case it helps.

/// class MultiDofJoint
template<size_t DOF>
class MultiDofJoint :
    public Joint,
    public virtual common::SpecializedAddonManager< detail::MultiDofJointAddon<DOF> >
{
public:

  constexpr static size_t NumDofs = DOF;
  using Vector = Eigen::Matrix<double, DOF, 1>;

<<<<<<< HEAD
  using UniqueProperties = detail::MultiDofJointUniqueProperties<DOF>;
  using Properties = detail::MultiDofJointProperties<DOF>;
  using Addon = detail::MultiDofJointAddon<DOF>;
=======
  MultiDofJoint(const MultiDofJoint&) = delete;

  struct UniqueProperties
  {
    /// Lower limit of position
    Vector mPositionLowerLimits;
>>>>>>> master

  DART_BAKE_SPECIALIZED_ADDON_IRREGULAR( MultiDofJoint<DOF>::Addon, MultiDofJointAddon )

  /// Destructor
  virtual ~MultiDofJoint();

and I resolved this as:

/// class MultiDofJoint
template<size_t DOF>
class MultiDofJoint :
    public Joint,
    public virtual common::SpecializedAddonManager< detail::MultiDofJointAddon<DOF> >
{
public:

  constexpr static size_t NumDofs = DOF;
  using Vector = Eigen::Matrix<double, DOF, 1>;

  using UniqueProperties = detail::MultiDofJointUniqueProperties<DOF>;
  using Properties = detail::MultiDofJointProperties<DOF>;
  using Addon = detail::MultiDofJointAddon<DOF>;

  DART_BAKE_SPECIALIZED_ADDON_IRREGULAR( MultiDofJoint<DOF>::Addon, MultiDofJointAddon )

  MultiDofJoint(const MultiDofJoint&) = delete;

  /// Destructor
  virtual ~MultiDofJoint();

@jslee02
Copy link
Member Author

jslee02 commented Jan 21, 2016

It seems AppVeyor is failing to build since constexpr is not supported in Visual Studio 2013 (2015 supports it). I'm thinking to use a macro to resolve this something like:

// constexpr is not supported until Visual Studio 2015 (_MSC_VER == 1900)
#if defined _MSC_VER && _MSC_VER < 1900
  #define DART_CONSTEXPR const
#else
  #define DART_CONSTEXPR constexpr
#endif

@mxgrey
Copy link
Member

mxgrey commented Jan 21, 2016

There might be some things in the Addon/Node implementations that rely on constexpr variables for template arguments (I don't know for sure off the top of my head; we'd have to test it), so replacing it with const might not necessarily work.

I believe VS2015 is just as readily available on Windows 7 as VS2013, so would it not be fair to make VS2015 the minimum requirement? Or does Travis-CI not support VS2015 yet?

@jslee02
Copy link
Member Author

jslee02 commented Jan 21, 2016

You're right. The macro couldn't resolve it.

As VS2015 community is free for everyone, it seems to be fair to make VS2015 the minimum requirement.

AppVeyor supports VS2015, but the problem is that we don't have the DART dependencies built with VS2015 yet. I'm working on this, but it would take more days. So I'd like to increase the minimum requrired VS version to 2015 and don't care the build test until I create the dependencies then.

- also cmake 3.1.3 is now the minimum requirement for Visual Studio since MSVC14 variable is supported since that cmake version.
@jslee02
Copy link
Member Author

jslee02 commented Jan 22, 2016

I made Visual Studio 2015 the minimum requirement in the last two commit. @mxgrey Does this look good to merge?

@mxgrey
Copy link
Member

mxgrey commented Jan 22, 2016

Your resolution of the merge conflict looks good to me. If that was the only major one, I think we should be good to merge.

@jslee02
Copy link
Member Author

jslee02 commented Jan 22, 2016

Yes, it was the only conflict. Merging now. Thanks!

jslee02 added a commit that referenced this pull request Jan 22, 2016
@jslee02 jslee02 merged commit bded530 into grey/addons Jan 22, 2016
@jslee02 jslee02 deleted the grey/addons_merging_master branch January 27, 2016 06:34
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.

7 participants