5
\$\begingroup\$

I'm learning how to use Python with the Raspberry Pi. I successfully followed the tutorial for how to have a Python script run on the Pi to check for new email and turn on an LED if any new messages are in an inbox. I wanted to give myself a challenge by adding a feature to blink a secondary LED to indicate the number of waiting messages. I played around for a while and got a script working, but would like to know if I implemented this correctly.

Am I using threading correctly? Is there an easier way to check a feed every n seconds, and if new messages are present, then continually run blink() until the next feed check?

#!/user/bin/env python

from threading import Thread, Event
from time import sleep
import feedparser
import RPi.GPIO as GPIO

GPIO.setmode(GPIO.BCM)
BLUE_LED = 17
BIG_LED = 23
GPIO.setup(BLUE_LED, GPIO.OUT)
GPIO.setup(BIG_LED, GPIO.OUT)

USERNAME = "user@gmail.com"
PASSWORD = "gzvioCNiAvFvKoqY"

class Repeat(Thread):
    def __init__(self,delay,function,*args,**kwargs):
        Thread.__init__(self)
        self.abort = Event()
        self.delay = delay
        self.args = args
        self.kwargs = kwargs
        self.function = function
    def stop(self):
        self.abort.set()
    def run(self):
        while not self.abort.isSet():
            self.function(*self.args,**self.kwargs)
            self.abort.wait(self.delay)

def blink(count):
    for x in range(0, count):
        print "blink", x + 1
        GPIO.output(BLUE_LED, True)
        sleep(.25)
        GPIO.output(BLUE_LED, False)
        sleep(.25)
    sleep(1) #pause between blinks

try:
    while True:

        print "Starting b"
        b = Repeat(1,blink,0)
        b.start()
        big_led = 0

        print "---> ---> Checking feed"
        messages = int(feedparser.parse("https://" + USERNAME + ":" + PASSWORD + "@mail.google.com/gmail/feed/atom")["feed"]["fullcount"])

        if messages >= 1:
            # turn on big LED
            print "Turning on Big LED"
            GPIO.output(BIG_LED, True)
            big_led = 1
            print "There are", messages, "unread email messages"
            b = Repeat(1,blink,messages)
            b.start()
        else:
            print "There are no new messages"
            if big_led == 1:
                print "Turning off Big LED"
                GPIO.output(BIG_LED, False)

        sleep(60) # check the feed every minute for new mail
        print "Stopping b"
        b.stop()
        b.join()
except (KeyboardInterrupt, SystemExit):
    b.stop()
    b.join()
    GPIO.cleanup()
    print 'Program Stopped Manually!'
    raise
\$\endgroup\$
  • 1
    \$\begingroup\$ I would replace Thread.__init__(self) with the super(Repeat, self).__init__() as it is a better way of calling base functions. \$\endgroup\$ – flakes Apr 15 '14 at 18:18
  • \$\begingroup\$ Also consider breaking up larger lines into smaller portions to keep on track with PEP8's "Limit all lines to a maximum of 79 characters" rule. And use the sting.format function when surrounding strings with more text. ie) "https://{}:{}@...".format(username,password) \$\endgroup\$ – flakes Apr 15 '14 at 18:29
  • \$\begingroup\$ Minor point: the try...except could quite possibly be better a try...finally. \$\endgroup\$ – Veedrac Apr 16 '14 at 15:38
  • \$\begingroup\$ Great suggestions! Thank you. I'm going to try Veedrac's structure below and look up what PEP8 says. I generally like to avoid writing long lines of code. \$\endgroup\$ – backwardm Apr 17 '14 at 22:10
3
\$\begingroup\$

That looks about right. Your thread doesn't do very much, though, so you might want to consider something like so (untested):

def blink(abort, delay, count):
    while not abort.isSet():
        for x in range(0, count):
            print "blink", x + 1
            GPIO.output(BLUE_LED, True)
            sleep(.25)
            GPIO.output(BLUE_LED, False)
            sleep(.25)
        sleep(1) #pause between blinks
        abort.wait(delay)

Then later

thread_stop = Event()
b = threading.Thread.run(target=blink, args=(thread_stop, 1, 0))

which prevents need for the whole class. It's not a suggestion as much as a note that you did more than you needed to.

Note that your blink should really be using abort.wait and should cooperate with the other abort.wait to have sensible timings.

\$\endgroup\$
  • \$\begingroup\$ Thanks for your answer Veedrac. I'll give your suggestion a try. I didn't know about about.wait which seems like exactly what I was going for. Good to know I at least got the threading aspect right—even though it is doing very little. \$\endgroup\$ – backwardm Apr 17 '14 at 22:07
  • \$\begingroup\$ The start parameter of range defaults to zero, so you can write for x in range(count) which is the Python idiom for this scenario. \$\endgroup\$ – Adam Apr 18 '14 at 7:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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