Building thermostat state machine in Python

Question

I used a Raspberry Pi and Adafruit LCD with buttons to build a small thermostat for my house.

This is the hardware I used to you can imagine how the button and the code work: https://www.adafruit.com/products/1110

I know I need to build some kind of state machine but I don't know how to handle persistance in python that is why I decided to use filesystem and global variables.

I would be really happy to know what are your suggestions on this topic and what are the biggest errors in my implementation.

This is the script controlling the lcd buttons, I also wrote another that checks the produced text files content and turns on / off heater to fullfill user's requests.

# -*- coding: utf-8 -*-
#!/usr/bin/python
import Adafruit_CharLCD as LCD
import time
import datetime
import subprocess
import urllib2
import json
import services
# Initialize the LCD using the pins
lcd = LCD.Adafruit_CharLCDPlate()
# Make list of button value, text, and backlight color.
buttons = ( (LCD.SELECT, 'Select', (1,1,1)),
 (LCD.LEFT, 'Left' , (1,0,0)),
 (LCD.UP, 'Up' , (0,0,1)),
 (LCD.DOWN, 'Down' , (0,1,0)),
 (LCD.RIGHT, 'Right' , (1,0,1)) )
temp_1_path = '/home/osmc/Scripts/sensor_data/first_floor_temp.txt'
temp_1 = open(temp_1_path)
temp_1 = temp_1.read()
target_temp_1_path = '/home/osmc/Scripts/sensor_data/target_temp_1.txt'
target_temp_1 = open(target_temp_1_path)
target_temp_1 = target_temp_1.read()
thermostate_status_path = '/home/osmc/Scripts/sensor_data/thermostate_status.txt'
thermostate_status = open(thermostate_status_path)
thermostate_status = thermostate_status.read()
backlight_on = True
backlight_turned_on_at = datetime.datetime.now()
timeout_value = datetime.timedelta(seconds=180)
# Write file
def write_file(file_path, value):
 f=open(file_path,'w')
 f.write(str(value))
 f.close();
# Toggle backlight
def toggle_backlight():
 global backlight_on
 global backlight_turned_on_at
 if backlight_on:
 lcd.set_backlight(0.0)
 backlight_on = False
 else:
 backlight_turned_on_at = datetime.datetime.now()
 lcd.set_backlight(0.1)
 backlight_on = True
 time.sleep(0.5)
def timeout_triggered():
 global backlight_on
 lcd.set_backlight(0.0)
 backlight_on = False
# Main Screen
def main_screen(target_temp_1):
 lcd.message("Current Target")
 lcd.set_cursor(0,1)
 lcd.message(" ")
 lcd.set_cursor(0,1)
 lcd.message(str(temp_1) + ' C')
 if thermostate_status == "on":
 lcd.set_cursor(8,1)
 lcd.message("*")
 else:
 lcd.set_cursor(8,1)
 lcd.message(" ")
 lcd.set_cursor(10,1)
 lcd.message(str(target_temp_1) + ' C')
def update_temp_1():
 global temp_1_path
 temp_1 = open(temp_1_path)
 temp_1 = temp_1.read()
 lcd.set_cursor(0,1)
 lcd.message(str(temp_1) + ' C')
def modify_target_temp_1(action):
 global target_temp_1
 if action == "increase":
 target_temp_1 = float(target_temp_1)
 target_temp_1 += 0.5
 else:
 target_temp_1 = float(target_temp_1)
 target_temp_1 -= 0.5
 target_temp_1 = str(target_temp_1)
 lcd.set_cursor(10,1)
 lcd.message(str(target_temp_1) + ' C')
 write_file(target_temp_1_path, target_temp_1)
 time.sleep(0.2)
def modify_thermostate_status(status):
 global id
 if status == "on":
 lcd.set_cursor(8,1)
 lcd.message("*")
 else:
 lcd.set_cursor(8,1)
 lcd.message(" ")
 write_file(thermostate_status_path, status)
print 'Thermostate started'
print 'Press Ctrl-C to quit.'
main_screen(target_temp_1)
while True:
 # Loop through each button and check if it is pressed.
 update_temp_1()
 now = datetime.datetime.now()
 delta = now - backlight_turned_on_at
 if delta > timeout_value:
 timeout_triggered()
 for button in buttons:
 if lcd.is_pressed(button[0]):
 if button[1] == "Select":
 modify_thermostate_status("off")
 elif button[1] == "Up":
 modify_target_temp_1("increase")
 elif button[1] == "Down":
 modify_target_temp_1("decrease")
 elif button[1] == "Right":
 toggle_backlight()
 elif button[1] == "Left":
 modify_thermostate_status("on")

Answer 1

First of all, your code is not very pythonic and fragile. For example, it would be better to read file paths from environment or configuration, so both of your scripts will use the same values.

Then,

x = open(x)
x = x.read()

if very ugly, it can be done in one line and probably behind the function like:

def read_file_contents(filepath):
 return open(filepath).read()

Also, there should be feedback from the actuator part that it really understood the command, and at any rate actuator needs to be notified first, then screen state changed (or there may be some quick "timer" on the screen, if screen update is long process, which I guess is not the case).

That is, do not repeat the code, which does the same thing, in many places: update_temp should also use read_file_contents.

If you know how to do classes in Python, you can create a class for each thermostat, encapsulating it's state. Right now your module is doing that encapsulation and it may be fine as long as it does the job (and you do not plan to support more than one device).

(Also I do not understand why time.sleep is in one function, and not in the loop. Have you forgot it there?)

Communicating via file system is not a problem: in your case it's most reliable and simple way, without much overhead, with default buffering. (I know, not everyone will agree, that is why there are other means of interprocess communication).

What do you mean by needing a state machine? You have already implemented state machine, but it is scattered in the code, and state is in several variables. Also, you are certainly aware, that in your implementation buttons have different priorities. Personally, I do not like that this fact is implicit in the code. I'd read button state at one moment and then continued with interpreting what does it mean if up and down are pressed at the same time.

I've mentioned only some suggestions, which are doable in your current style (remove repetition, do not reuse variables, maybe, put your state into one object or dictionary to feel better).

There are endless possibilities to completely rewrite the code using some "pure" methodology, to make it "perfect". Most important is that your code is readable, can be understood by humans, does the job (plus, in the real world testability also matters).

Also, you have subprocess and some other imports. Not sure why do you need those, so can't say more without seeing how they are used.

Edit: wikipedia has a good article on Automata-based programming. Please, note, that for simple cases using automata-based approach in general-purpose programming language makes the size bigger, not smaller.