clean code #7802
clean code #7802
Conversation
change a little bit to make the code more readable
| @@ -829,16 +829,6 @@ FixedwingPositionControl::control_position(const math::Vector<2> &curr_pos, cons | |||
| wp_distance_save = 0.0f; | |||
| } | |||
|
|
|||
| // create virtual waypoint which is on the desired flight path but | |||
There was a problem hiding this comment.
Because it's not used anywhere.
There was a problem hiding this comment.
Did you flight test with these changes?
There was a problem hiding this comment.
Thanks @oneWayOut. I was actually thinking of restoring this functionality, but we might as well delete this unused chunk until I get around to it.
There was a problem hiding this comment.
In SITL, the new code works well. I have not do a real flight test yet. @TSC21
There was a problem hiding this comment.
That's definitely good practice, although in this case I think SITL is sufficient.
| @@ -1041,6 +1030,11 @@ FixedwingAttitudeControl::task_main() | |||
| _wheel_ctrl.reset_integrator(); | |||
| } | |||
|
|
|||
| float roll_sp = _att_sp.roll_body; | |||
There was a problem hiding this comment.
Maybe add a line comment saying /* Update attitude setpoint variables */
There was a problem hiding this comment.
As there is no comment before, I just leave it as that. Maybe you can add as you wish.
There was a problem hiding this comment.
It was you that added these variables before at this point so if you don't mind adding this comment, we appreciate.
There was a problem hiding this comment.
do I need to do a new PR? @TSC21
There was a problem hiding this comment.
A new commit. But let it as is, no issue if there's no description.
|
Looks good, I'll merge once CI passes. There's still a lot of dead code in these controllers that make them seem more complicated than they actually are. |
change a little bit to make the code more readable