HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Python UDP class to control drone

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
controludpdronepythonclass

Problem

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
import threading

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.thread = threading.Thread(target=self.send_loop)
self.thread.daemon = True
self.lock = threading.Lock()
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):
self.thread.start()

def stop(s

Solution

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?

Context

StackExchange Code Review Q#159553, answer score: 4

Revisions (0)

No revisions yet.