# Python UDP class to control drone

I'm currently building a small plane with Raspberry Pi Zero W. I wrote a small code which uses UDP to control it.

Here I'm resending packets regularly in a thread because of the joystick input (it gives a lot of values when used and time.sleep() in a pygame event loop does bad things). Am I doing it the right way?

I'm also sending three bytes only per packets (which is enough for the control), but should I use power of 2 packet size? And why?

import socket
import struct
import time

S_TYPE = "BBB"

class Server:
def __init__(self, ip="", port=8080, timeout=-1):
self.ip = ip
self.port = port
self.sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
self.sock.bind((ip, port))
if (timeout > 0):
self.sock.settimeout(timeout)

def recv(self):
data = self.sock.recv(struct.calcsize(S_TYPE))
res = struct.unpack("BBB", data)
return (res)

class Client:
def __init__(self, ip, port=8080, interval=100):
self.ip = ip
self.port = port
self.data = (0, 0, 0)
self.interval = interval
self.sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
self.running = True

def send(self, data_bytes):
self.sock.sendto(data_bytes, (self.ip, self.port))

def send_loop(self):
'''Send loop is used to regulary send data in a thread'''
while (self.running):
self.lock.acquire()
data_bytes = struct.pack(S_TYPE, *self.data)
self.lock.release()
self.send(data_bytes)
time.sleep(self.interval / 1000.0)

def update(self, a, b, c):
'''Update data which is sent'''
self.lock.acquire()
self.data = (a, b, c)
self.lock.release()

def start(self):

def stop(self):
# __del__ wouldn't be called with a thread running
self.running = False


EDIT: do you see any problem not using a lock for self.running?

### Client

In __init__ you should have

    self.running = False


and start should look like this:

def start(self):
if not self.running:
self.running = True


self.lock.acquire()
self.data = (a, b, c)
self.lock.release()


it's safer and more elegant to write

with self.lock:
self.data = (a, b, c)


Parentheses aren't necessary in

while (self.running):


Some notes:

• data as a name has been criticised elsewhere; I would simply say that any variable or property should be self-explanatory within context. a, b, and c should also have more descriptive names.
• You don't need self.ip and self.port in the server - they are only used in the constructor. YAGNI.
• You should default timeout to None and forward it directly to the socket configuration. Making your code have different timeout semantics from the socket library is probably just going to cause confusion.
• Why return (res) instead of return res?
• IP and port of the server might be useful. On the other hand, you can probably obtain them from the bound socket. EDIT OK, you win. Apr 1 '17 at 21:39

You extracted "BBB" into a pseudo-const, which is good. However, you missed one:

res = struct.unpack("BBB", data)


This obviously should've been:

res = struct.unpack(S_TYPE, data)


If you aren't sure whether you replaced them all, Ctrl + F works in pretty much all editors. Often there's even a search and replace :-)

• You could also make it even more obvious and add it to the constructor as it's only used inside the class. Apr 3 '17 at 17:54
• @Dex'ter it is used in two classes
– mou
Apr 3 '17 at 19:17