@marioir: Am
Style Guide for Python Code orientiert sich das immer noch nicht.
Die Protokolldatei wird auch immer noch nicht wieder geschlossen. Das ist bei Protokolldateien nicht ganz unwichtig wenn man die ”live” beobachten möchte, denn nur wenn man `flush()` auf einer Datei aufruft die man absichtlich offen lässt, oder eben wenn man die Datei schliesst, ist sichergestellt das auch zu dem Zeitpunkt eventuell intern gepufferte Daten geschrieben und damit ausserhalb des Programms in der Datei sichtbar sind. In Deinem Fall kann es sonst sogar passieren das die Schreibvorgänge nicht in der Reihenfolge in der sie getätigt wurden in der Datei auftauchen. Sirius3 hat die ``with``-Anweisung ja schon erwähnt und auch gezeigt wie man sie verwendet.
Den Namen `file` sollte man nicht neu binden, denn das ist bereits der Name des eingebauten Dateidatentyps. `logfile` ist ein unpassender Name für einen Datei*namen*.
Man sollte bei Namen auch unnötige Abkürzungen vermeiden. `message` ist wesentlich deutlicher als `msg`.
Python hat in der Standardbibliothek übrigens ein `logging`-Modul.
`urllib2` wird importiert, aber nirgends verwendet.
Anstatt `os.system()` sollte man das `subprocess`-Modul verwenden.
`anwesend` und `abwesend` sind keine guten Namen für Funktionen. Funktionen und Methoden werden üblicherweise nach Tätigkeiten benannt weil sie etwas tun und um sie besser von Dingen wie Zuständen oder passiven Werten unterscheiden zu können. Die beiden Funktionen sind auch fast identisch, da sollte man eine Funktion draus machen und diese entsprechend parametrisieren.
Die `counter()`-Funktion würde ich nicht aus der Wartefunktion herausziehen. Jedenfalls nicht unter *dem* Namen. Insbesondere weil die `counter()`-Funktion eigentlich die komplette Wartefunktion enthält, denn die zwei anderen Zeilen in Deiner Wartefunktion kann man da natürlich ausführen, die sind aber redundant wenn die `counter()`-Funktion richtig arbeitet.
Das (Zeit ab)Warten wird in der Funktion an der falschen Stelle gemacht beziehungsweise fehlt ein warten, denn es wird nur gewartet wenn das Mobiltelefon nicht anpingbar war. Im anderen Fall wird ohne Pause gepingt wie blöde. Was der Recher oder das Netz an der Stelle halt hergibt. Das ist Dir wahrscheinlich nur wegen der ersten beiden eigentlich überflüssigen Zeilen in der Wartefunktion nicht aufgefallen, durch die dieses Ping-Flooding erst einsetzt wenn das Telefon das erste mal nicht erreichbar war.
In der `counter()`-Funktion macht der ``else``-Zweig keinen Sinn da der in jedem Fall nach der Schleife ausgeführt wird. Der Inhalt gehört also einfach ganz normal hinter die Schleife. Und das erneute setzen von `x` auf 0 ist sinnlos da die Schleife die `x` als Bedingung prüft an der Stelle nicht mehr durchlaufen wird. Ebenso ist der Aufruf der anderen Wartefunktion unsinnig, denn wenn man an der Stelle stattdessen direkt zum Aufrufer zurückkehrt wird die als nächstes von der ``while``-Schleife im Hauptprogramm ja sowieso aufgerufen. Du denkst an der Stelle viel zu kompliziert und weiter als das innherhalb dieser Funkion nötig ist.
Ebenso überflüssig ist im Hauptprogramm der erste ``if``-Test. Ist der nicht vorhanden wird `warteAufAnwesenheit()` als erstes aufgerufen, was genau den gleichen Effekt hat.
Das schalten des Lichts sollte nicht in den Wartefunktionen passieren, denn das erwartet man beim Namen der Funktion nicht das die ausser warten noch etwas anderes macht.
Ich komme dann ungefähr bei so etwas heraus (ungetestet):
Code: Alles auswählen
#!/usr/bin/env python
from __future__ import absolute_import, division, print_function
import httplib
import subprocess
import time
from contextlib import closing
LOG_FILENAME = 'logfile.log'
MOBILE_DEVICE_IP = '192.168.1.22'
LIGHT_SERVER_IP = '192.168.1.26'
CHECK_INTERVAL = 10 # sekunden
AWAY_CHECK_INTERVAL = 10 # Wartezeit abwesend
AN, AUS = True, False
def log(message):
with open(LOG_FILENAME, "a") as logfile:
logfile.write(
'{0}: {1}\n'.format(time.strftime('%d.%m.%Y %H:%M:%S'), message)
)
def ist_anwesend():
return subprocess.call(['ping', '-c', '1', MOBILE_DEVICE_IP]) == 0
def warte_auf_anwesenheit():
while not ist_anwesend():
time.sleep(CHECK_INTERVAL)
def warte_auf_abwesenheit():
away_count = 0
while away_count < 10:
if not ist_anwesend():
away_count += 1
time.sleep(AWAY_CHECK_INTERVAL)
else:
away_count = 0
time.sleep(CHECK_INTERVAL)
def schalte_licht(state):
with closing(httplib.HTTPConnection(LIGHT_SERVER_IP, 80)) as connection:
connection.connect()
connection.request(
'GET', '/control?key={0}'.format(80 if state else 81)
)
log('Licht {0}geschaltet'.format('ein' if state else 'aus'))
def main():
while True:
warte_auf_anwesenheit()
schalte_licht(AN)
warte_auf_abwesenheit()
schalte_licht(AUS)
if __name__ == '__main__':
main()
Ich persönlich würde die deuschen Bezeichner noch durch Englische ersetzen und das externe `requests`-Modul für die Anfragen an den Webserver verwenden.
