@Brathorst: Also ein offensichtliches Problem bei dem Quelltext ist die Einrückung. Du hast da Tabulatorzeichen und Leerzeichen gemischt was in Deinem Editor mit Deinen Einstellungen richtig aussehen mag, hier im Forum aber beispielsweise nicht. Wobei das eventuell auch vom Browser anhängen kann. Ausserdem weicht die Einrückung von den im
Style Guide for Python Code angegebenen vier Leerzeichen pro Ebene ab.
`os` und `Popen` werden importiert, aber nicht verwendet.
Namen komplett in Grossbuchstaben sind per Konvention für Konstanten vorgesehen. `SPOT_1` und `SPOT_2` sind allerdings Objekte deren Zustand im Laufe des Programms verändert wird, damit also nicht Konstant. Bei den beiden Objekten hast Du die Pin-Nummern als Zahlen angegeben anstatt wie bei den anderen Pin-Nummern Konstanten zu definieren.
Auf Modulebene sollten nur Konstanten, Funktionen, und Klassen definiert werden. Das Hauptprogramm steht üblicherweise in einer Funktion die `main()` heisst. Die `blitz()`-Funktion ist zudem mitten im Hauptprogramm an einer willkürlich erscheinenden Stelle definiert, was das Lesen an der Stelle etwas erschwert.
Wenn man das Hauptprogramm in eine Funktion verschiebt, fällt auf das `blitz()` Variablen benutzt die nicht als Argumente übergeben werden, sondern einfach so ”magisch” aus der Umgebung verwendet werden. Werte, ausser Konstanten, sollten Funktionen als Argumente betreten und als Rückgabewerte verlassen.
`device` ist als Name ein bisschen zu generisch für eine LED-Matrix.
Was im ``except``-Block steht, sollte in ein ``finally`` verschoben werden, denn auch wenn das Programm aus anderen Gründen abbricht, beispielsweise ein Programmierfehler, möchte man, dass die Aufräumarbeiten durchgeführt werden.
Das Programm steckt solange der Auslöseknopf nicht gedrückt wird in einer Endlosschleife die den Prozessor unnötig belastet. Dieses „busy waiting“ ist deshalb keine gute Idee. Das GPIO-Modul hat eine Funktion um auf eine Flanke zu warten die intern effizienter implementiert sein sollte als so eine Schleife. Damit spart man praktischerweise dann auch eine Einrück-Ebene ein.
`snap` ist eigentlich nur ein Flag und kein Zähler. Das würde deutlicher wenn man den Code entsprechend formuliert. Ausserdem ist ”Schnapp”/schnappen/einrasten vielleicht kein so passender Name für das was der Wert eigentlich bedeutet. Wenn man eine ”Endlosschleife” daraus macht, die im Erfolgsfall durch ein ``break`` abgebrochen wird, kann man sich den Namen komplett sparen.
Laut erster Zeile handelt es sich um Python 2, da ist ``print`` allerdings eine Anweisung und keine Funktion. Also sollte man entweder durch den entsprechenden `__future__`-Import dafür sorgen, dass es eine Funktion ist, oder es nicht so schreiben als wäre es eine.
In der inneren ``while``-Schleife werden Sachen mehrfach gemacht die man *einmal* davor erledigen könnte. Zum Beispiel die Bereitschafts-LED ausschalten oder den Blitz leicht anschalten. Die Helligkeit der LED-Matrix wird sogar nur in dieser Schleife immer wieder auf den gleichen konstanten Wert gesetzt. Das könnte man aus allen Schleife heraus ziehen.
Bei den ganzen `pixel()`-Aufrufen könnte man die Unterschiede in eine Datenstruktur herausziehen und eine Schleife darüber schreiben. Auch den Countdown kann man mit einer Schleife kürzer schreiben.
``shell=True`` sollte man beim `subprocess`-Modul nicht verwenden sofern man das nicht *wirklich* braucht. Und selbst dann sollte man versuchen eine Lösung ohne eine zusätzliche Shell zwischen dem Python-Programm und dem gestarteten Prozess zu finden. Die Pfade kann man als Konstanten herausziehen, damit sich das besser anpassen lässt wenn man das an eine andere Stelle verschiebt.
Warum werden die externen Programme mit ``sudo`` aufgerufen? Hat das laufende Programm selbst nicht schon diese Rechte um auf die GPIOs zugreifen zu können?
Aus einem Python-Programm heraus ein anderes Python-Modul als Programm zu starten ist nicht die erste Wahl. Was macht `wait.py` denn?
Ich lande dann ungefähr bei dem hier (ungetestet):
Code: Alles auswählen
#!/usr/bin/env python
# coding: utf8
# Einzelfoto und LED Matrix
from __future__ import absolute_import, division, print_function
import os
import subprocess
import sys
import threading
import time
from max7219 import led
from RPi import GPIO
BASE_PATH = '/home/pi'
IMAGE_PATH = os.path.join(BASE_PATH, 'photobox/photobox%H%M%S.jpg')
PROGRAM_PATH = os.path.join(BASE_PATH, 'scripts/photobox')
PRINT_SCRIPT_PATH = os.path.join(PROGRAM_PATH, 'jonas/assemble_and_print_1')
WAIT_PROGRAM_PATH = os.path.join(PROGRAM_PATH, 'wait.py')
BUTTON_PIN = 24
PRINT_LED_PIN = 22
READY_LED_PIN = 23
SPOT_PINS = [18, 25]
def flash(pwms):
for pwm in pwms:
pwm.ChangeDutyCycle(100)
time.sleep(0.5)
for pwm in pwms:
pwm.ChangeDutyCycle(0)
def main():
led_matrix = led.matrix()
led_matrix.brightness(4)
try:
GPIO.setmode(GPIO.BCM)
GPIO.setup(BUTTON_PIN, GPIO.IN)
GPIO.setup([READY_LED_PIN, PRINT_LED_PIN], GPIO.OUT)
flash_pwms = list()
for pin in SPOT_PINS:
GPIO.setup(pin, GPIO.OUT)
pwm = GPIO.PWM(pin, 100)
pwm.start(0)
flash_pwms.append(pwm)
GPIO.output(READY_LED_PIN, True)
GPIO.output(PRINT_LED_PIN, False)
while True:
GPIO.wait_for_edge(BUTTON_PIN, GPIO.RISING)
GPIO.output(READY_LED_PIN, False)
for pwm in flash_pwms:
pwm.ChangeDutyCycle(5)
while True:
print('go')
for x, y in [
(1, 1), (2, 1), (5, 1), (6, 1),
(0, 2), (3, 2), (4, 2), (7, 2),
(0, 3), (4, 3), (7, 3),
(0, 4), (2, 4), (3, 4), (4, 4), (7, 4),
(0, 5), (3, 5), (4, 5), (7, 5),
(1, 6), (2, 6), (5, 6), (6, 6),
]:
led_matrix.pixel(x, y, 1)
time.sleep(1.6)
for output, led_letter in [
('3', '3'), ('2', '2'), ('1', '1'), ('Smile!', '\x01')
]:
time.sleep(0.4)
print(output)
led_matrix.clear()
led_matrix.letter(0, ord(led_letter))
time.sleep(1)
threading.Timer(1.5, flash, [flash_pwms]).start()
print('SNAP')
gphoto_output = subprocess.check_output(
[
'gphoto2',
'--capture-image-and-download',
'--filename', IMAGE_PATH,
],
stderr=subprocess.STDOUT,
)
print(gphoto_output)
led_matrix.clear()
if 'ERROR' not in gphoto_output:
break
print('please wait while your photos print...')
#
# Build image and send to printer.
#
GPIO.output(PRINT_LED_PIN, True)
subprocess.Popen(['sudo', 'sh', PRINT_SCRIPT_PATH])
subprocess.call(['sudo', sys.executable, WAIT_PROGRAM_PATH])
time.sleep(2)
#
# TODO Wie lange dauert der Druckvorgang tatsaechlich?
#
GPIO.output(PRINT_LED_PIN, False)
print('ready for next round')
GPIO.output(READY_LED_PIN, True)
except KeyboardInterrupt:
pass # Intentionally ignored.
finally:
GPIO.cleanup()
led_matrix.clear()
if __name__ == '__main__':
main()
Die Hauptfunktion ist so deutlich zu lang. Das würde ich als nächstest in Angriff nehmen. Und wenn man dann beim Python lernen bei Klassen angekommen ist, kann man sicher auch das eine oder andere sinnvoll in einer Klasse kapseln.
