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

Add Gravity Feature, fix LinkFeatures_TEST #275

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions dartsim/src/LinkFeatures_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <ignition/physics/FrameSemantics.hh>
#include <ignition/physics/GetBoundingBox.hh>
#include <ignition/physics/Link.hh>
#include <ignition/physics/World.hh>
#include <ignition/physics/sdf/ConstructWorld.hh>
#include <ignition/physics/sdf/ConstructModel.hh>
#include <ignition/physics/sdf/ConstructLink.hh>
Expand All @@ -42,6 +43,7 @@
struct TestFeatureList : ignition::physics::FeatureList<
ignition::physics::AddLinkExternalForceTorque,
ignition::physics::ForwardStep,
ignition::physics::Gravity,
ignition::physics::sdf::ConstructSdfWorld,
ignition::physics::sdf::ConstructSdfModel,
ignition::physics::sdf::ConstructSdfLink,
Expand Down Expand Up @@ -79,16 +81,17 @@ TestWorldPtr LoadWorld(
{
sdf::Root root;
const sdf::Errors errors = root.Load(_sdfFile);
EXPECT_TRUE(errors.empty());
EXPECT_TRUE(errors.empty()) << errors;
const sdf::World *sdfWorld = root.WorldByIndex(0);
// Make a copy of the world so we can set the gravity property
// TODO(addisu) Add a world property feature to set gravity instead of this
// hack
sdf::World worldCopy;
worldCopy.Load(sdfWorld->Element());

worldCopy.SetGravity(math::eigen3::convert(_gravity));
return _engine->ConstructWorld(worldCopy);
EXPECT_NE(nullptr, sdfWorld);

auto graphErrors = sdfWorld->ValidateGraphs();
EXPECT_EQ(0u, graphErrors.size()) << graphErrors;

TestWorldPtr world = _engine->ConstructWorld(*sdfWorld);
EXPECT_NE(nullptr, world);
world->SetGravity(_gravity);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add an expectation on GetGravity just to exercise the API?

Copy link
Member Author

Choose a reason for hiding this comment

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

I had intended to do that, but then I realized that I would need to move the AssertVectorApprox class definition up above this function in order to use it in the expectation. So I stalled out on doing it. I'll do it now though

Copy link
Member Author

Choose a reason for hiding this comment

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

done in ef1012d

return world;
}

// A predicate-formatter for asserting that two vectors are approximately equal.
Expand Down Expand Up @@ -119,6 +122,12 @@ TEST_F(LinkFeaturesFixture, LinkForceTorque)
{
auto world = LoadWorld(this->engine, TEST_WORLD_DIR "/empty.sdf",
Eigen::Vector3d::Zero());
{
AssertVectorApprox vectorPredicate(1e-10);
EXPECT_PRED_FORMAT2(vectorPredicate, Eigen::Vector3d::Zero(),
world->GetGravity(physics::FrameID::World()));
}

// Add a sphere
sdf::Model modelSDF;
modelSDF.SetName("sphere");
Expand Down
19 changes: 0 additions & 19 deletions dartsim/src/ShapeFeatures_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,25 +98,6 @@ class ShapeFeaturesFixture : public ::testing::Test
protected: TestEnginePtr engine;
};

TestWorldPtr LoadWorld(
const TestEnginePtr &_engine,
const std::string &_sdfFile,
const Eigen::Vector3d &_gravity = Eigen::Vector3d{0, 0, -9.8})
{
sdf::Root root;
const sdf::Errors errors = root.Load(_sdfFile);
EXPECT_TRUE(errors.empty());
const sdf::World *sdfWorld = root.WorldByIndex(0);
// Make a copy of the world so we can set the gravity property
// TODO(addisu) Add a world property feature to set gravity instead of this
// hack
sdf::World worldCopy;
worldCopy.Load(sdfWorld->Element());

worldCopy.SetGravity(math::eigen3::convert(_gravity));
return _engine->ConstructWorld(worldCopy);
}

// A predicate-formatter for asserting that two vectors are approximately equal.
class AssertVectorApprox
{
Expand Down
16 changes: 16 additions & 0 deletions dartsim/src/WorldFeatures.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,22 @@ const std::string &WorldFeatures::GetWorldCollisionDetector(const Identity &_id)
return world->getConstraintSolver()->getCollisionDetector()->getType();
}

/////////////////////////////////////////////////
void WorldFeatures::SetWorldGravity(
const Identity &_id, const LinearVectorType &_gravity)
{
auto world = this->ReferenceInterface<dart::simulation::World>(_id);
world->setGravity(_gravity);
}

/////////////////////////////////////////////////
WorldFeatures::LinearVectorType WorldFeatures::GetWorldGravity(
const Identity &_id) const
{
auto world = this->ReferenceInterface<dart::simulation::World>(_id);
return world->getGravity();
}

/////////////////////////////////////////////////
void WorldFeatures::SetWorldSolver(const Identity &_id,
const std::string &_solver)
Expand Down
8 changes: 8 additions & 0 deletions dartsim/src/WorldFeatures.hh
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ namespace dartsim {

struct WorldFeatureList : FeatureList<
CollisionDetector,
Gravity,
Solver
> { };

Expand All @@ -45,6 +46,13 @@ class WorldFeatures :
public: const std::string &GetWorldCollisionDetector(const Identity &_id)
const override;

// Documentation inherited
public: void SetWorldGravity(
const Identity &_id, const LinearVectorType &_gravity) override;

// Documentation inherited
public: LinearVectorType GetWorldGravity(const Identity &_id) const override;

// Documentation inherited
public: void SetWorldSolver(const Identity &_id, const std::string &_solver)
override;
Expand Down
78 changes: 77 additions & 1 deletion include/ignition/physics/World.hh
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <string>

#include <ignition/physics/FeatureList.hh>
#include <ignition/physics/FrameSemantics.hh>

namespace ignition
{
Expand Down Expand Up @@ -61,6 +62,81 @@ namespace ignition
};
};

/////////////////////////////////////////////////
using GravityRequiredFeatures = FeatureList<FrameSemantics>;

/////////////////////////////////////////////////
/// \brief Get and set the World's gravity vector in a specified frame.
class IGNITION_PHYSICS_VISIBLE Gravity
: public virtual
FeatureWithRequirements<GravityRequiredFeatures>
{
/// \brief The World API for getting and setting the gravity vector.
public: template <typename PolicyT, typename FeaturesT>
class World : public virtual Feature::World<PolicyT, FeaturesT>
{
public: using LinearVectorType =
typename FromPolicy<PolicyT>::template Use<LinearVector>;

public: using RelativeGravityType = RelativeQuantity<
LinearVectorType, PolicyT::Dim,
detail::VectorSpace<typename PolicyT::Scalar, PolicyT::Dim>>;

/// \brief Set the World gravity vector.
/// \param[in] _gravity The desired gravity as a Relative Gravity
/// (a quantity that contains information about the coordinates
/// in which it is expressed).
public: void SetGravity(const RelativeGravityType &_gravity);

/// \brief Set the World gravity vector. Optionally, you may specify
/// the frame whose coordinates are used to express the gravity vector.
/// The World frame is used as a default if no frame is specified.
/// \param[in] _gravity Gravity vector.
/// \param[in] _inCoordinatesOf Frame whose coordinates are used
/// to express _gravity.
public: void SetGravity(
const LinearVectorType &_gravity,
const FrameID &_forceInCoordinatesOf = FrameID::World());

/// \brief Get the World gravity vector as a Relative Gravity
/// (a quantity that contains information about the coordinates
/// in which it is expressed).
/// \return Relative Gravity vector.
public: RelativeGravityType GetGravity() const;
Copy link
Member Author

Choose a reason for hiding this comment

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

it now occurs to me that the two GetGravity functions will be confusing for compilers since they can both be called with no arguments and have different return types. I'm inclined to remove this one that returns RelativeGravityType

Copy link
Contributor

Choose a reason for hiding this comment

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

It's weird that the compiler doesn't catch this. I'd also vote for removing the function that returns RelativeGravityType.

Copy link
Member Author

Choose a reason for hiding this comment

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

removed in 7fa6ab2


/// \brief Get the World gravity vector. Optionally, you may specify
/// the frame whose coordinates are used to express the gravity vector.
/// The World frame is used as a default if no frame is specified.
/// \param[in] _inCoordinatesOf Frame whose coordinates are used
/// to express _gravity.
/// \return Gravity vector in corrdinates of _inCoordinatesOf.
public: LinearVectorType GetGravity(
const FrameID &_forceInCoordinatesOf = FrameID::World()) const;
};

/// \private The implementation API for the gravity.
public: template <typename PolicyT>
class Implementation : public virtual Feature::Implementation<PolicyT>
{
public: using LinearVectorType =
typename FromPolicy<PolicyT>::template Use<LinearVector>;

/// \brief Implementation API for setting the gravity vector, which is
/// expressed in the World frame..
/// \param[in] _id Identity of the world.
/// \param[in] _gravity Name of gravity.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: "Name"->"Value"?

Copy link
Member Author

Choose a reason for hiding this comment

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

public: virtual void SetWorldGravity(
const Identity &_id, const LinearVectorType &_gravity) = 0;

/// \brief Implementation API for getting the gravity expressed in the
/// world frame.
/// \param[in] _id Identity of the world.
/// \return Name of gravity.
public: virtual LinearVectorType GetWorldGravity(
const Identity &_id) const = 0;
};
};

/////////////////////////////////////////////////
class IGNITION_PHYSICS_VISIBLE Solver : public virtual Feature
{
Expand All @@ -83,7 +159,7 @@ namespace ignition
{
/// \brief Implementation API for setting the solver.
/// \param[in] _id Identity of the world.
/// \param[in] _collisionDetector Name of solver.
/// \param[in] _solver Name of solver.
public: virtual void SetWorldSolver(
const Identity &_id, const std::string &_solver) = 0;

Expand Down
66 changes: 66 additions & 0 deletions include/ignition/physics/detail/World.hh
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,72 @@ const std::string &CollisionDetector::World<PolicyT, FeaturesT>::
->GetWorldCollisionDetector(this->identity);
}

/////////////////////////////////////////////////
template <typename PolicyT, typename FeaturesT>
void Gravity::World<PolicyT, FeaturesT>::SetGravity(
const RelativeGravityType &_gravity)
{
// Resolve to world coordinates
auto &impl = *this->template Interface<FrameSemantics>();
auto gravityInWorld =
detail::Resolve(impl, _gravity, FrameID::World(), FrameID::World());

this->template Interface<Gravity>()
->SetWorldGravity(this->identity, gravityInWorld);
}

/////////////////////////////////////////////////
template <typename PolicyT, typename FeaturesT>
void Gravity::World<PolicyT, FeaturesT>::SetGravity(
const LinearVectorType &_gravity,
const FrameID &_inCoordinatesOf)
{
// Call SetWorldGravity directly if using world coordinates
if (_inCoordinatesOf == FrameID::World())
{
this->template Interface<Gravity>()
->SetWorldGravity(this->identity, _gravity);
}
// Otherwise make a RelativeGravity object and call the other API
else
{
RelativeGravityType gravityInRef(_inCoordinatesOf, _gravity);
this->SetGravity(gravityInRef);
}
}

/////////////////////////////////////////////////
template <typename PolicyT, typename FeaturesT>
auto Gravity::World<PolicyT, FeaturesT>::GetGravity() const
-> RelativeGravityType
{
return RelativeGravityType(
FrameID::World(),
this->template Interface<Gravity>()
->GetWorldGravity(this->identity));
}

/////////////////////////////////////////////////
template <typename PolicyT, typename FeaturesT>
auto Gravity::World<PolicyT, FeaturesT>::GetGravity(
const FrameID &_inCoordinatesOf) const
-> LinearVectorType
{
// Return quickly if using world coordinates
auto gravityInWorld = this->template Interface<Gravity>()
->GetWorldGravity(this->identity);
if (_inCoordinatesOf == FrameID::World())
{
return gravityInWorld;
}

// Otherwise resolve to proper frame
auto &impl = *this->template Interface<FrameSemantics>();
RelativeGravityType gravityInRef(FrameID::World(), gravityInWorld);
return detail::Resolve(impl, gravityInRef, FrameID::World(),
_inCoordinatesOf);
}

/////////////////////////////////////////////////
template <typename PolicyT, typename FeaturesT>
void Solver::World<PolicyT, FeaturesT>::SetSolver(
Expand Down