Skip to content

Commit

Permalink
Remove comments
Browse files Browse the repository at this point in the history
Signed-off-by: Mateusz Palczuk <[email protected]>
  • Loading branch information
TauTheLepton committed Dec 6, 2023
1 parent 55cf2e3 commit 24ebe32
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 46 deletions.
31 changes: 2 additions & 29 deletions common/math/geometry/test/test_catmull_rom_spline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,16 +205,6 @@ TEST(CatmullRomSpline, GetCollisionPointIn2D)
}
}

/**
* This test is not passing because the getCollisionPointIn2D function calling getCollisionPointIn2D
* on HermiteCurve, which operates on a normalized lengths. After that the accumulated collision S
* is calculated as a sum of previous HermiteCurves lengths + this HermiteCurve collision S (which
* is calculated relative to the total length of this HermiteCurve).
*
* Possible fix:
* - Add autoscale argument to the HermiteCurve getCollisionPointIn2D function and if it is true
* then multiply S value by total length (like in almost every other member function of this class)
*/
TEST(CatmullRomSpline, getCollisionPointIn2D)
{
const std::vector<geometry_msgs::msg::Point> points{
Expand Down Expand Up @@ -286,11 +276,11 @@ TEST(CatmullRomSpline, getCollisionPointIn2DPolygon)

const auto collision_s0 = spline.getCollisionPointIn2D(polygon);
EXPECT_TRUE(collision_s0);
EXPECT_NEAR(collision_s0.value(), 0.56727227, EPS); // TODO check after fix if value is OK
EXPECT_NEAR(collision_s0.value(), 0.56727227, EPS);

const auto collision_s1 = spline.getCollisionPointIn2D(polygon, true);
EXPECT_TRUE(collision_s1);
EXPECT_NEAR(collision_s1.value(), 0.56727227, EPS); // TODO check after fix if value is OK
EXPECT_NEAR(collision_s1.value(), 0.56727227, EPS);
}

TEST(CatmullRomSpline, getCollisionPointIn2DEmpty)
Expand All @@ -308,10 +298,6 @@ TEST(CatmullRomSpline, getMaximum2DCurvatureLine)
EXPECT_DOUBLE_EQ(spline.getMaximum2DCurvature(), 0.0);
}

/**
* This test fails, because this function chooses maximum value and not the value with greatest
* absolute value. The curvature -10 is much tighter than 2, but here 2 would be the maximum.
*/
TEST(CatmullRomSpline, getMaximum2DCurvatureCurve)
{
const std::vector<geometry_msgs::msg::Point> points{
Expand Down Expand Up @@ -361,22 +347,9 @@ TEST(CatmullRomSpline, getPolygon)
EXPECT_POINT_NEAR(polygon[23], makePoint(4.0, 0.5), EPS);
}

/**
* Statement spline.getPolygon(1, 0) throws this exception:
* /home/aorusadmin/autoware_ss2_test_implementation/src/simulator/scenario_simulator/common/math/geometry/src/spline/catmull_rom_spline.cpp:256: failed to calculate curve index
* This happens because the num_points (= 0) is used in getLeftBounds to calculate step_size (which
* is then equal to infinity). This leads to an infinite `s` value which propagates this way:
* getLeftBounds -> getLeftBoundsPoint -> getNormalVector -> getCurveIndexAndS (which throws this exception)
*
* Possible fix:
* - Catch this exception and throw our own which will have a meaningful description.
* - Add early return in getPolygon when num_points == 0 so that it does not throw.
*/
TEST(CatmullRomSpline, getPolygonEdge)
{
math::geometry::CatmullRomSpline spline = makeCurve();
// FIXME should this throw a propagated error, or catch it and throw its own with a clear message what has happened?
// EXPECT_THROW(spline.getPolygon(1, 0), common::SimulationError);
std::vector<geometry_msgs::msg::Point> polygon0 = spline.getPolygon(1.0, 0);
EXPECT_EQ(polygon0.size(), static_cast<size_t>(0));

Expand Down
17 changes: 0 additions & 17 deletions common/math/geometry/test/test_catmull_rom_subspline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,6 @@ TEST(CatmullRomSubspline, getLength_zero)
EXPECT_DOUBLE_EQ(spline1.getLength(), 0.0);
}

/**
* This test tests function that does not function properly yet. I have opened a github issue about
* the spline collision calculation (which directly affects subspline collision calculation).
*/
TEST(CatmullRomSubspline, getCollisionPointIn2D)
{
const auto spline_ptr = makeLine();
Expand Down Expand Up @@ -88,19 +84,6 @@ TEST(CatmullRomSubspline, getCollisionPointIn2D)
EXPECT_NEAR(ans11.value(), std::hypot(1.25, 3.75), EPS);
}

/**
* This test fails because the subspline logic is incorrect. It obtains the collision with
* the polygon (either first or lst one depending on the search_backward parameter) and then - if
* the collision point is outside of subspline - returns nothing. This leads to incorrect behavior
* when the polygon collides with the spline in more than one point, but some of these points are
* not on the subspline. If the polygon collides with the spline 2 times and the first collision
* point is outside of the subspline, but the second collision point is inside of the subspline and
* the search_backward parameter is set to true, the second (valid) collision is ignored and nothing
* is returned.
* The possible fix for this may be to add function in spline that returns all collision points, not
* only the first one, and in subspline check all of them and choose the first one that is in the
* subspline.
*/
TEST(CatmullRomSubspline, getCollisionPointIn2D_shiftedBeginning)
{
const auto spline_ptr = makeLine();
Expand Down

0 comments on commit 24ebe32

Please sign in to comment.