Eisbahn Das Programm in #41 ist ziemlich unübersichtlich. Es ist ein sehr viel Code auf Modulebene in dem eine Funktionsdefinition ”versteckt” ist, wobei diese ”Funktion” über vier globale Variablen mit dem ganzen Rest zusammenhängt. Um das zu verstehen, muss man tatsächlich die ganzen 150 Zeilen am Stück verstehen.
Konstanten sind nicht GROSS_GESCHRIEBEN und werden auch mit Variablendefinitionen vermischt, das heisst auch da muss man auch den ganzen Code durchgehen um zu verstehen was konstant und was variabel ist.
``as`` beim Importieren ist zum umbenennen da, `GPIO` wird aber gar nicht umbenannt.
Es werden teilweise Variablen meilenweit von der Stelle entfernt definiert an der sie letztlich benutzt werden. Teilweise sogar bevor überhaupt klar ist ob sie überhaupt genutzt werden. Unter anderem weil die Verarbeitung der Kommandozeilenargumente sich so komischt durch das ganze Hauptprogramm zieht, statt das erst mal abzuklären bevor man irgendwas macht was man am Ende sowieso nicht braucht wenn die Angaben dort nicht korrekt waren.
Kommentare sollen dem Leser einen Mehrwert über den Code geben. Faustregel: Kommentare beschreiben nicht *was* der Code macht, denn das steht da bereits als Code, sondern warum er das macht. Sofern das nicht offensichtlich ist. Offensichtlich ist in aller Regel auch was in der Dokumentation von Python und den verwendeten Bibliotheken steht. Das sieht fast so ein bisschen ChatGPT aus der Kommentierungsstil.
In Kommentaren sollten insbesondere keine Werte aus dem Programm noch mal vorkommen. Das ist eine Fehlerquelle wenn man diese Werte mal ändert, und nicht alle Kommentare auch mitändert in denen der Wert unnötigerweise auch noch mal drin steht.
Pfade bastelt man nicht mit Zeichenkettenoperationen zusammen. Das ist fehleranfällig weil da bestimmte Regeln erfüllt sein müssen. Dafür gibt es das `pathlib`-Modul.
Der Test auf das Kommandozeilenargument ist extrem umständlich geschrieben. valid_location = len(sys.argv) == 2 and sys.argv[1] in ["Pforte_Zuweg", "Garten"] muss man echt nicht auf 21 Zeilen Quelltext strecken.
Der Code akzeptiert Pforte_Zuweg und Garten, der Benutzungshinweis behauptet aber auch noch Hundehaus. Diese Information für Test und Ausgabe sollte nur einmal im Quelltext stehen, beispielsweise als Liste, und Prüfung und Textausgabe sollte darauf zugreifen.
Die Elemente von `sys.argv` sind bereits Zeichenketten. Ein `str()` bringt da nichts.
Das zusammenstückeln von Zeichenketten und Werten mittels ``+`` und `str()` ist eher BASIC als Python. Dafür gibt es die `format()`-Methode auf Zeichenketten und f-Zeichenkettenliterale.
Das ``try``/``finally`` umfasst nicht den gesamten Code in dem `GPIO.cleanup()` nötig ist.
Der Kommentar „# some debug message“ passt nicht zu `logging.info()` und bei `logging.debug()` braucht man den Kommentar nicht mehr. Was davon ist jetzt falsch? Der Kommentar oder `info()` statt `debug()`?
Diese Rahmen bei den Logausgaben funktionieren nicht, weil da ja dynamisch Werte eingesetzt werden die unterschiedliche Längen haben können und damit nicht zu den Linien mit statischer Länge passen.
Da sind zwei Kommentare zu `sleep()` wo ich denke das Du da was falsch verstanden hast. `sleep()` wird *nicht* durch erkannte Flanken unterbrochen. Also die Kommentare ``# loop forever, interrupt would break the sleep`` und ``# just go cyclic to sleep in main (would be interrupted if edge is detected)`` sind inhaltlich falsch.
Die Aufteilung zwischen Rückruffunktion und Hauptprogramm ist auch komisch. Rückruffunktionen bei so etwas sollten so kurz wie möglich sein. Am besten einfach nur ein Flag setzen oder Werte in eine Queue schreiben. Dann kann das Hauptprogramm auch *sofort* reagieren und man kann tatsächlich so etwas wie `sleep()` das von einer erkannten Flanke unterbrochen wird programieren in dem man beim `Queue.get()` ein `timeout` verwendet.
`trigger_filename_part` wird an *drei* Stellen benutzt um mit gleichem Code den gleichen Dateinamen zusamenzusetzen.
Man macht keine Vergleiche mit literalen Wahrheitswerten. Bei dem Vergleich kommt doch nur wieder ein Wahrheitswert bei heraus. Entweder der, den man sowieso schon hatte; dann kann man den auch gleich nehmen. Oder das Gegenteil davon; dafür gibt es ``not``.
Komplett ungetestet:
#!/usr/bin/env python3
#
# script could be run as daemon/service, see below for a short how-to manage it
# with actual systemd
#
# <location> is a mandatory parameter and should be Pforte_Zuweg or others
# /home/pi/rpicamera/trigger.py <location>
#
import logging
import sys
import time
from datetime import datetime as DateTime, timezone as Timezone
from pathlib import Path
from queue import Queue
from RPi import GPIO
TRIGGER_FILE_EXTENSION = ".trg"
TRIGGER_PATH = Path("/home/pi/rpicamera/trigger")
MIN_TRIGGERLENGTH = 20
MAX_TRIGGERLENGTH = 300
BOUNCE_TRIGGERLENGTH = 10
LOG_PATH = Path("/home/pi/rpicamera/log")
LOG_FILENAME = "surveillance.log"
LOCATIONS = ["Pforte_Zuweg", "Garten", "Hundehaus"]
MOTION_SENSOR_PIN = 17
def get_time_diff(timestamp_a, timestamp_b):
return (timestamp_a - timestamp_b).total_seconds()
def main():
logging.basicConfig(
filename=LOG_PATH / LOG_FILENAME,
format="%(asctime)s %(levelname)s: %(message)s",
datefmt="%Y-%m-%d %H:%M:%S",
level=logging.INFO,
)
if not (len(sys.argv) == 2 and sys.argv[1] in LOCATIONS):
print(
"wrong usage of command line parameters identified,"
" aborting the script"
)
print("correct usage is /home/pi/rpicamera/trigger.py <location>")
print("<location> should be", " or ".join(LOCATIONS))
print(f"you called {sys.argv[0]}, complete arguments are: {sys.argv}")
sys.exit(2)
else:
location = sys.argv[1]
logging.info(
"trigger script at %s for motion detection has been started right now",
location,
)
try:
GPIO.setmode(GPIO.BCM)
GPIO.setup(MOTION_SENSOR_PIN, GPIO.IN, pull_up_down=GPIO.PUD_UP)
while GPIO.input(MOTION_SENSOR_PIN) != 1:
time.sleep(0.1)
edge_queue = Queue()
GPIO.add_event_detect(
MOTION_SENSOR_PIN,
GPIO.BOTH,
callback=lambda pin: edge_queue.put(
(
GPIO.input(pin),
DateTime.utcnow().replace(tzinfo=Timezone.utc),
)
),
bouncetime=20,
)
logging.info(
"Interrupt is configured, now wait for some motion at %s", location
)
trigger_active = False
for pir_state, timestamp in iter(edge_queue.get, None):
logging.info(
"trigger script for %s, interrupt/callback entered", location
)
#
# Check if a falling edge is present = motionsensor has detetced
# motion
#
if pir_state == 0:
triggertime = timestamp
if not trigger_active:
local_time = triggertime.astimezone(None)
trigger_file_path = TRIGGER_PATH / (
f"{local_time:%Y-%m-%d-%H-%M-%S}_loc_{location}"
f"{TRIGGER_FILE_EXTENSION}"
)
trigger_file_path.write_bytes(b"")
logging.info(
"motion was detected at %s, triggerfile was created: %s",
location,
local_time,
)
trigger_active = True
logging.info(
"trigger script for %s detected some motion", location
)
else:
stoptime = DateTime.utcnow()
logging.info(
"trigger script for %s, no more motion was detected",
location,
)
if trigger_active:
if get_time_diff(timestamp, triggertime) >= MAX_TRIGGERLENGTH:
trigger_active = False
trigger_file_path.unlink()
logging.info(
"trigger script for %s removed the trigger %s as"
" maximum allowed triggerlength is exceeded",
location,
local_time,
)
elif stoptime >= triggertime:
if (
get_time_diff(timestamp, triggertime)
>= MIN_TRIGGERLENGTH
and get_time_diff(timestamp, stoptime)
>= BOUNCE_TRIGGERLENGTH
):
trigger_active = False
trigger_file_path.unlink()
logging.info(
"trigger script for %s removed the trigger %s",
location,
local_time,
)
except Exception:
logging.exception(
"something went wrong at %s interrupt handling script", location
)
finally:
logging.info(
"script for the %s interrupt handling was aborted and is ending now",
location,
)
GPIO.cleanup()
if __name__ == "__main__":
main()
Alles anzeigen