Skip to content

Commit 65968c8

Browse files
WIP Adding testing (and fixing bugs)
InterpMotion has no unit tests at all. This floats on top of pr_476 and adds the skeleton of a unit test. While writing the unit test, I encountered issues: 1. The default constructor (documented to be identity) wasn't. 2. The constructors were incredibly erratic. Some did partial initialization. This commit adds addresses issues as encountered. # Conflicts: # include/fcl/math/motion/interp_motion-inl.h
1 parent fa0921a commit 65968c8

File tree

6 files changed

+130
-39
lines changed

6 files changed

+130
-39
lines changed

include/fcl/math/motion/interp_motion-inl.h

Lines changed: 20 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@
4040

4141
#include "fcl/math/motion/interp_motion.h"
4242

43-
namespace fcl
44-
{
43+
namespace fcl {
4544

4645
//==============================================================================
4746
extern template
@@ -50,46 +49,21 @@ class FCL_EXPORT InterpMotion<double>;
5049
//==============================================================================
5150
template <typename S>
5251
InterpMotion<S>::InterpMotion()
53-
: MotionBase<S>(), angular_axis(Vector3<S>::UnitX())
54-
{
55-
// Default angular velocity is zero
56-
angular_vel = 0;
57-
58-
// Default reference point is local zero point
59-
60-
// Default linear velocity is zero
61-
}
52+
: InterpMotion(Transform3<S>::Identity(), Transform3<S>::Identity(),
53+
Vector3<S>::Zero()) {}
6254

6355
//==============================================================================
6456
template <typename S>
6557
InterpMotion<S>::InterpMotion(
6658
const Matrix3<S>& R1, const Vector3<S>& T1,
6759
const Matrix3<S>& R2, const Vector3<S>& T2)
68-
: MotionBase<S>(),
69-
tf1(Transform3<S>::Identity()),
70-
tf2(Transform3<S>::Identity())
71-
{
72-
tf1.linear() = R1;
73-
tf1.translation() = T1;
74-
75-
tf2.linear() = R2;
76-
tf2.translation() = T2;
77-
78-
tf = tf1;
79-
80-
// Compute the velocities for the motion
81-
computeVelocity();
82-
}
60+
: InterpMotion(R1, T1, R2, T2, Vector3<S>::Zero()) {}
8361

8462
//==============================================================================
8563
template <typename S>
8664
InterpMotion<S>::InterpMotion(
8765
const Transform3<S>& tf1_, const Transform3<S>& tf2_)
88-
: MotionBase<S>(), tf1(tf1_), tf2(tf2_), tf(tf1)
89-
{
90-
// Compute the velocities for the motion
91-
computeVelocity();
92-
}
66+
: InterpMotion<S>(tf1_, tf2_, Vector3<S>::Zero()) {}
9367

9468
//==============================================================================
9569
template <typename S>
@@ -117,13 +91,22 @@ InterpMotion<S>::InterpMotion(
11791
}
11892

11993
//==============================================================================
94+
95+
// Note: In the name of using delegating constructors, we introduce the
96+
// silliness below. We *unpack* the transforms tf1_ and tf2_ and just so that we
97+
// can pack them *back* into the member fields. Eigen forces us to do this
98+
// because we *can't* go the other way; there is no constructor of a Transform3
99+
// based on a Matrix3 and a Vector3. It might be worth creating a utility that
100+
// does that just so this could become more compact. The constructor that this
101+
// delegates to would, instead, do something like:
102+
// InterpMotion(MakeTransform(R1, T1), MakeTransform(R2, T2), O) {}
103+
// And then the core functionality for initializing the class would go into this
104+
// constructor.
120105
template <typename S>
121-
InterpMotion<S>::InterpMotion(
122-
const Transform3<S>& tf1_, const Transform3<S>& tf2_, const Vector3<S>& O)
123-
: MotionBase<S>(), tf1(tf1_), tf2(tf2_), tf(tf1), reference_p(O)
124-
{
125-
// Do nothing
126-
}
106+
InterpMotion<S>::InterpMotion(const Transform3<S>& tf1_,
107+
const Transform3<S>& tf2_, const Vector3<S>& O)
108+
: InterpMotion(tf1_.linear(), tf1_.translation(), tf2_.linear(),
109+
tf2_.translation(), O) {}
127110

128111
//==============================================================================
129112
template <typename S>

include/fcl/math/motion/interp_motion.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ class FCL_EXPORT InterpMotion : public MotionBase<S>
9090
void getTaylorModel(TMatrix3<S>& tm, TVector3<S>& tv) const;
9191

9292
protected:
93-
9493
void computeVelocity();
9594

9695
Quaternion<S> deltaRotation(S dt) const;

test/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ foreach(test ${tests})
9797
add_fcl_test(${test})
9898
endforeach(test)
9999

100+
add_subdirectory(broadphase)
100101
add_subdirectory(geometry)
102+
add_subdirectory(math)
101103
add_subdirectory(narrowphase)
102-
add_subdirectory(broadphase)

test/math/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
add_subdirectory(motion)

test/math/motion/CMakeLists.txt

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
set(tests
2+
test_interp_motion.cpp
3+
)
4+
5+
# Build all the tests
6+
foreach(test ${tests})
7+
add_fcl_test(${test})
8+
endforeach(test)
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/*
2+
* Software License Agreement (BSD License)
3+
*
4+
* Copyright (c) 2020. Toyota Research Institute
5+
* All rights reserved.
6+
*
7+
* Redistribution and use in source and binary forms, with or without
8+
* modification, are permitted provided that the following conditions
9+
* are met:
10+
*
11+
* * Redistributions of source code must retain the above copyright
12+
* notice, this list of conditions and the following disclaimer.
13+
* * Redistributions in binary form must reproduce the above
14+
* copyright notice, this list of conditions and the following
15+
* disclaimer in the documentation and/or other materials provided
16+
* with the distribution.
17+
* * Neither the name of CNRS-LAAS and AIST nor the names of its
18+
* contributors may be used to endorse or promote products derived
19+
* from this software without specific prior written permission.
20+
*
21+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
22+
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
23+
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
24+
* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
25+
* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
26+
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
27+
* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
28+
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
29+
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
30+
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
31+
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
32+
* POSSIBILITY OF SUCH DAMAGE.
33+
*/
34+
35+
/** @author Sean Curtis (sean@tri.global) (2020) */
36+
37+
#include "fcl/math/motion/interp_motion.h"
38+
39+
#include <gtest/gtest.h>
40+
41+
#include "eigen_matrix_compare.h"
42+
43+
namespace fcl {
44+
namespace {
45+
46+
// TODO(SeanCurtis-TRI): Convert this to a parameterized test harness and use
47+
// TEST_P.
48+
49+
template <typename S>
50+
class InterpMotionTest : public ::testing::Test {};
51+
52+
typedef testing::Types<double, float> MyTypes;
53+
TYPED_TEST_CASE(InterpMotionTest, MyTypes);
54+
55+
TYPED_TEST(InterpMotionTest, Construction) {
56+
using S = TypeParam;
57+
const Transform3<S> I = Transform3<S>::Identity();
58+
59+
{
60+
// Default constructor; everything should be identity.
61+
InterpMotion<S> motion{};
62+
63+
Transform3<S> X_FA;
64+
motion.getCurrentTransform(X_FA);
65+
EXPECT_TRUE(CompareMatrices(X_FA.matrix(), I.matrix()));
66+
EXPECT_TRUE(
67+
CompareMatrices(motion.getLinearVelocity(), Vector3<S>::Zero()));
68+
EXPECT_EQ(motion.getAngularVelocity(), S(0));
69+
EXPECT_TRUE(CompareMatrices(motion.getAngularAxis(), Vector3<S>::UnitX()));
70+
EXPECT_TRUE(
71+
CompareMatrices(motion.getReferencePoint(), Vector3<S>::Zero()));
72+
}
73+
74+
{
75+
// Construct from start (R_FS, p_FSo) and goal (R_FG, p_FGo).
76+
}
77+
78+
{
79+
// Construct from start X_FS and goal X_FG.
80+
}
81+
82+
{
83+
// Construct from start (R_FS, p_FSo), goal (R_FG, p_FGo), and an origin of
84+
// rotation (p_FO).
85+
}
86+
87+
{
88+
// Construct from start X_FS, goal X_FG, and rotation origin p_FO.
89+
}
90+
}
91+
92+
} // namespace
93+
} // namespace fcl
94+
95+
//==============================================================================
96+
int main(int argc, char *argv[]) {
97+
::testing::InitGoogleTest(&argc, argv);
98+
return RUN_ALL_TESTS();
99+
}

0 commit comments

Comments
 (0)