Added wrapper for the move by command. by AndreasAZiegler · Pull Request #103 · AutonomyLab/bebop_autonomy

AndreasAZiegler · 2017-03-31T13:32:54Z

Added wrapper for the move by command in the Parrot SDK. This command moves the drone to a relative position and rotate heading by a given angle.

…op_autonomy into indigo-devel

mani-monaj

Thank you @AndreasAZiegler for this PR.

Please see my inline comments.
Please consider updating the documentation (in the docs folder) + add your information as a contributor to it.
Please consider adding unit tests for these two functionalities (check bebop_driver/test folder and this part of the docs)

mani-monaj · 2017-06-27T06:45:59Z

bebop_driver/cfg/autogenerated/BebopArdrone3.cfg



-exit(gen.generate(PACKAGE, "bebop_driver_nodelet", "BebopArdrone3"))
+exit(gen.generate(PACKAGE, "bebop_driver_nodelet", "BebopArdrone3"))


Please revert back changes to this file.

mani-monaj · 2017-06-27T06:47:38Z

bebop_driver/include/bebop_driver/autogenerated/ardrone3_state_callbacks.h


 };  // Ardrone3PilotingStateAltitudeChanged

+// Relative move ended.\n Informs about the move that the drone managed to do and why it stopped.


Do you need the following class to be part of this file? Unfortunately this file is auto-generated and will be overwritten by subsequent SDK updates.

No, shouldn't be needed. I guess I was not enough careful while adding files.

mani-monaj · 2017-06-27T06:48:04Z

bebop_driver/scripts/meta/templates/cfg.mustache

 {{/cfg_class}}

-exit(gen.generate(PACKAGE, "bebop_driver_nodelet", "Bebop{{project}}"))
+exit(gen.generate(PACKAGE, "bebop_driver_nodelet", "Bebop{{project}}"))


Please revert back changes to this file.

mani-monaj · 2017-06-27T06:49:47Z

bebop_driver/src/bebop.cpp

 }

+
+void Bebop::MoveBy(const float& dX, const float& dY, const float& dZ, const float& dPsi)


Please change float to double in the signature since the twist message fields (that you pass here) are all doubles.

mani-monaj · 2017-06-27T06:50:24Z

bebop_driver/src/bebop_driver_nodelet.cpp

  }
 }

+void BebopDriverNodelet::CmdMoveByCallback(const geometry_msgs::TwistConstPtr& twist_ptr)


Please lint this function (indents, extra blank lines).

I'm not sure, if I understood your comment correctly. Pleas let me know, if it's the case.

mani-monaj · 2017-06-27T06:51:23Z

bebop_driver/src/bebop_driver_nodelet.cpp

+{
+  try
+  {
+    ROS_INFO("Setting picture format to %f", format_ptr->data);


I don't think %f is correct here, since the input is of type uint8_t. Please fix.

mani-monaj · 2017-06-27T06:52:16Z

bebop_driver/src/bebop_driver_nodelet.cpp

+  try
+  {
+    ROS_INFO("Setting picture format to %f", format_ptr->data);
+    bebop_ptr_->SetPictureFormat(format_ptr->data);


What if format_ptr->data is not set to a value supported by SetPictureFormat? Please add a guard (or a warning) against this.

mani-monaj · 2017-06-27T06:53:09Z

bebop_driver/src/bebop_video_decoder.cpp

                     " x " + boost::lexical_cast<std::string>(codec_ctx_ptr_->width));

-    const uint32_t num_bytes = avpicture_get_size(PIX_FMT_RGB24, codec_ctx_ptr_->width, codec_ctx_ptr_->width);
+    const uint32_t num_bytes = avpicture_get_size(AV_PIX_FMT_RGB24, codec_ctx_ptr_->width, codec_ctx_ptr_->width);


Could you please explain this change?

The current ffmpeg uses AV_PIX_FMT_RGB24. Older versions of ffmpeg provide PIX_FMT_RGB24.

mani-monaj · 2017-06-27T06:53:15Z

bebop_driver/src/bebop_video_decoder.cpp

                     boost::lexical_cast<std::string>(num_bytes));
    ThrowOnCondition(0 == avpicture_fill(
-                       reinterpret_cast<AVPicture*>(frame_rgb_ptr_), frame_rgb_raw_ptr_, PIX_FMT_RGB24,
+                       reinterpret_cast<AVPicture*>(frame_rgb_ptr_), frame_rgb_raw_ptr_, AV_PIX_FMT_RGB24,


mani-monaj · 2017-06-27T06:53:21Z

bebop_driver/src/bebop_video_decoder.cpp


    img_convert_ctx_ptr_ = sws_getContext(codec_ctx_ptr_->width, codec_ctx_ptr_->height, codec_ctx_ptr_->pix_fmt,
-                                          codec_ctx_ptr_->width, codec_ctx_ptr_->height, PIX_FMT_RGB24,
+                                          codec_ctx_ptr_->width, codec_ctx_ptr_->height, AV_PIX_FMT_RGB24,


Relates to AutonomyLab#93 AutonomyLab#107 AutonomyLab#111

…cript." This reverts commit f92e45f.

…efinitions." This reverts commit ffed1bd.

…ple tests.

…movement. Adapted some values. All changes are related the piloting with move by command unit test.

…it test ist now working.

AndreasAZiegler · 2017-07-18T11:35:04Z

I applied your feedback and also added a unit test. As for the usage of the event "RELATIVE MOVE ENDED" arsdk version 3.12.6 is required, it would be good, if you can merge the PR "Add support for SDK 3.12.6 #120" before this one. I and then adapt the unit test accordingly.

AndreasAZiegler added 4 commits March 31, 2017 15:02

Added cmd_move_by command.

8e1a570

Merge branch 'indigo-devel' of https://github.com/AndreasAZiegler/beb…

763c2e1

…op_autonomy into indigo-devel

Removed Arch Linux specific python changes.

23fb05e

Added SetPictureFormat

b7636d6

mani-monaj requested changes Jun 27, 2017

View reviewed changes

mani-monaj and others added 22 commits July 12, 2017 10:27

Add maintainers to readme, docs and manifest (closes AutonomyLab#115)

ecaf843

Remove the Release build flag from the documentation

81d01cc

Relates to AutonomyLab#93 AutonomyLab#107 AutonomyLab#111

Added cmd_move_by command.

6e51598

Removed Arch Linux specific python changes.

ff19906

Reverted changes to autogenerated file.

44353d1

Revert changes in an autogenerated file.

a5dfd32

Reverted changes in a template.

3ddc996

Changed floats to doubles in MoveBy signature.

dc373e7

Removed blank line.

4c990b0

Corrcted type for unit8_t.

20c57aa

Added entry in the contribute list.

1cf2eec

Added documentation for the command move by.

15089f9

Added documentation of the set_picture_format command.

19b5ad0

Adapted entry in the contribute list.

90035f9

Added unit test for move by command.

ee73812

Adapted generate script to use the current (3.12.6) message definitions.

ffed1bd

Updated autogenerated files with the adapted gnerate python script.

f92e45f

Revert "Updated autogenerated files with the adapted gnerate python s…

0aba6dd

…cript." This reverts commit f92e45f.

Revert "Adapted generate script to use the current (3.12.6) message d…

75be564

…efinitions." This reverts commit ffed1bd.

Moved state definitions into test class as they will be used by multi…

035106c

…ple tests.

Removed StopBebop() at some places. Added speed check after end of a …

b93ec8a

…movement. Adapted some values. All changes are related the piloting with move by command unit test.

Adapted values in PilotingWithMoveBy unit test. PilotingWithMoveBy un…

0051e05

…it test ist now working.



		exit(gen.generate(PACKAGE, "bebop_driver_nodelet", "BebopArdrone3")) No newline at end of file
		exit(gen.generate(PACKAGE, "bebop_driver_nodelet", "BebopArdrone3"))


		}; // Ardrone3PilotingStateAltitudeChanged

		// Relative move ended.\n Informs about the move that the drone managed to do and why it stopped.

		}


		void Bebop::MoveBy(const float& dX, const float& dY, const float& dZ, const float& dPsi)

Conversation

AndreasAZiegler commented Mar 31, 2017

Uh oh!

mani-monaj left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndreasAZiegler commented Jul 18, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants