Pygame Anfänger: Wer gibt mir Rückmeldung zu meinem Skript?

Hier werden alle anderen GUI-Toolkits sowie Spezial-Toolkits wie Spiele-Engines behandelt.
Antworten
sirhc
User
Beiträge: 5
Registriert: Montag 4. Januar 2016, 14:16

Hallo,

ich habe vor mit meinem Raspberry und einer DSLR eine kleine Fotobox zu bauen. Dazu sollen auf gewisse Trigger eine Slideshow ablaufen (3...2...1...Lächeln) -> Bildaufnhame -> Bild anzeigen -> wieder von vorne.

Was soll ich sagen: Es funktioniert auch!
Den Code habe ich mir aus den diversen Docs und Foren zusammengesucht, vor Python habe ich vor langer Zeit recht viel mit PHP gemacht, sonst habe ich gar nichts mehr mit der Materie zu tun. Ich habe das Gefühl vor allem Pygame massiv zu "miss"-brauchen - daher meine Frage: Kann mal jemand meinen Code begutachten und eventuell ein paar Rückmeldungen geben? Gerne natürlich auch, wenn was ganz gut ist :D

Hier einmal leserlicher im Pastebin:
http://pastebin.com/ktPmGnMT
Und hier für Leute, die nicht nach Extern wollen ;-):

Code: Alles auswählen

#!/usr/bin/env python

import pygame
import os
import sys
import re
import piggyphoto
from glob import glob
from pygame.locals import *

## vars
(B, H) = (1280, 800)
BLACK = (0, 0, 0)
XY = [0, 0]

## thumbail size
thx = 714
thy = 474

## image settings
ImgFolder = "images/"
ImgPre = "IMG_"

## initialize surface, set caption, mouse invisible
pygame.init()
surf = pygame.display.set_mode((B, H), 0, 32)
pygame.display.set_caption('Photo.Box v1.0')
pygame.mouse.set_visible(False)

## fill bg + start image
surf.fill(BLACK)
start = pygame.image.load("start.png").convert_alpha()
surf.blit(start, XY)
pygame.display.update()

## toogle fullscreen
def ToggleScreen():
	pygame.display.toggle_fullscreen()

    
## function to number images taken
def ImgCount(Folder):
	if not os.path.exists(Folder):
		os.makedirs(Folder, 0777)
    
	if os.path.isfile(Folder + ImgPre + '0001.jpg'):
		files = []
		for f in glob(os.path.join(Folder, "*.jpg")):
			files.append(os.path.splitext(os.path.basename(f))[0])

		files = sorted(files)
		lastpic = files[-1]
		lastpic = re.findall(r'\d+', lastpic)
		lastpic = int(''.join(lastpic))
		Count = lastpic + 1
			
		return str(Count).zfill(4)
    
	else:
		Count = str(1).zfill(4)
	
		return Count

    
## main func
def PhotoBooth():
	## screensize
	screen = pygame.display.Info()
	w = screen.current_w
	h = screen.current_h
	
	## vars
	global xy, start, thx, thy, ImgFolder, ImgPre
	bg1 = pygame.image.load("1.png").convert_alpha()
	bg2 = pygame.image.load("2.png").convert_alpha()
	bg3 = pygame.image.load("3.png").convert_alpha()
	smile = pygame.image.load("smile.png").convert_alpha()
	proc = pygame.image.load("proc.png").convert_alpha()

	
	Count = ImgCount(ImgFolder)
	Filename = ImgFolder + ImgPre + Count + ".jpg"
	
	cam = piggyphoto.camera()
	
	## Sequence1 
	surf.fill(BLACK)
	surf.blit(bg1, XY)
	pygame.display.update()
	pygame.time.delay(500)
	surf.fill(BLACK)
	pygame.display.update()
	pygame.time.delay(500)
	
	## Sequence2
	surf.blit(bg2, XY)
	pygame.display.update()
	pygame.time.delay(500)
	surf.fill(BLACK)
	pygame.display.update()
	pygame.time.delay(500)
	
	## Sequence3
	surf.blit(bg3, XY)
	pygame.display.update()
	pygame.time.delay(500)
	surf.blit(smile, XY)
	pygame.display.update()
	pygame.time.delay(50)
	
	## Take picture
	cam.capture_image(Filename)
	
	## create thumbnail
	##th = Image.open(Filename)
	##th = th.resize((thx, thy), Image.ANTIALIAS) 
	##th = th.save(thumb)
	
	## wait while your image is processed
	surf.blit(proc, XY)
	pygame.display.update()
	pygame.time.delay(500)
	
	## display taken image
	photo = pygame.image.load(Filename).convert_alpha()
	photo = pygame.transform.scale(photo, (w, h))
	surf.blit(photo, XY)
	pygame.display.update()
	pygame.time.delay(5000)
	surf.fill(BLACK)

	pygame.time.delay(500)
	
	## back to start
	surf.blit(start, XY)
	pygame.display.update()

	
## loop
while 1:
	event = pygame.event.wait()
	
	if event.type == KEYDOWN:
		if event.key == pygame.K_ESCAPE:
			ToggleScreen()
	
	if event.type == KEYDOWN:
		if event.key == pygame.K_SPACE:
			PhotoBooth()

	if event.type == pygame.MOUSEBUTTONDOWN and event.button==3:
		PhotoBooth()
		
	if event.type == KEYDOWN:
		if event.key == pygame.K_q:
			pygame.quit()
			sys.exit()
			
	if event.type == QUIT:
		pygame.quit()
		sys.exit()		
	pygame.time.delay(50)
Vielen Dank im Voraus!
Grüße,
Chris
Zuletzt geändert von Anonymous am Montag 4. Januar 2016, 14:50, insgesamt 1-mal geändert.
Grund: Quelltext in Python-Codebox-Tags gesetzt.
Üpsilon
User
Beiträge: 222
Registriert: Samstag 15. September 2012, 19:23

Hi!
Die Variablennamen sind teilweise nicht aussagekräftig. Was sollen denn B und H und XY sein?
Lies mal die offiziellen Stil-Richtlinien: https://www.python.org/dev/peps/pep-0008/
Für Kommentare braucht man nur ein Rautezeichen.
Die Zeile mit dem global ist unnötig. global braucht man dann, wenn man in einer Funktion globale Variablen ändern will. Also eigentlich nie.
Beim Countdown willst du bestimmt eine Schleife benutzen. Das sähe ungefähr so aus (prinzipiell und ungetestet):

Code: Alles auswählen

for ziffer in "321":
    bild = pygame.image.load(ziffer+".png").convert_alpha()
    surf.fill(BLACK)
    surf.blit(bild, XY)
    pygame.display.update()
    pygame.time.delay(1000)
Statt files=sorted(files) kann man auch files.sort() sagen.
In Z.60 kann man auch gleich return aufrufen.
Liebe Grüße.
PS: Die angebotene Summe ist beachtlich.
BlackJack

@sirhc: Die Namensschreibweisen folgen nicht alle dem Style Guide for Python Code.

Auf Modulebene sollte kein Code stehen der nicht Konstanten, Funktionen, oder Klassen definiert. Das Hauptprogramm gehört auch in eine Funktion die üblicherweise `main()` heisst. Funktionen (und Methoden) sollten ausser auf Konstanten auf keine Werte zugreifen die nicht als Argumente übergeben wurden und ``global`` hat in einem Programm das übersichtlich und verständlich bleiben soll nichts zu suchen.

Kommentare sollten einen Mehrwert zum Quelltext bilden, also nicht beschreiben was da im Code sowieso schon steht. Ein Kommentar beschreibt üblicherweise *warum* etwas so gemacht wird wie es im Code steht. Sofern das nicht aus dem Code schon offensichtlich wird wie bei ``## loop`` vor einer ``while``-Schleife. Wer das ohne den Kommentar nicht sieht, dem hilft auch der Kommentar nicht weiter. Wenn ein Kommentar einen Namen erklären muss, dann ist wahrscheinlich der Name schlecht gewählt. Zum Beispiel weil der abgekürzt wurde wie beispielsweise bei `thx` und `thy` die wohl besser `tumbnail_width` und `thumbnail_height` heissen sollten. Auch sehr unschön sind `B` und `H` was wohl für `BREITE` und `HOEHE` steht, in einem Quelltext der sonst englische Bezeichner/Abkürzungen verwendet. Wirklich nicht gut sind Kommentare die nicht stimmen, zum Beispiel ``## vars`` vor der Definition von Konstanten oder ``## main func`` for einer Funktion die gar nicht die Hauptfunktion ist.

Warum sind vor den Kommentaren eigentlich immer zwei ``#``-Zeichen?

Pfadangaben werden mit Funktionen aus `os.path` manipuliert und nicht mit einfachen Zeichenkettenoperationen.

Kommentare vor einer Funktion, die beschreiben was die Funktion tut, wären besser Docstrings.

Da Funktionen etwas *tun* werden sie üblicherweise nach Tätigkeiten benannt um sie besser von ”passiven” Werten unterscheiden zu können.

Die `ImgCount()`-Funktion tut mehr als der Name und der Kommentar erwarten lassen. Eine Funktion welche die nächste Dateinummer ermittelt, sollte nicht gleichzeitig Verzeichnisse anlegen. Die Funktion selber ist sehr umständlich und komisch. `re.findall()` um *ein* Ergebnis zu finden und das *eine* Ergebnis dann mit `str.join()` ”zusammensetzen” wobei die Zeichenkette vollkommen egal ist, weil sie ja nie *zwischen* andere Werte gesetzt wird‽

Bei der Anzeige der Bildsequenz wiederholt sich viel Code den man in eine Schleife (und in eine Funktion) herausziehen kann. Die statischen Bilder muss man auch nicht unbedingt für jedes Foto neu laden.

Python hat einen eigenen Datentyp für Wahrheitswerte (`bool`) mit den Werten `True` und `False`. Die sollte man verwenden wenn man literale Wahrheitswerte meint, statt Zahlen.

In der Hauptschleife sind zu viele ``if``\s. Einige kann man zusammenfassen und die verbleibenden sollten, wo sie sich gegeneinander ausschliessen, durch ``elif``\s ersetzt werden.

Ich komme dann als Zwischenergebnis auf so etwas (ungetestet):

Code: Alles auswählen

#!/usr/bin/env python
import os
import sys
import re
from glob import glob

import piggyphoto
import pygame
from pygame.locals import *
 
__version__ = '1.1'

BLACK = (0, 0, 0)
TOP_LEFT = (0, 0)
DISPLAY_SIZE = (1280, 800)
IMAGES_PATH = 'images'
IMAGE_NAME_TEMPLATE = 'IMG_{0}.jpg'

   
def get_next_image_number(path):
    try:
        last_path = max(
            glob(os.path.join(path, IMAGE_NAME_TEMPLATE.format('*')))
        )
    except ValueError:
        return 0  # No matching filenames found.
    else:
        last_filename = os.path.basename(last_path)
        match = re.search(r'\d+', last_filename)
        return int(match.group()) + 1 if match else 0


def play_sequence(surface, images):
    for image, duration in images:
        surface.fill(BLACK)
        if image:
            surface.blit(image, TOP_LEFT)
        pygame.display.update()
        pygame.time.delay(duration)


def load_images():
    background_image_1 = pygame.image.load('1.png').convert_alpha()
    background_image_2 = pygame.image.load('2.png').convert_alpha()
    background_image_3 = pygame.image.load('3.png').convert_alpha()
    smile_image = pygame.image.load('smile.png').convert_alpha()
    processed_image = pygame.image.load('proc.png').convert_alpha()
    return {
        'sequence': [
            (background_image_1, 500),
            (None, 500),
            (background_image_2, 500),
            (None, 500),
            (background_image_3, 500),
            (smile_image, 50),
        ],
        'processed_image': processed_image,
    }
    
   
def take_photo(surface, images):
    play_sequence(surface, images['sequence'])

    camera = piggyphoto.camera()  # TODO Really instanciate this for each photo?

    if not os.path.exists(IMAGES_PATH):
        os.makedirs(IMAGES_PATH, 0777)
    filename = os.path.join(
        IMAGES_PATH,
        IMAGE_NAME_TEMPLATE.format(
            format(get_next_image_number(IMAGES_PATH), '04d')
        )
    )
    camera.capture_image(filename)
   
    surface.blit(images['processed_image'], TOP_LEFT)
    pygame.display.update()
    pygame.time.delay(500)
   
    photo = pygame.transform.scale(
        pygame.image.load(filename).convert_alpha(), surface.get_size()
    )
    surface.blit(photo, TOP_LEFT)
    pygame.display.update()
    pygame.time.delay(5000)
    surface.fill(BLACK)
    pygame.time.delay(500)
 

def main():
    pygame.init()
    surface = pygame.display.set_mode(DISPLAY_SIZE, 0, 32)
    pygame.display.set_caption('Photo.Box v' + __version__)
    pygame.mouse.set_visible(False)
     
    surface.fill(BLACK)
    start_image = pygame.image.load('start.png').convert_alpha()
    surface.blit(start_image, TOP_LEFT)
    pygame.display.update()
    images = load_images()

    while True:
        event = pygame.event.wait()
        
        if event.type == KEYDOWN and event.key == pygame.K_ESCAPE:
            pygame.display.toggle_fullscreen()
        elif (
            event.type == KEYDOWN and event.key == pygame.K_SPACE
            or event.type == pygame.MOUSEBUTTONDOWN and event.button == 3
        ):
            take_photo(surface, images)
            surface.blit(start_image, TOP_LEFT)
            pygame.display.update()
        elif (
            event.type == KEYDOWN and event.key == pygame.K_q
            or event.type == QUIT
        ):
            pygame.quit()
            return

        pygame.time.delay(50)


if __name__ == '__main__':
    main()
sirhc
User
Beiträge: 5
Registriert: Montag 4. Januar 2016, 14:16

Wow, vielen Dank euch Beiden!

Den Style-Guide kannte ich noch gar nicht. Das kommt wohl davon, wenn man mit einem Buch anfängt, in dem Python nur kurz als Beiwerk angerissen wird und den Rest aus Stackoverflow etc. bezieht.

Das Breite, Höhe etc. war zB ein solches Relikt aus dem Buch (Pygame Übungen), welches ich im Nachhinein nicht mehr angepasst habe ...
Manchmal ist es vielleicht besser alles nochmal von "vorn" zu schreiben.

Wenn ich das richtig sehe, dann sollte ich mich vor allem mit den Funktionen zum Dateisystem, format, sinnvoller Nutzung von Schleifen und dem Styleguide beschäftigen?! Zudem mehr einzelne Funktionen, jeweils für genau eine Funktion und dann nur die main() Funktion in der Hauptschleife.

Bzgl. des ToDo: Sollte ich die Kamera-Initialisierung lieber noch in die main()-Funktion packen? Oder lieber nicht, da sie dort "nichts zu suchen" hat?

Und: Die DSLR macht (als nicht RAW) jpgs bis ~8MB. Diese laden dann etwas verzögert "von oben nach unten". Würden Thumbnails da Abhilfe schaffen? Bzw: Wie groß sollte eine Bilddatei maximal sein, dass PyGame (oder wahrscheinlich besser gesagt: ein Raspberry Pi) sie laden kann? Gibt's da Werte? Ansonsten probiere ich es einfach heute Abend mal aus.

Eine konkrete Frage habe ich noch: Angenommen die DSLR kann aus irgendeinem Grund KEIN Foto aufnehmen (Kamera nicht verbunden, Kamera kann nicht fokussieren, ...), dann gibt sie eine Fehlermeldung aus und der Code bricht ab. Kann man so etwas in Python abfangen und dann ggf. einfach eine kurze Meldung ausgeben und dann wieder von vorn starten?
Hier die Ausgabe:

Code: Alles auswählen

pi@raspberrypi:~/Box $ ./Box.py
Traceback (most recent call last):
  File "./Box.py", line 110, in <module>
    main()
  File "./Box.py", line 100, in main
    take_photo(surface, images)
  File "./Box.py", line 63, in take_photo
    camera = piggyphoto.camera()  # TODO Really instanciate this for each photo?
  File "/usr/lib/python2.7/dist-packages/piggyphoto/__init__.py", line 222, in __init__
    self.init()
  File "/usr/lib/python2.7/dist-packages/piggyphoto/__init__.py", line 239, in init
    check(ans)
  File "/usr/lib/python2.7/dist-packages/piggyphoto/__init__.py", line 205, in check
    raise libgphoto2error(result, message)
piggyphoto.libgphoto2error: Unknown model (-105)
Für einen Tip wie man so etwas realisieren kann wäre ich sehr dankbar - brauche auch keinen kompletten Code :)

Viele Grüße,
Chris
BlackJack

@sirhc: Bei dem Todo war halt die Frage ob es nötig ist für jedes Foto ein neues Kamera-Objekt zu erstellen. Beziehungsweise auch ob das ”teuer” ist. Wenn das schnell geht und nicht grossartig Ressourcen verbraucht kann man es auch so lassen wie es ist.

Wie gross die Bilder als JPEG sind ist nicht so wichtig, sondern die Anzahl der Pixel. Danach kann man sich ja ausrechnen wie viel Arbeitsspeicher so ein Bild verbraucht. Und man kann sich natürlich auch den Speicherverbrauch des Programms/Systems anschauen ob es da eng wird oder genug Luft ist.

Ausnahmen kann man behandeln, die müssen nicht zum Programmabbruch führen.
sirhc
User
Beiträge: 5
Registriert: Montag 4. Januar 2016, 14:16

Hey,

ah, habe es auch einfach mit try: und except: hinbekommen die Fehler abzufangen, wenn keine Kamera verbunden ist, und auch, wenn kein Bild gemacht werden kann. Ich dachte zuerst, dass man except auf die Fehlermeldung anpassen muss und es sonst nicht funktioniert - aber klappt alles gut!

Vielen Dank!
Viele Grüße,
Chris
BlackJack

@sirhc: Bei ``except`` sollte schon (mindestens) eine konkrete Ausnahme stehen sonst greift das bei *jeder* Ausnahme, auch solchen mit denen man gar nicht rechnet und das macht die Fehlersuche dann schwer bis unmöglich.
sirhc
User
Beiträge: 5
Registriert: Montag 4. Januar 2016, 14:16

Ok, dann brauche ich wohl doch noch einmal Hilfe:
Wenn ich mir in den Docs die möglichen "excepts" ansehe (https://docs.python.org/2/library/exceptions.html) - finde ich keine, die auf die Fehlermeldung passt - also quasi Abbruch durch "importierte Funktion"?

Viele Grüße,
Chris
Sirius3
User
Beiträge: 17703
Registriert: Sonntag 21. Oktober 2012, 17:20

@sirhc: Du brauchst doch nur in die Fehlermeldung zu schauen, die Du bekommst: piggyphoto.libgphoto2error
sirhc
User
Beiträge: 5
Registriert: Montag 4. Januar 2016, 14:16

Sirius3: Ok, das war einfacher als gedacht :D
Dankeschön!
Antworten