Jean Dupont wrote:
> Dear all,
> I made a simple gui with tkinter. I can imagine there are things which I
> did which are "not optimal". So what I ask is to comment on my code
> preferable with snippets of code which show how to do improve my code.
> #!/usr/bin/env python
> import Tkinter
> import time
> import RPi.GPIO as GPIO
> GPIO.setmode(GPIO.BOARD)
> GPIO.setup(26,GPIO.OUT)
> GPIO.setup(24,GPIO.OUT)
> #hardware : connect 2 leds:
> #board-pin 26 on/off led; control with buttons
> #board-pin 24 led with pwm dimming and frequency; control via sliders
> top = Tkinter.Tk()
> top.geometry("600x400+310+290")
> var1 = DoubleVar()
> var2 = DoubleVar()
> i=0
> p=GPIO.PWM(24,1)
> p.start(50)
> def btn_on_cmd():
> text3.configure(bg = "#00FF00")
> text3.delete(0.1,END)
> text3.insert("0.1","ON ")
> GPIO.output(26,True)
> def btn_off_cmd():
> text3.configure(bg = "#FF4000")
> text3.delete(0.1,END)
> text3.insert("0.1","OFF")
> GPIO.output(26, False)
> def timer0():
> global i
> i=i+1
> text1.delete(0.1,END)
> text1.insert(4.2,"Timer: " + str(i))
> label1.configure(text=time.strftime("%H:%M:%S"))
> top.after(1000, timer0)
> def Set_PWM(var1):
> DC = float(var1)
> p.ChangeDutyCycle(DC)
> def Set_FREQ(var2):
> FR = float(var2)
> p.ChangeFrequency(FR)
> btn_on = Button(top, text ="On", command = btn_on_cmd)
> btn_on.place(x=10,y=100)
> btn_off = Button(top, text ="Off", command = btn_off_cmd)
> btn_off.place(x=100,y=100)
> text1 =Text(top, bg = "#009BFF", font=("Helvetica",14), height = 1, width
> = 15)
> text1.place(x=5,y=5)
> text3=Text(top, bg = "red", font=("Helvetica",12),height = 1, width = 4)
> text3.place(x=60,y=60)
> label1 = Label(top,relief=RAISED,bg =
> "#EFF980",font=("Helvetica",14),height = 1, width = 15)
> label1.place(x=5,y=350)
> label2= Label(top,relief=RAISED,bg =
> "#BFBFBF",font=("Helvetica",10),height = 1, text= "Freq (Hz)")
> label2.place(x=420,y=320)
> label3= Label(top,relief=RAISED,bg =
> "#BFBFBF",font=("Helvetica",10),height = 1, text= "DC %")
> label3.place(x=520,y=320)
> slider1 = Scale(top,variable = var1,length = 300,resolution = 1,command =
> Set_PWM)
> slider1 = Scale(top,variable = var1,length = 300,resolution = 1,command =
> Set_PWM) slider1.place(x=500,y=5)
> slider1.set(50)
> slider2 = Scale(top,variable = var2,length = 300,from_= 0.1, to =
> 50,resolution = 0.1,command = Set_FREQ) slider2.place(x=400,y=5)
> slider2.set(2)
> timer0()
> top.mainloop()
> GPIO.cleanup()
>
>
> thanks in advance
> jean
First and foremost a program has to do what the author wants it to do.
Everything else is secondary. You are likely to have such a program on your
machine, so congrats :)
However, the version you posted does not run, probably because you started
to replace
from Tkinter import *
top = Tk()
...
var1 = DoubleVar()
with the -- better --
import Tkinter
top = Tkinter.Tk()
...
but stopped too early, so that the line
var1 = DoubleVar
will raise a NameError. The fix is mechanical: run the program, go to the
line with the NameError and add the 'Tkinter.' prefix.
Once you have done that you should take the time to find good variable
names. var1? I have no idea what value that could hold until I've read the
whole program. That's feasible here, but program size may grow over time,
and can you still tell me what var1 means next week? I recommend names that
reflect the problem domain, e. g. `var_dutycycle` or just `dutycycle`.
Next you should consider grouping the code by topic -- not necessarily into
functions; having a section that does the setup for the dutycycle data and
widgets and one for the time etc., visually separated by one or two empty
lines should be sufficient.
If you're ambitious you should read up on the grid layout manager. I allows
widget size to change depending on the window size.
The Text widget can be used to write whole Editors (like IDLE) -- it does no
harm here, but seems a bit heavyweight for just an On/Off display. I would
go with a Label or Entry.
What does a red widget with no text mean, by the way? On, off, or undefined?
Personally, I like to start with a defined state. An easy way to achieve
this is to call
button_off_cmd() # or button_on_cmd()
manually before your program enters the mainloop() -- just like you did with
timer0().
PS: An easy way to get an idea of what a script does is to run it. I'd guess
that by keeping the Rasperry-Pi-specific code in you are reducing the number
of readers who can do that by a few orders of magnitude. I managed to get it
to run with the following ad-hoc changes:
$ diff -u raspberry_orig.py raspberry_mock.py
--- raspberry_orig.py 2014-01-17 16:10:20.843334832 +0100
+++ raspberry_mock.py 2014-01-17 16:10:58.970855503 +0100
@@ -1,7 +1,36 @@
#!/usr/bin/env python
import Tkinter
+from Tkinter import *
import time
-import RPi.GPIO as GPIO
+
+try:
+ import RPi.GPIO as GPIO
+except ImportError:
+ class Name(str):
+ def __repr__(self):
+ return self
+
+ class GPIO:
+ def __init__(self, prefix):
+ self.prefix = prefix
+ def __getattr__(self, name):
+ if name in ["BOARD", "OUT"]:
+ return Name(name)
+ if name == "PWM":
+ return GPIO("PWM")
+ if name == "__call__":
+ def c(*args):
+ print "Initializing {}{}".format(self.prefix, args)
+ return self
+ return c
+ def f(*args):
+ print "Calling {}.{}{}".format(self.prefix, name, args)
+ return f
+
+ GPIO = GPIO("GPIO")
+
+
+
GPIO.setmode(GPIO.BOARD)
GPIO.setup(26,GPIO.OUT)
GPIO.setup(24,GPIO.OUT)
--
https://mail.python.org/mailman/listinfo/python-list
