Time and temperature displaying program for Raspberry Pi

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 on user input of one of the GPIO pins.

Please review my code, especially the use of the global variables, indentation and comments.

```
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'

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?

StackExchange Code Review Q#20109, answer score: 11