search

nobleo / path_tracking_pid

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

`Controller::distToSegmentSquared()` should be refactored #122

Closed lewie-donckers closed 2 years ago

lewie-donckers commented 2 years ago

I think Controller::distToSegmentSquared() should be refactored in several ways:

  1. Its name suggests it calculates a distance but the main purpose is to determine a pose: the closest pose on segment. I propose we change the name to reflect this.
  2. Beside the pose, it also returns distances but these can easily be calculated by the caller (if required). I propose we remove those distances from the output.
  3. It's currently a const member function but it only uses the data member estimate_pose_angle_enabled_. I propose we replace that with an input parameter.
  4. If the previous point has been done, the function can be extracted (to calculations.h/cpp for example) and unit tests can be added.
  5. To do this, distSquared() from the anonymous namespace in controller.cpp would have to be moved too. I propose we do this and add unit tests for that as well.
rokusottervanger commented 2 years ago

I approve!

cesar-lopez-mar commented 2 years ago

I also agree, it should be more explicit what the function really does.