Time and temperature displaying program for Raspberry Pi

Question

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.
    pylab.savefig('gp.png', facecolor=fig.get_facecolor(),bbox_inches='tight', dpi=80,pad_inches=0.03)
    pil_im = Image.open('gp.png')
    pil_out = pil_im.rotate(-90)
    pil_out.save("gp.png")
    img = pygame.image.load('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()
 ax = fig.add_subplot(111)
  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

Jamal · Accepted Answer · 2014-02-02 07:43:37Z

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

asked	1 year ago
viewed	4446 times
active	5 months ago

current community

your communities

more stack exchange communities

Time and temperature displaying program for Raspberry Pi

1 Answer 1

Your Answer

Not the answer you're looking for? Browse other questions tagged python beginner or ask your own question.

Hot Network Questions

current community

your communities

more stack exchange communities

Time and temperature displaying program for Raspberry Pi

1 Answer 1

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged python beginner or ask your own question.

Related

Hot Network Questions