Fix bug limiting collision between half space and ellipse

[SeanCurtis-TRI]

Essentially, if I asked for a collision between (e, hs) it would work, but (hs, e) would not work. This is because the look up table was missing an entry. This adds the missing entry and a supporting unit test.

---

This change is 

[SeanCurtis-TRI]

+@sherm1 for review, please.

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @sherm1)

[sherm1]

Great -- so nice to have a test!
with a few nits and questions

Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @SeanCurtis-TRI)

---

include/fcl/narrowphase/detail/collision_func_matrix-inl.h, line 762 at r1 (raw file):

  collision_matrix[GEOM_HALFSPACE][GEOM_PLANE] = &ShapeShapeCollide<Halfspace<S>, Plane<S>, NarrowPhaseSolver>;
  collision_matrix[GEOM_HALFSPACE][GEOM_HALFSPACE] = &ShapeShapeCollide<Halfspace<S>, Halfspace<S>, NarrowPhaseSolver>;
  collision_matrix[GEOM_HALFSPACE][GEOM_ELLIPSOID] = &ShapeShapeCollide<Halfspace<S>, Ellipsoid<S>, NarrowPhaseSolver>;

nit: to be consistent with the other entries, the halfspace/ellipsoid entry should be placed just after halfspace/sphere.

---

test/narrowphase/detail/test_collision_func_matrix.cpp, line 4 at r1 (raw file):

 * Software License Agreement (BSD License)
 *
 *  Copyright (c) 2018. Toyota Research Institute

nit 2021

---

test/narrowphase/detail/test_collision_func_matrix.cpp, line 90 at r1 (raw file):

/* The collision function matrix defines the function that can be used to
 evaluate collision between two CollisionGeometry instances abased on their

nit abased -> based (or did you mean debased :)

---

test/narrowphase/detail/test_collision_func_matrix.cpp, line 91 at r1 (raw file):

/* The collision function matrix defines the function that can be used to
 evaluate collision between two CollisionGeometry instances abased on their
 node type. In NodeType, there are nine primitive types, eight BV types, and

nit NODE_TYPE

---

test/narrowphase/detail/test_collision_func_matrix.cpp, line 96 at r1 (raw file):

  1 There's a function for all primitive pairs, in both orderings.
  2 There's a function for each (BV, geometry) combination.

BTW but not (geometry, BV), right? Is that worth an explicit mention?

---

test/narrowphase/detail/test_collision_func_matrix.cpp, line 118 at r1 (raw file):

                                BV_KDOP16, BV_KDOP18, BV_KDOP24};

  const CollisionFunctionMatrix<GJKSolver_libccd<double>> matrix_struct;

BTW consider a comment mentioning that the GJKSolver template parameter is a dummy here and not used for anything (assuming that's right).

[SeanCurtis-TRI]

Hopefully fixed the typos in the octomap-dependent code.

Reviewable status: 1 of 3 files reviewed, all discussions resolved (waiting on @sherm1)

---

include/fcl/narrowphase/detail/collision_func_matrix-inl.h, line 762 at r1 (raw file):

Previously, sherm1 (Michael Sherman) wrote…

nit: to be consistent with the other entries, the halfspace/ellipsoid entry should be placed just after halfspace/sphere.

Done

---

test/narrowphase/detail/test_collision_func_matrix.cpp, line 4 at r1 (raw file):

Previously, sherm1 (Michael Sherman) wrote…

nit 2021

Done I got the other year. :)

---

test/narrowphase/detail/test_collision_func_matrix.cpp, line 90 at r1 (raw file):

Previously, sherm1 (Michael Sherman) wrote…

nit abased -> based (or did you mean debased :)

Done

---

test/narrowphase/detail/test_collision_func_matrix.cpp, line 91 at r1 (raw file):

Previously, sherm1 (Michael Sherman) wrote…

nit NODE_TYPE

Done

---

test/narrowphase/detail/test_collision_func_matrix.cpp, line 96 at r1 (raw file):

Previously, sherm1 (Michael Sherman) wrote…

BTW but not (geometry, BV), right? Is that worth an explicit mention?

Done I've made that explicit.

Although, to be perfectly frank, I can't figure out why. After all, they have both ways for GEOM_OCTREE. Go figure.

---

test/narrowphase/detail/test_collision_func_matrix.cpp, line 118 at r1 (raw file):

Previously, sherm1 (Michael Sherman) wrote…

BTW consider a comment mentioning that the GJKSolver template parameter is a dummy here and not used for anything (assuming that's right).

Oops. I actually meant to make this templated on both solvers to determine that they both supported the same matrix. It's why I included headers for both; but then I forgot. Test has been updated.

[sherm1]

Reviewed 3 of 3 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved