Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Controller::findPositionOnPlan() should be refactored #123

Closed
lewie-donckers opened this issue Mar 7, 2022 · 1 comment · Fixed by #140, #136, #137, #138 or #139
Closed

Controller::findPositionOnPlan() should be refactored #123

lewie-donckers opened this issue Mar 7, 2022 · 1 comment · Fixed by #140, #136, #137, #138 or #139
Assignees
Labels
refactor Refactor request

Comments

@lewie-donckers
Copy link
Contributor

Controller::findPositionOnPlan() should be refactored:

  1. Its name suggests it only finds a position but it also sets member variable distance_to_goal_. It is incorrect (or at least odd) to only change the distance but not any other state (current plan index, etc). I propose we make this function const and do not set any data members. It should just do as the name suggests and find something. If required this value can be made part of the return value. (But perhaps it might be easy to calculate by the caller.)
  2. Parameter path_pose_idx is an output parameter. I propose we change it to be part of the return value.
  3. Parameter controller_state contains way too much state. Only the following members are used:
    • last_visited_pose_index is used as an output parameter. I propose we change it to be part of the return value.
    • current_global_plan_index is used as an in-/output parameter. I propose we split this into a pure input parameter and make the output part of the return value.
@cesar-lopez-mar
Copy link

Seems a good idea!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment