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::mpc_based_max_vel() and Controller::update() interaction should be refactored #125

Open
lewie-donckers opened this issue Mar 7, 2022 · 1 comment
Labels
refactor Refactor request

Comments

@lewie-donckers
Copy link
Contributor

Controller::mpc_based_max_vel() repeatedly calls Controller::update() but rolls back some of its side effects. This is a very convoluted and error-prone setup.

update() sets the following member variables:

Before mpc_based_max_vel() calls update() it creates a backup of controller_state_ and at certain points in its body (and just before the end) it overwrites the current state with this backup.

I propose the following:

  • Ideally mpc_based_max_vel() should be a const function that does not modify any state. Based on the name and function documentation, this should be possible.
  • If that's not feasible, update() should be refactored/split in such a way that at the very least no side effects need to be rolled back in mpc_based_max_vel().
@lewie-donckers lewie-donckers added the refactor Refactor request label Mar 7, 2022
@cesar-lopez-mar
Copy link

After our discussion indeed this can be changed by better making a copy of the state and running the simulation (calls to update included) using that copy. Currently we use the member variable in update so that part needs refactor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactor request
Projects
None yet
Development

No branches or pull requests

2 participants