patternpythonModerate
Automatically turn on Computer from Raspberry Pi (based on user presence in home)
Viewed 0 times
turnuserautomaticallyraspberrycomputerbasedhomefrompresence
Problem
I know the code is full of useless data and stylistic and analysis errors and I'd really love and appreciate to have a pro's idea about it so I can learn to code (and think) better.
import os, time
from ConfigParser import SafeConfigParser
configfilename = "imhome.ini"
scriptpath = os.path.abspath(os.path.dirname(__file__))
configfile = os.path.join(scriptpath, configfilename)
parser = SafeConfigParser()
parser.read(configfile)
ip_smartphone = parser.get('imhome', 'ip_smartphone')
mac_address_pc = parser.get('imhome', 'mac_address_pc')
hometime_path = parser.get('imhome', 'hometime_path')
in_path = parser.get('imhome', 'in_path')
mp3_path = parser.get('imhome', 'mp3_path')
maximum_time_before_logout = parser.get('imhome', 'maximum_time_before_logout')
if os.path.exists(hometime_path) == False:
os.system ("touch " + str(hometime_path))
if os.path.exists(in_path) == True:
os.system ("rm " + str(in_path))
finished = 1
while finished == 1:
check_presence = os.system ("ping -w 1 " + str (ip_smartphone) + " >/dev/null")
file_modification_time = (os.path.getmtime(hometime_path))
file_delta_time = time.time() - file_modification_time
if check_presence == 0 :
os.system ("rm " + str(hometime_path))
change_arrived=("echo " + str(check_presence) + " > hometime")
os.system (change_arrived)
if os.path.exists(in_path) == False:
# You can add any custom commands to execute when home below this row
os.system ("etherwake " + str(mac_address_pc))
os.system ("mpg123 " + str(mp3_path) + " >/dev/null")
os.system ("touch " + str(in_path))
# stop adding commands
elif check_presence == 256 and file_delta_time > int(maximum_time_before_logout):
if os.path.exists(in_path) == True:
os.system ("rm " + str(in_path))Solution
- Comments on your code
-
You make quite a lot of use here of
os.system, which runs a command in a subshell. This can be a risky function to use, because the shell can do anything. For example, you writeos.system ("rm " + str(hometime_path))but what if
hometime_path were -rf *?You're probably safe in this particular program because it's only for your own use and because the strings come from your own configuration file. But it's worth getting into the habit of doing this kind of thing properly, so that in situations which really are risky, you'll know what to do.
In the case of
rm you can use os.unlink:os.unlink(hometime_path)But in the general case, use
subprocess.call and use the -- argument (if necessary) to make sure that the arguments you pass cannot be misinterpreted as options. For example,subprocess.call(['rm', '--', hometime_path])In a case where you want to redirect standard output, like this:
os.system ("ping -w 1 " + str (ip_smartphone) + " >/dev/null")you can use the
stdout keyword argument to subprocess.call:subprocess.call(['ping', '-w', '1', '--', ip_smartphone],
stdout = open('/dev/null', 'w'))-
Most of your calls to
str are unnecessary: these configuration parameters ought to be strings already. (If they are not, I think it you'd prefer to get an exception so that you could fix the problem, rather than silently coercing the incorrect value to a string.)-
You don't need to compare booleans explicitly to
True and False. So instead of:if os.path.exists(in_path) == True:just write:
if os.path.exists(in_path):and instead of:
if os.path.exists(hometime_path) == False:just write:
if not os.path.exists(hometime_path):-
In this code you remove a file only if it exists:
if os.path.exists(in_path) == True:
os.system ("rm " + str(in_path))But the
rm command already has a option for this, so you could write instead:subprocess.call(['rm', '-f', '--', in_path])Or do it in pure Python:
try:
os.unlink(in_path)
except OSError: # No such file or directory
pass(This is an example of how it is often better to ask for forgiveness than permission.)
-
Assuming that
" > hometime" is a mistake for " > " + hometime_path (as suggested in another answer), then in this bit of code:os.system ("rm " + str(hometime_path))
change_arrived=("echo " + str(check_presence) + " > hometime")
os.system (change_arrived)you remove a file and then overwrite it with the value of
check_presence. There's no need to remove the file first: the second command will overwrite it if it exists.And in any case this is easy to do in pure Python:
with open(hometime_path, 'w') as f:
f.write('{}\n'.format(check_presence))-
You use the modification time of the file
hometime_path to remember the last time that your ping command succeeded. Why not remember this time in a variable in Python and avoid having to create and delete and examine this file?-
You use the existence of the file
in_path to remember whether your smartphone is connected. Why not remember this fact in a variable in Python and avoid having to create and delete and examine this file?- Rewrite
Making the above improvements results in something like this (but this is untested, so beware):
import os, subprocess, time
from ConfigParser import SafeConfigParser
from datetime import datetime, timedelta
configfilename = "imhome.ini"
scriptpath = os.path.abspath(os.path.dirname(__file__))
configfile = os.path.join(scriptpath, configfilename)
parser = SafeConfigParser()
parser.read(configfile)
ip_smartphone = parser.get('imhome', 'ip_smartphone')
mac_address_pc = parser.get('imhome', 'mac_address_pc')
mp3_path = parser.get('imhome', 'mp3_path')
timeout = timedelta(seconds = int(parser.get('imhome', 'maximum_time_before_logout')))
devnull = open('/dev/null', 'wb')
connected = False
disconnect_time = None
while True:
ping = subprocess.call(['ping', '-w', '1', '--', ip_smartphone],
stdout = devnull)
if ping == 0:
disconnect_time = datetime.now() + timeout
if not connected:
connected = True
subprocess.call(['etherwake', mac_address_pc])
subprocess.call(['mpg123', mp3_path], stdout = devnull)
elif ping == 256 and connected and datetime.now() > disconnect_time:
connected = False
time.sleep(1)Code Snippets
os.system ("rm " + str(hometime_path))os.unlink(hometime_path)subprocess.call(['rm', '--', hometime_path])os.system ("ping -w 1 " + str (ip_smartphone) + " >/dev/null")subprocess.call(['ping', '-w', '1', '--', ip_smartphone],
stdout = open('/dev/null', 'w'))Context
StackExchange Code Review Q#20646, answer score: 14
Revisions (0)
No revisions yet.