jenswaelkens
Posts: 12
Joined: Thu Sep 24, 2015 12:33 pm

[newbie] advice needed on code

Thu Sep 24, 2015 12:51 pm

Dear all,

I'm a beginner concerning the RPi and python-coding. I succeeded in writing a small program which allows me to control the light intensity produced by three LEDs (RGB) using PWM. However I'm quite sure my code is far from optimal, I feel the repetition of similar code for each colour could be avoided, but I don't know how to do that, let alone what's the best way to do it. Therefore I'd really appreciate input from experienced programmers for transforming my code into better code.

kind regards,
Jens
Here is my code:

Code: Select all

#!/usr/bin/env python
import Tkinter as tk
import time
import RPi.GPIO as gpio
#hardware : connect 3 leds with series resistors:
#BCM-pin 7 (CE1) on/off led; control with buttons
#BCM-pin 8 (CE0) red-led with pwm dimming and frequency control via sliders
#BCM-pin 18 blue-led with pwm dimming and frequency control via sliders
gpio.setmode(gpio.BCM)
#gpio.setwarnings(False)
output_on_off_led_pin=7
output_pwm_led_blue=18
output_pwm_led_red=8
output_pwm_led_green=25
gpio.setup(output_on_off_led_pin,gpio.OUT)
gpio.setup(output_pwm_led_blue,gpio.OUT)
gpio.setup(output_pwm_led_red,gpio.OUT)
gpio.setup(output_pwm_led_green,gpio.OUT)
top=tk.Tk()
top.geometry("800x400+210+290")
top.title("demo van tkinter")
dutycycle_var_blue=tk.DoubleVar()
dutycycle_var_red=tk.DoubleVar()
dutycycle_var_green=tk.DoubleVar()
freq_var_blue=tk.DoubleVar()
freq_var_red=tk.DoubleVar()
freq_var_green=tk.DoubleVar()
i=0
p_blue=gpio.PWM(output_pwm_led_blue,1)
p_blue.start(50)
p_red=gpio.PWM(output_pwm_led_red,1)
p_red.start(50)
p_green=gpio.PWM(output_pwm_led_green,1)
p_green.start(50)
def btn_on_cmd():
        text3.configure(bg = "#00FF00")
        text3.delete(0.1,tk.END)
        text3.insert("0.1","ON")
        gpio.output(output_on_off_led_pin,True)
def btn_off_cmd():
        text3.configure(bg="#FF4000")
        text3.delete(0.1,tk.END)
        text3.insert("0.1","OFF")       
        gpio.output(7,False)
def set_dc_blue(dutycycle_var_blue):
        DC=float(dutycycle_var_blue)
        p_blue.ChangeDutyCycle(DC)
def set_dc_red(dutycycle_var_red):
    DC=float(dutycycle_var_red)
    p_red.ChangeDutyCycle(DC)
def set_dc_green(dutycycle_var_green):
    DC=float(dutycycle_var_green)
    p_green.ChangeDutyCycle(DC)
def set_freq_blue(freq_var_blue):

User avatar
davef21370
Posts: 897
Joined: Fri Sep 21, 2012 4:13 pm
Location: Earth But Not Grounded

Re: [newbie] advice needed on code

Fri Sep 25, 2015 4:34 pm

You seem to have a good chunk of code missing but I get the general idea. Have a look into creating a class to store the variables and functions for controlling a single LED then create an instance of the class for each individual LED. Some people frown on OOPS but in this case I think it's the way to go as the LED's are actual objects.

Dave.
Apple say... Monkey do !!

scotty101
Posts: 3777
Joined: Fri Jun 08, 2012 6:03 pm

Re: [newbie] advice needed on code

Sat Sep 26, 2015 10:38 am

I didn't get a chance to write anything but have a look at the code provided at the below link. It creates all class for each GPIO pin and the UI that controls it.

viewtopic.php?f=32&t=41639
Electronic and Computer Engineer
Pi Interests: Home Automation, IOT, Python and Tkinter

User avatar
RogerW
Posts: 286
Joined: Sat Dec 20, 2014 12:15 pm
Location: London UK

Re: [newbie] advice needed on code

Sat Sep 26, 2015 2:29 pm

You could try this.

Code: Select all

# led.py
# written by Roger Woollett

import RPi.GPIO as gp
import Tkinter as tk

# class for a non dimmable LED
class Switch:
	def __init__(self,pin):
		# save the pin number
		self.pin = pin
		
		gp.setup(pin,gp.OUT)
		
		# initialise to off
		self.off()
		
	def on(self):
		gp.output(self.pin,gp.HIGH)
		self.on_state = True
		
	def off(self):
		gp.output(self.pin,gp.LOW)
		self.on_state = False
		
	def flip(self):
		if self.on_state:
			self.off()
		else:
			self.on()

# class for PWM faders
class Fader:
	def __init__(self,pin):
		
		gp.setup(pin,gp.OUT)
		
		# set frequency to 1kHz
		self.pwm = gp.PWM(pin,1000)
		# initially off
		self.pwm.start(0.0)
		
	# set the level(0.0 to 100.0)
	def set(self,value):
		self.pwm.ChangeDutyCycle(value)
		
class FrameWindow(tk.Frame):
	def __init__(self,master,*args,**kwargs):
		tk.Frame.__init__(self,master,*args,**kwargs)
		
		# trap user hitting exit button
		self.bind('<Destroy>',self.on_destroy)
		# set to use Board pin numbering
		gp.setmode(gp.BCM)
		
		# create a Switch object
		self.on_off = Switch(14)
		self.green = Fader(15)
		self.yellow = Fader(18)
		self.red = Fader(23)
		
		# create a button co control the on/off LED
		tk.Button(self,text = 'On/Off',command = self.on_switch).grid(row = 0,column = 0)
		
		# create sliders to control the dimmable LEDs
		tk.Scale(self,length = 100,troughcolor = 'green',
				 command = self.on_green).grid(row = 0,column = 1)
		tk.Scale(self,length = 100,troughcolor = 'yellow',
				 command = self.on_yellow).grid(row = 0,column = 2)
		tk.Scale(self,length = 100,troughcolor = 'red',
				 command = self.on_red).grid(row = 0,column = 3)

	# called when the on/off button is pressed 
	def on_switch(self):
		self.on_off.flip()
	
	# These functions are called when the sliders move	
	def on_green(self,value):
		self.green.set(float(value))
		
	def on_yellow(self,value):
		self.yellow.set(float(value))
		
	def on_red(self,value):
		self.red.set(float(value))
		
	# clean up when user presses exit (X) button
	def on_destroy(self,x):
		gp.cleanup()

class App(tk.Tk):
	def __init__(self):
		tk.Tk.__init__(self)
			
		self.title('LED program')
		
		# create and pack a frame window
		FrameWindow(self).pack()
		
# create an App object and execute main loop
App().mainloop()		
I favour using classes rather more than many people so you may want to use a different style. All LEDs have the + leg connected to a GPIO and the - leg connected to ground via a resistor (220 ohm).
I used GPIO14 for the switched LED, 15 for green, 18 for yellow and 23 for red. These are the only coloured LEDs I have.
I used python 2 - to use python 3 change Tkinter to tkinter.
To use the GPIOs you have to have sudo priviledge. One easy way to do this is to open a command window and type "sudo idle". Then you can edit/run as normal but as sudo.
Come back if you have more questions.

jenswaelkens
Posts: 12
Joined: Thu Sep 24, 2015 12:33 pm

Re: [newbie] advice needed on code

Mon Sep 28, 2015 1:47 pm

davef21370 wrote:You seem to have a good chunk of code missing but I get the general idea. Have a look into creating a class to store the variables and functions for controlling a single LED then create an instance of the class for each individual LED. Some people frown on OOPS but in this case I think it's the way to go as the LED's are actual objects.

Dave.
Dear Dave,
My apologies, you are right the code was incomplete, here is the complete code:
#!/usr/bin/env python
#in plaats van onderstaande lijn die weliswaar toelaat om her en
#der het voorvoegsel tk. weg te laten is het beter de import
#te doen zoals twee lijnen lager en de tk. expliciet te schrijven
#from Tkinter import *
import Tkinter as tk
import time
import RPi.GPIO as gpio
#hardware : connect 3 leds with series resistors:
#BCM-pin 7 (CE1) on/off led; control with buttons
#BCM-pin 8 (CE0) red-led with pwm dimming and frequency control via sliders
#BCM-pin 18 blue-led with pwm dimming and frequency control via sliders
gpio.setmode(gpio.BCM)
#gpio.setwarnings(False)
output_on_off_led_pin=7
output_pwm_led_blue=18
output_pwm_led_red=8
output_pwm_led_green=25
gpio.setup(output_on_off_led_pin,gpio.OUT)
gpio.setup(output_pwm_led_blue,gpio.OUT)
gpio.setup(output_pwm_led_red,gpio.OUT)
gpio.setup(output_pwm_led_green,gpio.OUT)
top=tk.Tk()
top.geometry("800x400+210+290")
top.title("demo van tkinter")
dutycycle_var_blue=tk.DoubleVar()
dutycycle_var_red=tk.DoubleVar()
dutycycle_var_green=tk.DoubleVar()
freq_var_blue=tk.DoubleVar()
freq_var_red=tk.DoubleVar()
freq_var_green=tk.DoubleVar()
i=0
p_blue=gpio.PWM(output_pwm_led_blue,1)
p_blue.start(50)
p_red=gpio.PWM(output_pwm_led_red,1)
p_red.start(50)
p_green=gpio.PWM(output_pwm_led_green,1)
p_green.start(50)
def btn_on_cmd():
text3.configure(bg = "#00FF00")
text3.delete(0.1,tk.END)
text3.insert("0.1","ON")
gpio.output(output_on_off_led_pin,True)
def btn_off_cmd():
text3.configure(bg="#FF4000")
text3.delete(0.1,tk.END)
text3.insert("0.1","OFF")
gpio.output(7,False)
def set_dc_blue(dutycycle_var_blue):
DC=float(dutycycle_var_blue)
p_blue.ChangeDutyCycle(DC)
def set_dc_red(dutycycle_var_red):
DC=float(dutycycle_var_red)
p_red.ChangeDutyCycle(DC)
def set_dc_green(dutycycle_var_green):
DC=float(dutycycle_var_green)
p_green.ChangeDutyCycle(DC)
def set_freq_blue(freq_var_blue):
FR = float(freq_var_blue)
p_blue.ChangeFrequency(FR)
def set_freq_red(freq_var_red):
FR = float(freq_var_red)
p_red.ChangeFrequency(FR)
def set_freq_green(freq_var_green):
FR = float(freq_var_green)
p_green.ChangeFrequency(FR)
btn_on=tk.Button(top,text ="On",command=btn_on_cmd)
btn_on.place(x=10,y=100)
btn_off=tk.Button(top,text ="Off",command=btn_off_cmd)
btn_off.place(x=100,y=100)
text3=tk.Text(top,bg="red",height=1,width=4)
text3.place(x=60,y=60)
label_freq_green=tk.Label(top,bg="#BFBFBF",height=1,text= "freq green [Hz]")
label_freq_green.place(x=220,y=320)
label_freq_blue=tk.Label(top,bg="#BFBFBF",height=1,text= "freq blue [Hz]")
label_freq_blue.place(x=420,y=320)
label_dc_green=tk.Label(top,bg="#BFBFBF",height=1,text= "d.c. green [%]")
label_dc_green.place(x=320,y=320)
label_dc_blue=tk.Label(top,bg="#BFBFBF",height=1,text= "d.c. blue [%]")
label_dc_blue.place(x=520,y=320)
label_freq_red=tk.Label(top,bg="#BFBFBF",height=1,text="freq red [Hz]")
label_freq_red.place(x=620,y=320)
label_dc_red=tk.Label(top,bg="#BFBFBF",height=1,text="d.c. red [%]")
label_dc_red.place(x=720,y=320)
slider_dc_blue=tk.Scale(top,variable=dutycycle_var_blue,length=300,resolution=1,command=set_dc_blue)
slider_dc_blue.set(50)
slider_dc_blue.place(x=520,y=5)
slider_dc_red=tk.Scale(top,variable=dutycycle_var_red,length=300,resolution=1,command=set_dc_red)
slider_dc_red.place(x=720,y=5)
slider_dc_red.set(50)
slider_dc_green=tk.Scale(top,variable=dutycycle_var_green,length=300,resolution=1,command=set_dc_green)
slider_dc_green.place(x=320,y=5)
slider_dc_green.set(50)
slider_freq_blue=tk.Scale(top,variable=freq_var_blue,length=300,from_=0.1,to=50,resolution=0.1,command=set_freq_blue)
slider_freq_blue.place(x=420,y=5)
slider_freq_blue.set(2)
slider_freq_red=tk.Scale(top,variable=freq_var_red,length=300,from_=0.1,to=50,resolution=0.1,command=set_freq_red)
slider_freq_red.place(x=620,y=5)
slider_freq_red.set(2)
slider_freq_green=tk.Scale(top,variable=freq_var_green,length=300,from_=0.1,to=50,resolution=0.1,command=set_freq_green)
slider_freq_green.place(x=220,y=5)
slider_freq_green.set(2)
#initialise
btn_off_cmd()
#now we enter the main loop
top.mainloop()
gpio.cleanup()

jenswaelkens
Posts: 12
Joined: Thu Sep 24, 2015 12:33 pm

Re: [newbie] advice needed on code

Mon Sep 28, 2015 3:02 pm

RogerW wrote:You could try this.

Code: Select all

# led.py
# written by Roger Woollett

import RPi.GPIO as gp
import Tkinter as tk

# class for a non dimmable LED
class Switch:
	def __init__(self,pin):
		# save the pin number
		self.pin = pin
		
		gp.setup(pin,gp.OUT)
		
		# initialise to off
		self.off()
		
	def on(self):
		gp.output(self.pin,gp.HIGH)
		self.on_state = True
		
	def off(self):
		gp.output(self.pin,gp.LOW)
		self.on_state = False
		
	def flip(self):
		if self.on_state:
			self.off()
		else:
			self.on()

# class for PWM faders
class Fader:
	def __init__(self,pin):
		
		gp.setup(pin,gp.OUT)
		
		# set frequency to 1kHz
		self.pwm = gp.PWM(pin,1000)
		# initially off
		self.pwm.start(0.0)
		
	# set the level(0.0 to 100.0)
	def set(self,value):
		self.pwm.ChangeDutyCycle(value)
		
class FrameWindow(tk.Frame):
	def __init__(self,master,*args,**kwargs):
		tk.Frame.__init__(self,master,*args,**kwargs)
		
		# trap user hitting exit button
		self.bind('<Destroy>',self.on_destroy)
		# set to use Board pin numbering
		gp.setmode(gp.BCM)
		
		# create a Switch object
		self.on_off = Switch(14)
		self.green = Fader(15)
		self.yellow = Fader(18)
		self.red = Fader(23)
		
		# create a button co control the on/off LED
		tk.Button(self,text = 'On/Off',command = self.on_switch).grid(row = 0,column = 0)
		
		# create sliders to control the dimmable LEDs
		tk.Scale(self,length = 100,troughcolor = 'green',
				 command = self.on_green).grid(row = 0,column = 1)
		tk.Scale(self,length = 100,troughcolor = 'yellow',
				 command = self.on_yellow).grid(row = 0,column = 2)
		tk.Scale(self,length = 100,troughcolor = 'red',
				 command = self.on_red).grid(row = 0,column = 3)

	# called when the on/off button is pressed 
	def on_switch(self):
		self.on_off.flip()
	
	# These functions are called when the sliders move	
	def on_green(self,value):
		self.green.set(float(value))
		
	def on_yellow(self,value):
		self.yellow.set(float(value))
		
	def on_red(self,value):
		self.red.set(float(value))
		
	# clean up when user presses exit (X) button
	def on_destroy(self,x):
		gp.cleanup()

class App(tk.Tk):
	def __init__(self):
		tk.Tk.__init__(self)
			
		self.title('LED program')
		
		# create and pack a frame window
		FrameWindow(self).pack()
		
# create an App object and execute main loop
App().mainloop()		
I favour using classes rather more than many people so you may want to use a different style. All LEDs have the + leg connected to a GPIO and the - leg connected to ground via a resistor (220 ohm).
I used GPIO14 for the switched LED, 15 for green, 18 for yellow and 23 for red. These are the only coloured LEDs I have.
I used python 2 - to use python 3 change Tkinter to tkinter.
To use the GPIOs you have to have sudo priviledge. One easy way to do this is to open a command window and type "sudo idle". Then you can edit/run as normal but as sudo.
Come back if you have more questions.
Dear Roger,
Thank you very much for helping me with my problem. As you might have noticed, part of my code was missing and I have posted the complete original code now. Could you show me what exactly I should do in the part:
# create an App object and execute main loop
App().mainloop()

thanks in advance
jens

User avatar
RogerW
Posts: 286
Joined: Sat Dec 20, 2014 12:15 pm
Location: London UK

Re: [newbie] advice needed on code

Mon Sep 28, 2015 3:47 pm

That code should run just as it is.
The line:
App().mainloop()
creates an App object and then calls its mainloop function. It could have been written:
myapp = App()
myapp.mainloop()
but I prefer not to introduce uneccessary variables.

You have used different GPIO pins so you will need to change these lines in FrameWindow.__init__

Code: Select all

     
      # create a Switch object
      self.on_off = Switch(14)
      self.green = Fader(15)
      self.yellow = Fader(18)
      self.red = Fader(23)
 
Let us know how you get on.

jenswaelkens
Posts: 12
Joined: Thu Sep 24, 2015 12:33 pm

Re: [newbie] advice needed on code

Sun Oct 11, 2015 3:42 pm

RogerW wrote:That code should run just as it is.
The line:
App().mainloop()
creates an App object and then calls its mainloop function. It could have been written:
myapp = App()
myapp.mainloop()
but I prefer not to introduce uneccessary variables.

You have used different GPIO pins so you will need to change these lines in FrameWindow.__init__

Code: Select all

     
      # create a Switch object
      self.on_off = Switch(14)
      self.green = Fader(15)
      self.yellow = Fader(18)
      self.red = Fader(23)
 
Let us know how you get on.
Dear Roger,
I tried out your code and it works, I changed the pin numbers and first I thought it didn't work but after adding

Code: Select all

#!/usr/bin/env python
it worked fine, thanks a lot for helping me. I will now try to add the other three faders like in the original code.

kind regards,
Jens
p.s. I still wonder whether an optimization of the original code in a non object oriented way is also possible.

User avatar
RogerW
Posts: 286
Joined: Sat Dec 20, 2014 12:15 pm
Location: London UK

Re: [newbie] advice needed on code

Sun Oct 11, 2015 4:16 pm

jenswaelkens wrote: it worked fine, thanks a lot for helping me. I will now try to add the other three faders like in the original code.

kind regards,
Jens
p.s. I still wonder whether an optimization of the original code in a non object oriented way is also possible.
It is certainly possible to use a non object oriented approach but I think you will find that as the program becomes more complicated the OO approach makes more sense. This is particularly so as you add more instances of objects (in your case faders).

Good luck with your program.

jenswaelkens
Posts: 12
Joined: Thu Sep 24, 2015 12:33 pm

Re: [newbie] advice needed on code

Sun Oct 11, 2015 5:33 pm

RogerW wrote:
jenswaelkens wrote: it worked fine, thanks a lot for helping me. I will now try to add the other three faders like in the original code.

kind regards,
Jens
p.s. I still wonder whether an optimization of the original code in a non object oriented way is also possible.
It is certainly possible to use a non object oriented approach but I think you will find that as the program becomes more complicated the OO approach makes more sense. This is particularly so as you add more instances of objects (in your case faders).

Good luck with your program.
Dear Roger,
I noticed a small problem with your program, pressing x does not seem to quit the program. In fact I'd like it to stop when pressing CTRL-c but that doesn't seem to work neither. Do you have any idea what could cause this behavior?

kind regards,
Jens

User avatar
RogerW
Posts: 286
Joined: Sat Dec 20, 2014 12:15 pm
Location: London UK

Re: [newbie] advice needed on code

Mon Oct 12, 2015 8:34 am

jenswaelkens wrote: Dear Roger,
I noticed a small problem with your program, pressing x does not seem to quit the program. In fact I'd like it to stop when pressing CTRL-c but that doesn't seem to work neither. Do you have any idea what could cause this behavior?

kind regards,
Jens
I have tried to reproduce your problem but it works for me. I have had something similar in the past and it depended on exactly how I started the program.

When you click the X the function on_destroy in FrameWindow should be called. It would be worth putting a print statement in that function to check that it is being called.

If you cannot get it to work post the exact sequence you are using to start the program.

Return to “Python”