On Apr 12, 2016, at 00:52, Carolina Hern��ndez <carola.hp@gmail.com> wrote:

Hi Krish
Thanks for all your comments.

I agree with Krish that the code is well done, plus the documentation is very complete, I like that a lot!

About the #initialize method, I think you are right, and some code refactoring would solve it.

Yes, I would move both block definitions to their separate methods, that should make the code more readable. 

Also, about the #takeOff method, maybe the comments are not so explicative, but it just sends the take off command until the drone is actually flying. In practice, it sends it twice and then stops. The logic for the land command is very similar. They are designed that way to ensure the drone will take off or land after only one call of the corresponding method. 

For me at least the comment in takeOff is clear.

Thanks for your advice on avoiding Float and Integer overrides, it would be cleaner to include the methods in the ARDrone package.

Indeed, I think it would be better to have these on ARDCommandManager, as it is the only place they are used and furthermore don���t rely on private state of Float or Integer.

---> Save our in-boxes! http://emailcharter.org <---

Johan Fabry   -   http://pleiad.cl/~jfabry
PLEIAD and RyCh labs  -  Computer Science Department (DCC)  -  University of Chile