Gopro wifi by antonvh · Pull Request #665 · dronekit/dronekit-python

antonvh · 2016-09-14T18:01:53Z

Some of this is also in another PR. What I did:

listener on commands.next so I can get notified when a waypoint is reached
camera class that connects to a GoPro3 over wifi (more cameras to come), and syncs time
shutter() method that engages the shutter and logs a geotag.

…when a waypoint is reached and the drone starts heading for the next one.

…uts from the AUV.

…ro class,

mrpollo · 2016-09-15T17:58:47Z

dronekit/__init__.py

-            self._current_waypoint = m.seq
+            if m.seq != self._current_waypoint:
+                self._current_waypoint = m.seq
+                self.notify_attribute_listeners('commands.next',self._current_waypoint)


can you please add a space between the , self

mrpollo · 2016-09-15T18:04:58Z

dronekit/__init__.py

+
+    The message definition is here: https://pixhawk.ethz.ch/mavlink/#SERVO_OUTPUT_RAW
+
+


can you plz the extra new lines here?, and throughout if there are any other not needed newlines/spaces

mrpollo · 2016-09-15T18:05:53Z

dronekit/__init__.py

+
+    """
+
+    def __init__(self,


this is quite the signature, did you explore any other options to the servo attributes?, perhaps kwargs?

thinking about it kwargs makes sense here, if you don't have all the servo messages in kwargs, you won't be able to set them on the internal attrs of the class, you would then need to create a helper perhaps to get the servo message you are looking for .get(1) or some sort of helper that makes sense (this is just an idea)

mrpollo · 2016-09-15T18:08:06Z

dronekit/__init__.py

+        return "SERVO_OUTPUT_RAW: time_boot_us={},port={},servo1_raw={},servo2_raw={},servo3_raw={},servo4_raw={},servo5_raw={},servo6_raw={},servo7_raw={},servo8_raw={},servo9_raw={},servo10_raw={},servo11_raw={},servo12_raw={},servo13_raw={},servo14_raw={},servo15_raw={},servo16_raw={}".format(
+            self.time_boot_us, self.port, self.servo1_raw, self.servo2_raw, self.servo3_raw, self.servo4_raw,
+            self.servo5_raw, self.servo6_raw, self.servo7_raw, self.servo8_raw, self.servo9_raw, self.servo10_raw,
+            self.servo11_raw, self.servo12_raw, self.servo13_raw, self.servo14_raw, self.servo15_raw, self.servo16_raw)


We have been using the __str__ decorator for debugging purposes on all of our top-level classes, do you think this is the most useful representation of data we can get out of this class?, would you typically expect all of the variables to be set and by reading make something out of it?, what if we are logging this, do you think this is the appropriate way to read it?

mrpollo

Great addition, I would like it if you could make 2 PR's 1 for the servo class and the other for the much needed GoPro class.

I generally agree with your decisions but have some questions and advice I would like you to implement if you would like to continue contributing.

mrpollo · 2016-09-15T18:10:57Z

dronekit/cameras.py

+        self.vehicle = vehicle
+        FORMAT = '%(asctime)s, %(message)s'
+        DATEFORMAT = "%d-%m-%Y, %H:%M:%S"
+        logging.basicConfig(format=FORMAT, datefmt=DATEFORMAT, filename="camera_log.csv", filemode='w', level=logging.INFO)


i would expect the logging to be part of some other utility, or at least to be able to change the file name and formats used by it, what do you think?

mrpollo · 2016-09-15T18:11:55Z

dronekit/cameras.py

+
+    def log_vehicle_state(self, num_picture, cam_message):
+        if self.vehicle:
+            log_msg = ",".join(map(str,[num_picture, self.vehicle.commands.next,


I wonder if we should use the CSV lib for this, did you consider that when exploring this solution?

mrpollo · 2016-09-15T18:14:32Z

dronekit/cameras.py

+            return str(e)
+
+
+class GoProHero3(object):


I wonder if having a base class Camera object would be best here, you could inherit when creating a new camera object, that would allow us to maybe have a external modules, I would prefer that approach so we can standardize behind the same interfaces for cameras, this would allow for greater code re-use between apps.

mrpollo · 2016-09-15T18:15:35Z

dronekit/cameras.py

+        # for a list of http commands to control the GoPro camera
+
+        #TODO: convert these into methods.
+        self.on = "http://10.5.5.9/bacpac/PW?t=" + self.wifipassword + "&p=%01"


can you create a url builder util for this class?, and perhaps add the IP as a variable too

mrpollo · 2016-09-15T18:16:31Z

dronekit/cameras.py

+        self.res5mpMedium = "http://10.5.5.9/camera/PR?t=" + self.wifipassword + "&p=%03"
+        self.noSound = "http://10.5.5.9/camera/BS?t=" + self.wifipassword + "&p=%00"
+        self.sound70 = "http://10.5.5.9/camera/BS?t=" + self.wifipassword + "&p=%01"
+        self.sound100 = "http://10.5.5.9/camera/BS?t=" + self.wifipassword + "&p=%02"


are you planning on adding all of these to the gopro interface? I wonder if we should have a generic command class which is handling all of them, and expose shutter as a top level user of command('shutter')?

mrpollo · 2016-09-15T18:16:51Z

dronekit/cameras.py

+        pass
+
+
+def send_http_cmd(cmd):


this should def live inside the gopro class

mrpollo · 2016-09-15T18:19:43Z

examples/mission_basic/mission_basic.py

@@ -1,216 +1,241 @@
-#!/usr/bin/env python


i think your gopro example needs its own example section, the mission_basic example isn't a good fit since not all the users are using or will use a gopro

mrpollo · 2016-09-15T18:22:10Z

@peterbarker can you do some review of the APM interfaces and overall architecture plz?

@hamishwillee do you have any pointers for documentation?

Anton Vanhoucke added 9 commits September 11, 2016 12:58

Added attribute listener on 'commands.next', so you can get notified …

aeb0f7a

…when a waypoint is reached and the drone starts heading for the next one.

Demonstrate waypoint change callback

6ac775a

Added servo_output_raw attribute and listener to check for servo outp…

053ac6e

…uts from the AUV.

Now a gopto takes a pic at each waypoint. Todo: sync time, create gop…

de0666b

…ro class,

Added GoProHero 3 class and demo in basic mission

5773d2b

Logging geotags

49ab4ca

Moved basic mission back where it belongs

29f8723

fixed log and str error

1559c65

Clean up and log cam errors

476535d

antonvh mentioned this pull request Sep 14, 2016

Camera support? #664

Open

mrpollo reviewed Sep 15, 2016

View reviewed changes

mrpollo requested changes Sep 15, 2016

View reviewed changes


		The message definition is here: https://pixhawk.ethz.ch/mavlink/#SERVO_OUTPUT_RAW

Conversation

antonvh commented Sep 14, 2016

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!

mrpollo 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!

mrpollo commented Sep 15, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants