Python3 newbie

[tony1812]

Hi, I just started learning programming in Python3 on my own. I am wondering if someone be so kind to look at my code see if it make sense?
What this code does is taking sensor data from an Arduino, send data to Pi via serial
to dsplay data to the UI. the UI also has a button or two to control the relays from the Pi. I got the Label to display sensor data now. The button works fine as well. Any suggest to help me write better Pyphon code will be much appreciated.

import serial, time #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 meassages
GPIO.setup(40,GPIO.OUT)
#initial is on
GPIO.output(40,GPIO.HIGH)

#define the rearData function
def readData():
    #data = "Random number {}".format(random.randint(1, 99))
    data = ser.readline()
    #time.sleep(1)
    return data
    
#define toggle buttonfunction
def toggle():
    '''
    use
    t_btn.config('text')[-1]
    to get the present state of the toggle button
    '''
    if t_btn.config('text')[-1] == 'True':
        t_btn.config(text='False')
        GPIO.output(40,GPIO.LOW)
        
    else:
        t_btn.config(text='True')
        GPIO.output(40,GPIO.HIGH)

root = Tk()
root.geometry("800x400+0+0")
ser = serial.Serial('/dev/ttyUSB0', 9600)
var = StringVar()
var.set('hello')

dataLabel = Label(root, textvariable = var)
dataLabel.place(x=300, y=300)
#l.pack()
# Add a toggle button
t_btn = Button(root, text = "On/Off", command=toggle)
t_btn.place(x=380, y=350)

def updateValue(string_var, root_window):
    data = readData()
    string_var.set(data)
    root_window.after(1000, updateValue, string_var, root_window)


while True:
    #sleep(1) # Need this to slow the changes down
    #call readData()
    #data = readData()
    #print (str(data))
    #var.set(data)
    root.after(1000, updateValue, var, root)
    root.mainloop()
    

[nilamo]

Well, if you're looking for improvements, from tkinter import * probably should be something like import tkinter as tk. import * is not a good way to do things. Tutorials do it because it's easy, but it's also lazy and can lead to mistakes that are hard to trace down.

You call root.mainloop() inside of an infinite loop. I'm pretty sure that's something that should only be called once. Since it consumes the main thread, I'm not sure that while True: is needed at all.

[tony1812]

Well, if you're looking for improvements, from tkinter import * probably should be something like import tkinter as tk. import * is not a good way to do things. Tutorials do it because it's easy, but it's also lazy and can lead to mistakes that are hard to trace down.

You call root.mainloop() inside of an infinite loop. I'm pretty sure that's something that should only be called once. Since it consumes the main thread, I'm not sure that while True: is needed at all.

@nilamo Thank you for your reply. I tried what you suggested, replaced from tkinter import * to import tkinter as tk. some how Python does not recognise tk. Also the code below is with while True: taking, you are right, it does not make sense to have two loops. Now, I would like to add a delay on/off feature to the led so as turn led on every 5 seconds then turn of for 15 second. while read dat con-currently, where do I add this without opening a new thread? Thanks.

import serial, time #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 meassages
GPIO.setup(40,GPIO.OUT)
#initial is on
GPIO.output(40,GPIO.HIGH)

#define the rearData function
def readData():
    #data = "Random number {}".format(random.randint(1, 99))
    data = ser.readline()
    #time.sleep(1)
    return data
    
#define toggle buttonfunction
def toggle():
    '''
    use
    t_btn.config('text')[-1]
    to get the present state of the toggle button
    '''
    if t_btn.config('text')[-1] == 'ON':
        t_btn.config(text='OFF')
        GPIO.output(40,GPIO.LOW)
        
    else:
        t_btn.config(text='ON')
        GPIO.output(40,GPIO.HIGH)

root = Tk()
root.geometry("800x400+0+0")
ser = serial.Serial('/dev/ttyUSB0', 9600)
var = StringVar()
var.set('Gether Sensor data.')

dataLabel = Label(root, textvariable = var)
dataLabel.place(x=300, y=300)
#l.pack()
# Add a toggle button
t_btn = Button(root, text = "On/Off", command=toggle)
t_btn.place(x=380, y=350)

def ledOn():
    GPIO.output(40,GPIO.HIGH)
    
def ledOff():
    GPIO.output(40,GPIO.LOW)


def updateValue(string_var, root_window):
    data = readData()
    string_var.set(data)
    
    root_window.after(1000, updateValue, string_var, root_window)

root.after(1000, updateValue, var, root)

root.mainloop()

[sparkz_alot]

some how Python does not recognise tk

It will recognize it, however you need to pre-pend the 'tk' to it's object, for example "Label" becomes "tk.Label" or "Button" becomes "tk.Button" and so on.

[Larz60+]

some additional PEP8 observations

Output:
1. Place all imports on separate lines
import serial
import time
# import random

2. def readData():
   def updateValue(...)

   should not contain capital letters, use under score:

3.
chamge: 
    '''
    use
    t_btn.config('text')[-1]
    to get the present state of the toggle button
    '''
to:
    """
    use
    t_btn.config('text')[-1]
    to get the present state of the toggle button
    """
4. import statement time not used

5. typo in 'meassages': GPIO.setwarnings(False)  # disable annoying warning meassages

these are not show stoppers, rather PEP8 conformance

[tony1812]

Thank you for the corrections. Base on the previous code, I try to add a global var to keep track of the number of time the loop is excuted. But I don't know why the g_loopCount in update_value is never increamented?

Also a dumb question the code comment, is it always better to use double quote rather than single? The tutorials I read seem to use them interchaneably.

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

g_program_start_time = time.time() #new
g_loopCount = 0

#Prepare GPIO
import RPi.GPIO as GPIO
GPIO.setmode(GPIO.BOARD)
GPIO.setwarnings(False) #disable annoying warning meassages
GPIO.setup(40,GPIO.OUT)
#initial is on
#GPIO.output(40,GPIO.HIGH)

#define all functions
def read_data():
    #data = "Random number {}".format(random.randint(1, 99))
    data = ser.readline()
    #time.sleep(1)
    return data
    
def relay_on():
	GPIO.output(40,GPIO.HIGH)
	
def relay_off():
	GPIO.output(40,GPIO.LOW)

def update_value(string_var, root_window):
    g_loop_count = g_loop_count  + 1 # new
    data = read_data()
    string_var.set(data)
    g_loop_end_time = time.time() #new
    
    root_window.after(1000, updateValue, string_var, root_window)
	
#define toggle button function
def toggle():
    """
    use
    t_btn.config('text')[-1]
    to get the present state of the toggle button
    """
    if t_btn.config('text')[-1] == 'ON':
        t_btn.config(text='OFF')
        relay_on()
        #GPIO.output(40,GPIO.LOW)
        
    else:
        t_btn.config(text='ON')
        relay_off()
        #GPIO.output(40,GPIO.HIGH)

root = Tk()
root.geometry("800x400+0+0")
ser = serial.Serial('/dev/ttyUSB0', 9600)
var = String_var()
var.set('Gethar Sensor data.')

dataLabel = Label(root, textvariable = var)
dataLabel.place(x=300, y=300)

g_program_start_time_Label = Label(root, text = "program start at: " + str(hash(g_program_start_time)))
g_program_start_time_Label.place(x=300, y=250)

loop_count_Label = Label(root, text = "loop count is: " + str(g_loop_count))
loop_count_Label.place(x=300, y=275)

# Add a toggle button
t_btn = Button(root, text = "On/Off", command=toggle)
t_btn.place(x=380, y=350)


root.after(1000, update_value, var, root)

root.mainloop()
#END

[Larz60+]

it is declared as

g_loopCount # capital C

but used as:

g_loop_count

underscore and no capital C

[tony1812]

Opps thanks, I missed that, on my script I had coorected it. But this still doesn't change the fact the this goble never gets updated when entering the loop.

[Larz60+]

root.after(1000, update_value, var, root) 

This code gets called once, and only once when you start the application (after 1 sec)

To make this a recurring event, you can use something like: https://stackoverflow.com/questions/1657...ding-timer

[tony1812]

root.after(1000, update_value, var, root) 

This code gets called once, and only once when you start the application (after 1 sec)

To make this a recurring event, you can use something like: https://stackoverflow.com/questions/1657...ding-timer

hello, thanks for your reply.

I am a little confused. you said

root.after(1000, update_value, var, root) 

,"This code gets called once, and only once when you start the application (after 1 sec)." I was under the impression that update_value is in a loop that is called every 1 sec. Thanks.