Stimmen denn die Verbindungsparameter? Und was heisst abstürzen genau? Kann es sein dass man nach dem öffnen der Verbindung eine gewisse Zeit warten muss weil auf der anderen Seite in der ersten halben Sekunde oder so das Programm auf dem Mikrokontroller darauf wartet ob es ein neues Programm flashen soll? (Ist ein beliebtes ”Problem” bei Arduinos).
Muss dieses ständige öffnen und schliessen der Verbindung tatsächlich sein? Und wie reagiert die Gegenseite eigentlich auf das öffnen einer bereits offenen Verbindung denn so wie Du ganz am Anfang das `Serial`-Objekt erstellst wird dort bereits eine Verbindung geöffnet.
Die ganzen Tests ob der die Verbindung offen ist sind sinnfrei denn nach einem `open()`-Aufruf ist sie das auf jeden Fall. Wenn das öffnen nicht klappt wird eine Ausnahme ausgelöst und der Test würde eh nicht ausgeführt.
Das Schliessen der Verbindung würde ich jeweils mindestens in einen ``finally``-Zweig stecken oder die ``with``-Anweisung verwenden um sicherzustellen dass das `close()` auch in jedem Fall aufgerufen wird. Das gleiche gilt für Dateien.
Der GPIO-Import wird nirgends verwendet.
Bei den Namenschreibweisen lohnt sich ein Blick in den
Style Guide for Python Code. Besonders fällt das beim Klassennamen und den Namen der Konstanten auf die man so nicht als solche erkennen kann. Wobei die Klasse in der Form IMHO ein Missbrauch dieses Konstrukts ist.
Hauptprogramm und Variablen haben auf Modulebene nichts zu suchen. Damit schafft man sich undurchsichtige Abhängigkeiten zwischen Funktionen und Programme werden schnell unübersichtlich bis unwartbar. Zudem sollte man Module importieren können ohne das Seiteneffekte auftreten, insbesondere nicht so gravierende wie das Öffnen einer seriellen Verbindung. Normalerweise steckt man dafür das Hauptprogramm in einer `main()`-Funktion die über das folgende Idiom aufgerufen wird:
Das mischen von Deutsch und Englisch in Namen ist unschön.
Bei den Prüfabständen könnte man kommentieren das die in Sekunden angegeben sind.
Semikolons am Ende von Zeilen braucht man in Python nicht. Das Semikolon ist in Python kein Abschluss einer Anweisung sondern ein Trenner zwischen Anweisungen. Da man in der Regel nicht mehr als eine Anweisungen pro Zeile schreibt, braucht man das Semikolon in Python so gut wie nie.
``if``, ``while`` & Co sind keine Funktionen, also sollte man sie auch nicht so schreiben das es ausssieht als wären es Funktionsaufrufe. Nach dem Schlüsselwort kommt ein Leerzeichen und die überflüssigen Klammern um die Bedingung sollte man weglassen.
Es gibt einen Wahrheitstyp (`bool`) mit den Werten `True` und `False` die man verwenden sollte wenn man einen Wahrheitswert ausdrücken möchte. Anstelle von Zahlen.
`time.clock()` ist die falsche Wahl für Zeitmessungen. Das kann je nach System entweder die ”normale” Zeit oder aber die CPU-Zeit seit einem bestimmten Zeitpunkt sein. Und CPU-Zeit bedeutet die tickt nur weiter wenn die CPU für den Prozess arbeiten, wird also mit der Zeit immer mehr nachgehen weil die CPU ja auch für andere Prozesse Zeit verbraucht. `time.time()` wäre hier die richtige Wahl.
Einbuchstabige Namen die über einen Ausdruck hinaus gehen sind fast immer schlecht. Ausnahmen sind wirklich gebräuchliche Namen wie `x` und `y` für Koordinaten oder `i`, `j`, und `k` für ganzzahlige Laufvariablen.
Werte und Zeichenketten mit ``+`` zusammenzusetzen ist eher BASIC als Python. In Python gibt es dafür Zeichenkettenformatierung mittels `format()`-Methode.
Ein ``return`` ohne Wert am Ende einer Funktion macht keinen Sinn, denn am Ende kehrt die Funktion auch ohne zum Aufrufer zurück.
Das verteilen von Kommando senden und Antwort empfangen auf die beiden Funktionen, noch dazu mit schliessen der Verbindung zwischen senden und empfangen, ist keine gute Idee. Wahrscheinlich liegt da auch das Problem. Du sendest einen Befehl an den Mikrokontroller und schliesst die Verbindung über die der dann ja versucht zu antworten. Wenn man die Verbindung in `sendData()` nicht schliesst dann müsste man das in `saveData()` machen, womit man öffnen und schliessen auf zwei verschiedene Funktionen verteilt hätte, was ungünstig ist wenn man sicherstellen will das der Port auf jeden Fall wieder geschlossen wird. Das Empfangen der Daten hat in `saveData()` aber eigentlich sowieso nichts zu suchen, denn der Funktionsname passt dazu gar nicht. Speichern ist etwas anderes als Daten über eine serielle Verbindung zu empfangen *und* zu speichern. Die Kommunikation mit dem Mikrokontroller sollte man in *einer* Funktion abwickeln.
Wenn man die aktuelle Zeit formatiert und das in mehreren Schritten macht, sollte man trotzdem nur *einmal* den Zeitwert ermitteln und nicht in jedem Schritt aufs neue, denn zwischen den Schritten vergeht ja Zeit und am Ende kann es passieren das die Einzelteile der formatierten Zeitangabe zusammen gar nicht die aktuelle Zeit beschreiben, sondern im Extremfall bei dem gezeigten Quelltext fast ein Jahr daneben liegen kann. Auch sollte man für Aufzeichnungen dieser Art nicht die lokale Zeit verwenden sondern UTC. Damit erspart man sich zum Beispiel Probleme bei den Umstellungen zwischen lokaler Sommer- und Winterzeit.
Pfadangaben setzt man mit `os.path.join()` zusammen.
Statt auf Pfade vorher zu testen reagiert man besser auf eventuelle Fehler die beim anlegen entstehen können.
Den Dateimodus 'a+b+' gibt es nicht. Ich sehe auch nicht wofür man hier ein '+' bräuchte denn es werden ja nur Daten angehängt.
Ich käme dann ungefähr bei so etwas heraus (ungetestet):
Code: Alles auswählen
# coding: utf8
from __future__ import absolute_import, division, print_function
import errno
import os
import time
import random
from datetime import datetime as DateTime
from serial import Serial
DATA_PATH = '/home/pi/data'
#
# Periods in seconds.
#
CHECK_TEMPERATURE_PERIOD = 9
CHECK_AIR_PRESSURE_PERIOD = 30
CHECK_GAS_PERIOD = 20
CHECK_BRIGHTNESS_PERIOD = 60
CHECK_WIND_PERIOD = 10
CHECK_BATTERY_PERIOD = 10800
BLUE = '\033[94m'
GREEN = '\033[92m'
YELLOW = '\033[93m'
RED = '\033[91m'
NO_COLOR = '\033[0m'
def colorize(color, string):
return ''.join([color, string, NO_COLOR])
def save_data(name, data):
if not data:
data = colorize(RED, 'NO DATA')
print(name, data) # TODO Replace with `logging`.
now = DateTime.utcnow()
path = os.path.join(DATA_PATH, format(now, '%Y'), format(now, '%m'))
try:
os.makedirs(path)
except OSError as error:
if not error.errno == errno.EEXIST:
raise
with open(os.path.join(path, name + '.txt', 'a')) as data_file:
data_file.write(
'{0:%H:%M:%S} {1}\n'.format(now, random.randint(-20, 40))
)
def communicate(port, command):
port.write(command)
port.flush()
return port.readline()
def main():
temperature_timestamp = 0
air_pressure_timestamp = 0
gas_timestamp = 0
brightness_timestamp = 0
wind_timestamp = 0
battery_timestamp = 0
try:
while True:
now = time.time()
with Serial('/dev/ttyAMA0', baudrate=9600, timeout=1) as port:
if now - temperature_timestamp >= CHECK_TEMPERATURE_PERIOD:
temperature_timestamp = now
print(colorize(YELLOW, '#-{0:-<25}#'.format('Temperatur')))
save_data('Temperatur_Luft', communicate(port, '1'))
save_data('Temperatur_Boden', communicate(port, '4'))
if now - air_pressure_timestamp >= CHECK_AIR_PRESSURE_PERIOD:
air_pressure_timestamp = now
print(colorize(YELLOW, '#-{0:-<25}#'.format('Luftdruck')))
save_data('Luftdruck', communicate(port, '0'))
if now - gas_timestamp >= CHECK_GAS_PERIOD:
gas_timestamp = now
print(colorize(YELLOW, '#-{0:-<25}#'.format('Gas')))
save_data('Gas', communicate(port, '2'))
if now - brightness_timestamp >= CHECK_BRIGHTNESS_PERIOD:
brightness_timestamp = now
print(colorize(YELLOW, '#-{0:-<25}#'.format('Helligkeit')))
save_data('Helligkeit', communicate(port, '3'))
if now - wind_timestamp >= CHECK_WIND_PERIOD:
wind_timestamp = now
print(colorize(YELLOW, '#-{0:-<25}#'.format('Wind')))
save_data('Wind', communicate(port, '5'))
if now - battery_timestamp >= CHECK_BATTERY_PERIOD:
battery_timestamp = now
#
# TODO Use `sched` or something similar instead of this
# ”busy waiting” loop.
#
time.sleep(1)
except KeyboardInterrupt:
print('\nBeende')
if __name__ == '__main__':
main()
Die Programmstruktur in den ganzen ``if``\s die auf die Zeit prüfen ist fast identisch, das riecht sehr stark nach etwas das man refaktorisieren sollte. Prüfintervall, Name des Wertes, Befehl zum senden, und der Zeitstempel des letzten Tests gehören ja irgendwie zusammen und das ist im Grunde das einzige durch das sich die Codeabschnitte unterscheiden. Es könnte also Sinn machen diese Daten zu einem Datentyp zusammenzufassen.
Die Hauptschleife ist hauptsächlich mit „busy waiting” beschäftig, belastet also die CPU ohne dass sie etwas sinnvolles tut. Man sollte mindestens ein konstantes `time.sleep()` einführen, besser aber noch geziehlt auf das nächste Messereignis warten. In der Standardbibliothek gibt es beispielsweise das `sched`-Modul das dabei helfen kann.
