critic.

[tony1812]

Hello, I am a total Python newbie. This is my Python codes for an imaginary code exercise. Thanks to those who has helped me. I would welcome any costructive critics to help me to be a better python coder, if you don't mind.

The point of the code is to allow the ui to continue display data from sensors smoothly, while the backgrong threads execute to turn 4 realys on and off base on a set user set time with 4 indepent threads, each thread controls a relay. I think this is OK. But since I am new at python, I am sure there must be plenty for improvements. Thanks. :)

import serial, time, threading #random
from tkinter import *
from time import sleep

#Prepare GPIO
import RPi.GPIO as GPIO
GPIO.setmode(GPIO.BOARD)
GPIO.setwarnings(False) #disable annoying warning messages
GPIO.setup(40,GPIO.OUT)
GPIO.setup(38,GPIO.OUT)
GPIO.setup(36,GPIO.OUT)
GPIO.setup(32,GPIO.OUT)
#initially is off
GPIO.output(40,GPIO.LOW)
GPIO.output(38,GPIO.LOW)
GPIO.output(36,GPIO.LOW)
GPIO.output(32,GPIO.LOW)

# create a Relay_1 class
class Relay_1:
def __init__(self, on_duration, off_duration):
self._on_duration = on_duration
self._off_duration = off_duration
self.lock = threading.Lock()

@property
def off_duration(self):
return self._off_duration

@off_duration.setter
def off_duration(self, value):
with self.lock: # BLOCKING
self._off_duration = value
@property
def on_duration(self):
return self._on_duration

@on_duration.setter
def on_duration(self, value):
with self.lock: # BLOCKING
self._on_duration = value

def start(self):
thread = threading.Thread(target=self.run)
thread.setDaemon(True)
thread.start()

def run(self):
while True:
with self.lock: # BLOCKING
on_duration = self.on_duration
off_duration = self.off_duration
relay_on(40)
time.sleep(on_duration)
relay_off(40)
time.sleep(off_duration)
# End Relay_1 class

# create a Relay_2 class
class Relay_2:
def __init__(self, on_duration, off_duration):
self._on_duration = on_duration
self._off_duration = off_duration
self.lock = threading.Lock()

@property
def off_duration(self):
return self._off_duration

@off_duration.setter
def off_duration(self, value):
with self.lock: # BLOCKING
self._off_duration = value
@property
def on_duration(self):
return self._on_duration

@on_duration.setter
def on_duration(self, value):
with self.lock: # BLOCKING
self._on_duration = value

def start(self):
thread = threading.Thread(target=self.run)
thread.setDaemon(True)
thread.start()

def run(self):
while True:
with self.lock: # BLOCKING
on_duration = self.on_duration
off_duration = self.off_duration
relay_on(38)
time.sleep(on_duration)
relay_off(38)
time.sleep(off_duration)
# End Relay_2 class

# create a Relay_3 class
class Relay_3:
def __init__(self, on_duration, off_duration):
self._on_duration = on_duration
self._off_duration = off_duration
self.lock = threading.Lock()

@property
def off_duration(self):
return self._off_duration

@off_duration.setter
def off_duration(self, value):
with self.lock: # BLOCKING
self._off_duration = value
@property
def on_duration(self):
return self._on_duration

@on_duration.setter
def on_duration(self, value):
with self.lock: # BLOCKING
self._on_duration = value

def start(self):
thread = threading.Thread(target=self.run)
thread.setDaemon(True)
thread.start()

def run(self):
while True:
with self.lock: # BLOCKING
on_duration = self.on_duration
off_duration = self.off_duration
relay_on(36)
time.sleep(on_duration)
relay_off(36)
time.sleep(off_duration)
# End Relay_3 class

# create a Relay_4class
class Relay_4:
def __init__(self, on_duration, off_duration):
self._on_duration = on_duration
self._off_duration = off_duration
self.lock = threading.Lock()

@property
def off_duration(self):
return self._off_duration

@off_duration.setter
def off_duration(self, value):
with self.lock: # BLOCKING
self._off_duration = value
@property
def on_duration(self):
return self._on_duration

@on_duration.setter
def on_duration(self, value):
with self.lock: # BLOCKING
self._on_duration = value

def start(self):
thread = threading.Thread(target=self.run)
thread.setDaemon(True)
thread.start()

def run(self):
while True:
with self.lock: # BLOCKING
on_duration = self.on_duration
off_duration = self.off_duration
relay_on(32)
time.sleep(on_duration)
relay_off(32)
time.sleep(off_duration)
# End Relay_4 class


def read_data():
#data = "Random number {}".format(random.randint(1, 99)) #Test dummy data
data = ser.readline()
return data

def relay_on(pin):
GPIO.output(pin,GPIO.HIGH)

def relay_off(pin):
GPIO.output(pin,GPIO.LOW)

def update_value(string_var, ui_window):
data = read_data()
string_var.set(data)
ui_window.after(1000, update_value, string_var, ui_window)

#Slider function
#Relay 1 on/off
def r1_get_on_slider_value(event):
r1.on_duration = r1_on_slider.get()

def r1_get_off_slider_value(val):
r1.off_duration = r1_off_slider.get()

#Relay 2 on/off
def r2_get_on_slider_value(event):
r2.on_duration = r2_on_slider.get()

def r2_get_off_slider_value(val):
r2.off_duration = r2_off_slider.get()

#Relay 3 on/off
def r3_get_on_slider_value(event):
r3.on_duration = r3_on_slider.get()

def r3_get_off_slider_value(val):
r3.off_duration = r3_off_slider.get()

#Relay 4 on/off
def r4_get_on_slider_value(event):
r4.on_duration = r4_on_slider.get()

def r4_get_off_slider_value(val):
r4.off_duration = r4_off_slider.get()

ui = Tk()
ui.geometry("800x400+0+0")
ui.title("Command Center")
ser = serial.Serial('/dev/ttyUSB0', 9600)
var = StringVar()
var.set('Gather Sensor data.')

#slider
#Relay 1
r1_on_slider = Scale(ui, orient = HORIZONTAL, length = 300, width = 15, sliderlength = 60, from_ = 0, to = 100, command = r1_get_on_slider_value)
r1_on_slider.place(x=170, y=5)
r1_on_slider.set(4)
r1_on__label = Label(ui, text = "Relay 1 on duration. Default is 4 seconds")
r1_on__label.place(x=500, y=25)

r1_off_slider = Scale(ui, orient = HORIZONTAL, length = 300, width = 15, sliderlength = 60, from_ = 0, to = 100, command = r1_get_off_slider_value)
r1_off_slider.place(x=170, y=45)
r1_off_slider.set(15)
r1_off__label = Label(ui, text = "Relay 1 off duration. Default is 15 seconds")
r1_off__label.place(x=500, y=65)

# Relay 2
r2_on_slider = Scale(ui, orient = HORIZONTAL, length = 300, width = 15, sliderlength = 60, from_ = 0, to = 100, command = r2_get_on_slider_value)
r2_on_slider.place(x=170, y=85)
r2_on_slider.set(4)
r2_on__label = Label(ui, text = "Relay 2 on duration. Default is 4 seconds")
r2_on__label.place(x=500, y=105)

r2_off_slider = Scale(ui, orient = HORIZONTAL, length = 300, width = 15, sliderlength = 60, from_ = 0, to = 100, command = r2_get_off_slider_value)
r2_off_slider.place(x=170, y=125)
r2_off_slider.set(15)
r2_off__label = Label(ui, text = "Relay 2 off duration. Default is 15 seconds")
r2_off__label.place(x=500, y=145)

#Relay 3
r3_on_slider = Scale(ui, orient = HORIZONTAL, length = 300, width = 15, sliderlength = 60, from_ = 0, to = 100, command = r3_get_on_slider_value)
r3_on_slider.place(x=170, y=165)
r3_on_slider.set(4)
r3_on__label = Label(ui, text = "Relay 3 on duration. Default is 4 seconds")
r3_on__label.place(x=500, y=185)

r3_off_slider = Scale(ui, orient = HORIZONTAL, length = 300, width = 15, sliderlength = 60, from_ = 0, to = 100, command = r3_get_off_slider_value)
r3_off_slider.place(x=170, y=205)
r3_off_slider.set(15)
r3_off__label = Label(ui, text = "Relay 3 off duratio. Default is 15 secondsn")
r3_off__label.place(x=500, y=225)

#Relay 4
r4_on_slider = Scale(ui, orient = HORIZONTAL, length = 300, width = 15, sliderlength = 60, from_ = 0, to = 100, command = r4_get_on_slider_value)
r4_on_slider.place(x=170, y=245)
r4_on_slider.set(4)
r4_on__label = Label(ui, text = "Relay 4 on duration. Default is 4 seconds")
r4_on__label.place(x=500, y=265)

r4_off_slider = Scale(ui, orient = HORIZONTAL, length = 300, width = 15, sliderlength = 60, from_ = 0, to = 100, command = r4_get_off_slider_value)
r4_off_slider.place(x=170, y=285)
r4_off_slider.set(15)
r4_off__label = Label(ui, text = "Relay 4 off duration. Default is 15 seconds")
r4_off__label.place(x=500, y=305)

data_label = Label(ui, textvariable = var, anchor = CENTER)
data_label.place(x=260, y=340)



r1 = Relay_1(1, 1)
r1.start()

r2 = Relay_2(1, 1)
r2.start()

r3 = Relay_3(1, 1)
r3.start()

r4 = Relay_4(1, 1)
r4.start()




ui.after(0, update_value, var, ui)




ui.mainloop()
#END