# Time and temperature displaying program for Raspberry Pi

If possible, can someone please review my code? Especially the use of the Global variables, indentation and comments.

This is my first attempt at Python and it took me a month to come up with this. At lot of time was spent on matplotlib. A lot of answers I got from Stack Overflow.

This program displays, either the time, current temperature, 12hr graph of temps, 24 hr graph of temp or a week’s graph of temps. Selection is based by user input of one of the GPIO pins.

from subprocess import *
import matplotlib
from  numpy import genfromtxt
matplotlib.use('Agg')
import pylab
import Image
import pygame
import os
import time
from time import strftime
#from pygame.locals import*
#from matplotlib.dates import DateFormatter
import RPi.GPIO as GPIO
import datetime

def run_cmd(cmd):
"""Used to run Linux commands"""
p = Popen(cmd, shell=True, stdout=PIPE)
output = p.communicate()[0]
return output

def ckyorn( prompt, help="" ):
"""Used for Y/N prompt"""
a= ""
ok= False
while not ok:
a=raw_input( prompt + " [y,n,?]: " )
if a.upper() in [ 'Y', 'N', 'YES', 'NO' ]:
ok= True
return a.upper()[0]

def delete_old_data():
""" Used to delete old data in input file. Will check for data older than 14 days
and ask if you would like everything older than 7 days to be deleted. This should     not being asked more than once a week"""
now = datetime.datetime.now()
#get the first line of the data file, which will be the oldest entry
getfirstLine = run_cmd("head -1 temperature_logging | awk '{print $2}'") #convert string to time. firstLineTime = datetime.datetime.strptime(getfirstLine, '%m-%d-%Y-%H:%M:%S ') #Get the date and time for seven days ago from now. sevenDays = now - datetime.timedelta(days=7) removeFrom = sevenDays.strftime('%m-%d-%Y-%T') #If the data and time in the first line is older than 14 days, ask if okay to delete. if (now - firstLineTime) > datetime.timedelta(days=14): print ("More than 14 days worth of data has been collected.") var = ckyorn ("Would you like to delete data older than 7 days?") if var == 'Y' : sedCommand = "sed -n -i '/" + removeFrom[1:15] + "/,$p' temperature_logging"
run_cmd(sedCommand)

def displayText(text, size, line, color, clearScreen):

"""Used to display text to the screen. displayText is only configured to display
two lines on the TFT. Only clear screen when writing the first line"""
if clearScreen == True:
screen.fill((0, 0, 0))

font = pygame.font.Font(None, size)
text = font.render(text, 0, color)
textRotated = pygame.transform.rotate(text, -90)
textpos = textRotated.get_rect()
textpos.centery = 80
if line == 1:
textpos.centerx = 90
screen.blit(textRotated,textpos)
elif line == 2:
textpos.centerx = 40
screen.blit(textRotated,textpos)

def displayTime():
"""Used to display date and time on the TFT"""
screen.fill((0, 0, 0))

currentTimeLine1 = strftime("%H:%M:%S", time.localtime())
currentTimeLine2 = strftime("%d  %b", time.localtime())

font = pygame.font.Font(None, 50)
text1 = font.render(currentTimeLine1, 0, (0,250,150))
text2 = font.render(currentTimeLine2, 0, (0,250,150))

Surf1 = pygame.transform.rotate(text1, -90)
Surf2 = pygame.transform.rotate(text2, -90)

screen.blit(Surf1,(60,20))
screen.blit(Surf2,(10,20))

def graph(toKeep):
"""Used to display the graphs. Text will be shown before hand as
the graphs take time to generate"""
global firstTime

#Display some text stating that graphs are being generated
if toKeep == TwelveHours:
displayText('Creating', 35, 1,(200,200,1),True)
displayText('12 Hour Graph', 32, 2,(150,150,255), False)
elif toKeep == TwentyFourHours:
displayText('Creating', 35, 1,(200,200,1), True)
displayText('24 Hour Graph', 32, 2,(150,150,255), False)
elif toKeep == OneWeek:
displayText('Creating', 35, 1,(200,200,1), True)
displayText('One Week Graph', 28, 2,(150,150,255), False)
pygame.display.flip()

#Get temperature and time data from data file
temp = genfromtxt('temperature_logging', dtype=None, usecols=(0), skip_header = lines - toKeep)
timecaptured = genfromtxt('temperature_logging', dtype=None, usecols=(1), skip_header = lines - toKeep)

#Site the size of the font for the axis
for label in ax.get_xticklabels():
label.set_fontsize(8)
for label in ax.get_yticklabels():
label.set_fontsize(8)

#Create xaxis labels and only show every 12th or 96th
xlabels = range(toKeep)
if toKeep == OneWeek:
pylab.xticks(xlabels[::96], [v[:5:] for v in timecaptured[::96]])
else:
pylab.xticks(xlabels[::12], [v[11:16] for v in timecaptured[::12]])

#Plot the graph
pylab.plot(temp,linewidth=2, antialiased=True)

#Change some colours
ax.patch.set_facecolor('#FFFFCC')
ax.patch.set_alpha(0.5)

#Rotate the text in the xaxis
fig.autofmt_xdate(rotation=90)
#Set the yaxsis limits
pylab.ylim(0,40)

#Save the graph as an image and the re-open it, rotate it and then display to TFT.
pil_im = Image.open('gp.png')
pil_out = pil_im.rotate(-90)
pil_out.save("gp.png")
screen.blit(img,(0,0))
pygame.display.flip()

#Clear graph data in preparation for next plot
pylab.cla()

#Reset the firstTime Variable
firstTime = 0

def main():
global ax,fig,screen
global firstTime,lines
global TwentyFourHours,TwelveHours,OneWeek

size = width, height = 128, 160
TFTxSize = 2.28
TFTySize = 1.63
firstTime = True      #Used to work out if a function has already been run
TwentyFourHours = 288
TwelveHours = 144
OneWeek = 2016

whatToDisplay = 1     #What do display on screen
rotate = 2            #Used when automatically rotating the display.
startTime = time.time()   #Used to work out how much time has passed when rotating.

#Set the framebuffer device to be the TFT
os.environ["SDL_FBDEV"] = "/dev/fb1"

#Setup pygame display
pygame.init()
pygame.mouse.set_visible(0)
screen = pygame.display.set_mode(size)

#Setup plot area
fig = pylab.figure()
fig.set_size_inches(TFTxSize, TFTySize)
background = pygame.Surface(screen.get_size())
background = background.convert()

#Setup GPIO
GPIO.setmode(GPIO.BCM)
GPIO.setup(17, GPIO.IN) #Read GPIO 17 as input

#Open data file and count how many lines
file = open('temperature_logging')
lines = sum(1 for word in file if "." in word)
file.seek(0,0)

delete_old_data()

while True:
time.sleep(.5)
"""Check to see if button is pressed, and if so, increment by  one
or reset to one if over 6"""
if GPIO.input(17):
if whatToDisplay < 7:
whatToDisplay = whatToDisplay + 1
firstTime = True
else:
whatToDisplay = 1
firstTime = True
import pdb; pdb.set_trace()
if whatToDisplay == 1:    #Display time
displayTime()
elif whatToDisplay == 2:  #Display last temperature recorded.
file.seek(-26,2)      #Go to end of data file to get temperature
currentTemp = file.readline(5) + ' \xb0C'
displayText('Current Temp', 30, 1, (200,200,1), True )
displayText(currentTemp, 50, 2, (150,150,255), False )
elif  whatToDisplay == 4 and firstTime == True:
graph(TwelveHours)
elif  whatToDisplay  == 5 and firstTime == True:
graph(TwentyFourHours)
elif  whatToDisplay == 6 and firstTime == True:
graph(OneWeek)
elif  whatToDisplay == 3: #Rotate display
elapsedTime = time.time() - startTime
if elapsedTime > 5:
if rotate == 1:
rotate = 2
startTime = time.time()
else:
rotate = 1
startTime = time.time()
if rotate == 1:
displayTime()
elif rotate == 2:
file.seek(-26,2)
currentTemp = file.readline(5) + ' \xb0C'
displayText('Current Temp', 30, 1, (200,200,1), True )
displayText(currentTemp, 50, 2, (150,150,255), False )
#Write to TFT
pygame.display.flip()

if __name__ == '__main__':
main()

-
I see that you have accepted my answer. Don't you want to put a new version of your code here so that one can have a deeper look ? –  Josay Jan 5 '13 at 12:38
I'd love to see the new and improved version too :) –  Pitto Jan 17 '13 at 16:55

Too much to put as a comment, so this is going to be an answer :

As a general comment, you use many variables which are used only once, so they might not really be useful.

ckyorn can be rewritten in a more concise way. Also, its name is weird and one of the parameters is not used.

def ckyorn( prompt):
"""Used for Y/N prompt"""
while True:
a=raw_input( prompt + " [y,n,?]: " ).upper()
if a in [ 'Y', 'N', 'YES', 'NO' ]:
return a[0]


I think it would make sense if this function was to return a boolean instead of a string.

        if a in [ 'Y', 'YES']:
return True
if a in [ 'N', 'No']:
return False


And then, the calling code becomes clearer :

if ckyorn ("Would you like to delete data older than 7 days?"):


You can write if clearScreen: instead of if clearScreen == True:. This applies to different parts of your code.

Make sure you always extract common code from then and else blocks. For instance,

      if whatToDisplay < 7:
whatToDisplay = whatToDisplay + 1
firstTime = True
else:
whatToDisplay = 1
firstTime = True


is exactly the same as :

      if whatToDisplay < 7:
whatToDisplay = whatToDisplay + 1
else:
whatToDisplay = 1
firstTime = True


Please note that what you are doing on whatToDisplay could be done with the modular operator:

      whatToDisplay = 1+(whatToDisplay%7)


Data structures can be used to extract common code. For instance displayTime could be :

def displayTime():
"""Used to display date and time on the TFT"""
screen.fill((0, 0, 0))
font = pygame.font.Font(None, 50)
now=time.localtime()
for setting in [("%H:%M:%S",60),("%d  %b",10)] :
timeformat,dim=setting
currentTimeLine = strftime(timeformat, now)
text = font.render(currentTimeLine, 0, (0,250,150))
Surf = pygame.transform.rotate(text, -90)
screen.blit(Surf,(dim,20))


I haven't looked at it much so far but the global variables seem dodgy to me.

My intuition is that rotation should actually be a boolean as it takes only 2 different values. Then,

      if elapsedTime > 5:
if rotate == 1:
rotate = 2
startTime = time.time()
else:
rotate = 1
startTime = time.time()
if rotate == 1:
displayTime()
elif rotate == 2:
file.seek(-26,2)


would become :

      if elapsedTime > 5:
rotate = not rotate
startTime = time.time()
if rotate:
displayTime()
elif not rotate:
file.seek(-26,2)


Edit After a second look at delete_old_data :

• The variables names are not really good. In particular :

• getfirstline should be firstline
• sevenDays should be something like sevenDaysAgo
• removeFrom should be something like sevenDaysAgoStr
• You probably don't need to call shell command as you could do the same things in pure Python.

• The name of the file (temperature_logging) should appear only once in the code (you store it in a variable). The same applies to gp.png later in the code.
• sevenDays and removeFrom should be defined in the smallest possible scope. In your case, you don't need them until after if var == 'Y'.
• I don't know why you want to take the letters 1 from 15 in removeFrom. Can't you just make sure that removeFrom is formatted just the way you want using strftime?
-
Thank you so much for spending time with looking at my code. Its muchly appreciated. All your points are very much valid… and I have learned a great deal. I had no idea you can use the modulo operator to rotate a number like that…… a lot cleaner than what I was doing! And what you did to Def displayTime(): … again, I had no idea you could do this and it looks much more efficient. I’m going to go away and work on your other points. Thank you very much! –  Mwilliams03 Jan 3 '13 at 21:52
@Josay Your modular-operator-solution for whatToDisplay only does the same thing as the original code for values between 0 and (including) 7. For values > 7, it's totally different. –  us2012 Jan 3 '13 at 22:09
Actually, the point is that it never takes a value > 7. By making sure that this variable is only changed by this single statement, I think it enforces this and makes it clearer (only one branch to consider instead of 2 in the if-else structure). However, I do agree that more generally speaking, the two pieces of code have different effects. –  Josay Jan 3 '13 at 23:07
Also, this could be even clearer if we were to use values in [0,6] instead of values in [1,7] by re-indexing every thing. Then, we would just have : "whatToDisplay = whatToDisplay%7" –  Josay Jan 3 '13 at 23:10