Python siren control

Question

Below is some code I've put together to contol a siren for a fire service.

It works by webscraping a paging feed and looks for set triggers.

Is there a better way of doing my code or is this "pythonic" enough or can it be improved?

Im python self taught.

"""PYTHON SCRIPT TO CONTROL SIREN 

This script will webscape a paging feed and
in turn activate a siren on set incidents

siren should only sound for 30secconds between 0800 -2000 hrs

this has been made so the siren only goes off for some incidents not all

by s.rees (c) 2016
"""
from scraper import Scraper
import sys
import requests
from clint.textui import puts, colored
import RPi.GPIO as GPIO
from time import sleep
import time

time = datetime.datetime.now()

print "Starting"
print "Monitoring"

#Set GPIO siren is attached too
GPIO.setmode(GPIO.BOARD)
GPIO.setup(7, GPIO.OUT)

def siren ():
 GPIO.output(7, GPIO.HIGH)
 sleep(30)
 GPIO.output(7, GPIO.LOW)

if __name__ == "__main__":
# Load webscraper module
    scraper = Scraper(5, recent_messages=True)
    @scraper.register_handler
    def handler(msg):
# get msg from scraper and make it readable 
        puts(colored.yellow(msg.channel), newline=False)
        puts(" [", newline=False)
        puts(colored.green(msg.datetime), newline=False)
        puts("] ", newline=False)
        if msg.response:
            puts(colored.red(msg.text))
        else:
            puts(msg.text)
# set agency
        if 'MFS' in msg.channel:
#set unit
         if 'Station' in msg.channel:
          print "Brigade Found"
#set response type
                 if 'RESPOND RUBBISH OR WASTE' in msg.text:
                  print "Trigger found"
                  if (time > datetime.time(8) and time < datetime.time(20)):
                    print "Activate Siren"
                    siren ()
                    print 'done'
                    return
                  else:
                    print "out of time restrictions"
                    return
                if 'RESPOND GRASS FIRE' in msg.text:
                  print "Trigger found"
                  if (time > datetime.time(8) and time < datetime.time(20)):
                    print "Activate Siren"
                    siren ()
                    print 'done'
                    return
                  else:
                    print "out of time restrictions"
                    return
            else:
                    print "Brigade found but no trigger"
                    return
            else:
                return
 scraper.run()
 GPIO.cleanup()

Answer 1

Here are some code style and organization notes:

• remove unused imports (sys, requests)
• import time is also not needed since you are immediately shadowing it after by defining the variable time
• missing datetime import
• organize imports per PEP8
• use consistent 4-space indents
• no need for extra space after the function and before the opening parenthesis - e.g. siren() vs siren ()
• let's avoid hardcoding the constant values, like the number 7 and define it in a, say, GPIO_PIN "constant"
• let's move all the initial setup code to under the "main" block of the program
• use print() as a function as opposed to a statement (for Python 3.x compatibility)

Here is the improved initial part of the code without the "scraping" part:

from datetime import datetime
from time import sleep

import RPi.GPIO as GPIO
from clint.textui import puts, colored
from scraper import Scraper


GPIO_PIN = 7


def setup_gpio():
    """Sets GPIO siren is attached too."""
    GPIO.setmode(GPIO.BOARD)
    GPIO.setup(GPIO_PIN, GPIO.OUT)


def siren():
    """Make a siren (flash) on a desired output pin."""
    GPIO.output(GPIO_PIN, GPIO.HIGH)
    sleep(30)
    GPIO.output(GPIO_PIN, GPIO.LOW)


if __name__ == "__main__":
    print("Starting")
    print("Monitoring")

    now = datetime.now()

    setup_gpio()

Note that I've renamed time variable to a more readable now.

---

As far as the rest of the code in the "main" block of the program:

• please see if there is a better way to register a handler for the scraper - instead of defining a decorator, define the handler function before the "main" block and call register_handler() passing in the function:

  scraper.register_handler(handler)
  

• the time comparisons can be improved by using the .hour attribute of a datetime and short-circuiting:

  if 8 < now.hour < 20:
  

• you can omit the last "else" + "return" branch

• the prints around the siren() call should probably be put into the siren() function itself to avoid the code duplication