catszeid/ardrone_control

Control of the ARDrone 2.0 using Leap Motion in ROS Indigo.

ardrone_control's Issues

Code review with muh bros

For this code review, remember that we both want your code to get better. This is constructive criticism, and if you disagree with something, then you can explain yourself. Keep a level head and let's git gud together.

ardrone_control/scripts/leap_control.py

Line 231 in f00e5f8

pass

Silent failure, log something here

ardrone_control/scripts/leap_control.py

Line 94 in f00e5f8

vel_msg.linear.x = 0.0

ardrone_control/scripts/leap_control.py

Line 214 in f00e5f8

vel_msg.linear.x = 0.0

Code reuse, make a function to reset the velocities.

ardrone_control/scripts/leap_control.py

Line 100 in f00e5f8

if (len(hands) > 0):

Here, what I'd do is check if the number of hands is zero, and then continue if it is. This keeps the rest of the function at the base level and easier to read. This might not be the correct thing to do, but take a look at it and defend your reason in a comment :)

ardrone_control/scripts/leap_control.py

Line 130 in f00e5f8

if (first_hand.is_right):

Assign the r_roll_offset_left or l_roll_offset_left to a variable in the if block depending on what hand it is, and then use that variable in the following block. This will reduce code reuse.

ardrone_control/scripts/leap_control.py

Line 164 in f00e5f8

if (first_hand.is_right):

Here too.

ardrone_control/scripts/leap_control.py

Line 169 in f00e5f8

moved = True

See if you can find some way to not set this every time.

ardrone_control/scripts/leap_control.py

Line 212 in f00e5f8

if (not moved):

Do you need to reset this here? Is moved ever not set if you have moved? You might be able to get rid of this block of code and the moved variable if that's the case.

You've got a good start here, and I'd love to see more of this in action.