1. 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 write
os.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?
2. 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)