remove redundant/incorrect code. #377
Conversation
there is no need for an `initialized_` flag, because the controller manager will call `starting` to indicate the first iteration of a control cycle. The controller already does the correct thing in `starting`. If instead the code in `starting` were removed, and the `initialized_` flag kept, then this position controller would (incorrectly) jump to the previous set point if it is stopped and started (for example, if you hit and release the run-stop), since the `initialized_` flag was only being cleared in the constructor.
|
The history of the file doesn't go back enough to see what the original intentions were with the but if there's a way to see it, I'd be interested. |
|
Thanks @goretkin for your continuing efforts. As such, any changes made will have to be tested on a real robot to ensure that things are running the same. Can you confirm that this is the case? As well, perhaps do a search for the initialize_ parameter. Another class, script, or program might be using it to check if the robot has been initialized. |
|
I've run it on the PR2 I have access to without any changes that I can note, but of course it would be nice for someone else to confirm. In this case the change seems to be relatively benign, but ideally there would be something like unit tests using the gazebo simulator. That's out of my league for now, though! The |
|
replaced by #403 🐈 |
This does not change any behavior, but it removes misleading code.
there is no need for an
initialized_flag, because the controller managerwill call
startingto indicate the first iteration of a control cycle.The controller already does the correct thing in
starting. If instead thecode in
startingwere removed, and theinitialized_flag kept, then thisposition controller would (incorrectly) jump to the previous set point if it is
stopped and started (for example, if you hit and release the run-stop), since
the
initialized_flag was only being cleared in the constructor.