nobleo / path_tracking_pid

Path Tracking PID offers a tuneable PID control loop, decouling steering and forward velocity
Apache License 2.0
129 stars 38 forks source link

Documentation of `Controller::distToSegmentSquared()` incorrect #128

Closed lewie-donckers closed 2 years ago

lewie-donckers commented 2 years ago

While working on unit tests for Controller::distToSegmentSquared() I noticed the following.

The documentation suggests that the pose it returns is the point on the segment PV (start, end) that is closest to W (point). It does not do this.

I had the following test case:

TEST(PathTrackingPidCalculations, ClosestPointOnSegment_CloseToStart)
{
  const auto start = create_transform(2, 2, 0);
  const auto end = create_transform(4, 4, 0);
  const auto point = create_transform(0, 1, 0);
  const auto ref = start;

  EXPECT_EQ(ref, closestPointOnSegment(start, end, point, false));
}

point is closest to start of any point on start-end but the following is returned: [1.759, 2.319, 0].

rokusottervanger commented 2 years ago

Looks like a nice catch to me.

@cesar-lopez-mar is this expected behavior?

lewie-donckers commented 2 years ago

Oops. Might be false alarm. We messed up the documentation and because of that I messed up the order of the arguments in my unit tests. Will fix the documentation and tests and see what happens tomorrow.

cesar-lopez-mar commented 2 years ago

Indeed we had a discussion with Lewie and the segment is defined by V-W (not P-V)

rokusottervanger commented 2 years ago

Ah, alright. With that in mind, I did the math by hand for this test case. It should be exactly [1.760, 2.320, 0], so that checks out.

lewie-donckers commented 2 years ago

It's just a documentation "bug". I fixed it in PR #133 .

Also updated the title to better reflect this.