Fehler im Code!?

Python auf Einplatinencomputer wie Raspberry Pi, Banana Pi / Python für Micro-Controller
Antworten
CrazyAndy
User
Beiträge: 4
Registriert: Mittwoch 2. Januar 2019, 16:33

Hallo zusammen,

ein gesundes neues Jahr. Eins gleich vorweg, ich beschäftige mich sein ca. 1 Woche mit Python - ich bin Anfänger.
Ich habe Linux Kenntnisse und habe vor langer Zeit in Quick Basic programmiert.

Was habe ich vor? Ich bin Aquarianer und mit meinem Raspi will ich WS2801 LED's steuern. Weil es mich sehr stört ( und meinen Fischies) dass mein Licht von 0 auf 100% an und aus geht.

Ich habe einige Beispiele zum Steuern im Netz gefunden:
https://tutorials-raspberrypi.de/raspbe ... n-steuern/

Auch arbeite ich zum besseren Verständnis mit dem:
https://www.python-kurs.eu/python3_kurs.php

Und nun zu meinen Problem:
1. wieso geht nach dem Sonnenaufgang bzw. vor dem Sonnenuntergang kurz das Lich aus?
2. wie kann ich eine Zeitverzögerung zwischen dem Sonnenaufgang und Untergang von ca. 9 Stunden mit einbauen

Vielen Dank für eure Hilfe.

Hier soweit mein Script:

Code: Alles auswählen

import time
import Adafruit_WS2801
import Adafruit_GPIO.SPI as SPI

# LED Pixelanzahl
PIXEL_COUNT = 200

# Dimmgeschwindigkeit
n = 0

# Alternatively specify a hardware SPI connection on /dev/spidev0.0:
SPI_PORT   = 0
SPI_DEVICE = 0
pixels = Adafruit_WS2801.WS2801Pixels(PIXEL_COUNT, spi=SPI.SpiDev(SPI_PORT, SPI_DEVICE))

def Sonnenaufgang(pixels, wait=0.01, step=1):
    for j in range(int(256 // step)):
        for i in range(pixels.count()):
            r, g, b = pixels.get_pixel_rgb(i)
            r = int(max(0, r + step))
            g = int(max(0, g + step))
            b = int(max(0, b + step))
            pixels.set_pixel(i, Adafruit_WS2801.RGB_to_color( r, g, b ))
        pixels.show()
        if wait > 0:
            time.sleep(wait + n)


def Sonnenuntergang(pixels, wait=0.01, step=1):
    for x in range(PIXEL_COUNT):
        pixels.set_pixel_rgb(x, 255, 255, 255)
    for j in range(int(256 // step)):
        for i in range(pixels.count()):
            r, g, b = pixels.get_pixel_rgb(i)
            r = int(max(0, r - step))
            g = int(max(0, g - step))
            b = int(max(0, b - step))
            pixels.set_pixel(i, Adafruit_WS2801.RGB_to_color( r, g, b ))
        pixels.show()
        if wait > 0:
            time.sleep(wait + n)


if __name__ == "__main__":
    pixels.clear()  
    pixels.show()

    Sonnenaufgang(pixels)

    Sonnenuntergang(pixels)

    pixels.clear()            
    pixels.show()
    print("ENDE")
nezzcarth
User
Beiträge: 1632
Registriert: Samstag 16. April 2011, 12:47

Zu 2: Ich würde die Verzögerung nicht in das Skript einbauen, sondern das über Systemd Timer Units oder Cronjobs lösen. Die jeweilige aktion (Sonnenenauf/untergang) kann als Kommandozeilenparameter übergeben werden.

Ansonsten noch ein paar Anmerkungen: Funktionsnamen werden in Python qua PEP8 klein geschrieben. Die ganzen 'int()' kannst du dir sparen ('//' liefert -- anders als '/' -- eh einen Integer und bei den rgb Zuweisungen ist es m.M.n. zwecklos, da es an der Stelle entweder vorher schon einen Fehler gegeben hat oder irrelevant ist). Das 'if __name__ == '__main__'-Idiom wird üblicherweise verwendet, um eine main-Funktion aufzurufen; dass dort der eigentliche Code steht, sieht man eher selten.
Benutzeravatar
__blackjack__
User
Beiträge: 13004
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

@CrazyAndy: Ad 1.: Wie sind denn die RGB-Werte am Ende vom `Sonnenaufgang()`?

Ad 2.: Naja, Du weisst ja wie man Code um eine hundertstel Sekunde verzögert. Das geht mit 9 Stunden auch, ist halt nur eine andere Sekundenzahl.

Anmerkungen zum Quelltext: Auf Modulebene sollte nur Code stehen der Konstanten, Funktionen, und Klassen definiert. Das Hauptprogramm steht üblicherweise in einer Funktion die `main()` heisst.

`n` wird nicht wirklich benutzt. Das hätte für eine Konstante auch eine falsche Namensschreibweise und ist inhaltlich zu kurz und nichtssagend.

Namensschreibweise für Konstanten ist KOMPLETT_GROSS. Klassennamen in MixedCase, und alles andere klein_mit_unterstrichen.

Das Ergebnis einer Ganzzahldivision ist immer eine ganze Zahl. Die nochmal mit `int()` in eine ganze Zahl umzuwandeln ist also sinnfrei.

Ich sehe nicht so recht warum man eine Zeitverzögerung *und* `step` braucht. Was kann man damit effektiv erreichen was nur mit einer Zeitverzögerung nicht geht, ausser das Auf-/Abblenden weniger sanft zu gestalten. Will man das? Ausserdem wäre ich mal über eine Beschreibung in Worten zu `step` gespannt, und wie das zu dem Wort `step` passt. Im Grunde würde ich auch sagen das ist fehlerhaft, weil bei `step`-Werten bei denen die Division nicht glatt aufgeht, erreichen die LEDs nicht die maximale Helligkeit.

Zudem müssen die LEDs alle aus sein, damit das funktioniert, denn sonst bekommt man Werte grösser 255 für die Farbkomponenten und das führt dann zu einer Ausnahme. Im Grunde ist der Sonnenaufgang das hier:

Code: Alles auswählen

def sonnenaufgang(pixels, delay=0.01):
    for i in range(256):
        pixels.set_pixels_rgb(i, i, i)
        pixels.show()
        time.sleep(delay)
Sonnenauf- und Sonnenuntergang sehen auch viel zu ähnlich für *zwei* Funktionen aus. Das kann man mit *einer* erledigen. Oder mit einer zusätzlichen Hilfsfunktion die von den beiden Funktionen mit dem/den entsprechenden Argument(en) aufgerufen werden.

Ungetestet:

Code: Alles auswählen

#!/usr/bin/env python3
import time

from Adafruit_GPIO import SPI
from Adafruit_WS2801 import WS2801Pixels

LED_PIXEL_COUNT = 200
SPI_PORT = 0
SPI_DEVICE = 0


def clear(pixels):
    pixels.clear()
    pixels.show()


def show_levels(pixels, levels, delay):
    for level in levels:
        pixels.set_pixels_rgb(level, level, level)
        pixels.show()
        time.sleep(delay)


def sonnenaufgang(pixels, delay=0.01):
    show_levels(pixels, range(256), delay)


def sonnenuntergang(pixels, delay=0.01):
    show_levels(pixels, reversed(range(256)), delay)


def main():
    pixels = WS2801Pixels(LED_PIXEL_COUNT, spi=SPI.SpiDev(SPI_PORT, SPI_DEVICE))
    clear(pixels)    

    sonnenaufgang(pixels)
    sonnenuntergang(pixels)
    
    clear(pixels)
    print('ENDE')


if __name__ == '__main__':
    main()
“Most people find the concept of programming obvious, but the doing impossible.” — Alan J. Perlis
CrazyAndy
User
Beiträge: 4
Registriert: Mittwoch 2. Januar 2019, 16:33

Hallo zusammen,

nezzcarth
Zu 2: Ich würde die Verzögerung nicht in das Skript einbauen, sondern das über Systemd Timer Units oder Cronjobs lösen. Die jeweilige aktion (Sonnenenauf/untergang) kann als Kommandozeilenparameter übergeben werden.
Daran habe ich auch schon gedacht - kann es sein dass wenn ein Script über Wochen läuft es dan zu fehlern kommen kann?

__blackjack__
Ad 1.: Wie sind denn die RGB-Werte am Ende vom `Sonnenaufgang()`?
jetzt für den Anfang würde es reichen wenn von beginn des dimmens bis zu 100% Helligkeit einfach nur die Farbe 255, 255, 255 (weiss) ist.
`n` wird nicht wirklich benutzt. Das hätte für eine Konstante auch eine falsche Namensschreibweise und ist inhaltlich zu kurz und nichtssagend.
Wenn ich "n" eine Zahl von 2 zuweise, dann kann ich die Zeit des dimmens verlängern.
Das Ergebnis einer Ganzzahldivision ist immer eine ganze Zahl. Die nochmal mit `int()` in eine ganze Zahl umzuwandeln ist also sinnfrei.
Richtig. Habe ich in meinem Code gelöscht und funktioniert genau gleich
Ich sehe nicht so recht warum man eine Zeitverzögerung *und* `step` braucht. Was kann man damit effektiv erreichen was nur mit einer Zeitverzögerung nicht geht, ausser das Auf-/Abblenden weniger sanft zu gestalten. Will man das? Ausserdem wäre ich mal über eine Beschreibung in Worten zu `step` gespannt, und wie das zu dem Wort `step` passt. Im Grunde würde ich auch sagen das ist fehlerhaft, weil bei `step`-Werten bei denen die Division nicht glatt aufgeht, erreichen die LEDs nicht die maximale Helligkeit.
Egal wie, aber das dimmen ist nur in 255 Schritten möglich, (oder irre ich mich) ob ich "for j in range(int(256 // step)):" oder "for j in range(256):" rechnen - beides ergibt 0 ..... bis 255. Gedimmt wird von (1, 1, 1) bis (255, 255, 255)

Aber ein Problem in dem Script ist noch - ich muss die LED's mit (255, 255, 255) einschalten, da die LED's sonst bis zum Sonnenuntergang aus bleiben. Was habe ich da noch falsch?

So. Ich habe versucht einige Änderungen und Erklärungen und Hinweise umzusetzen und komme zum folgenden Script:

Code: Alles auswählen

import time
import Adafruit_WS2801
import Adafruit_GPIO.SPI as SPI

# LED Pixelanzahl
PIXEL_COUNT = 50

# Alternatively specify a hardware SPI connection on /dev/spidev0.0:
SPI_PORT   = 0
SPI_DEVICE = 0

def clear(pixels):
    pixels.clear()
    pixels.show()

def sonnenaufgang(pixels, delay=0.01):
    for j in range(256):
        for i in range(pixels.count()):
            r, g, b = pixels.get_pixel_rgb(i)
            r = max(0, r + 1)
            g = max(0, g + 1)
            b = max(0, b + 1)
            pixels.set_pixel(i, Adafruit_WS2801.RGB_to_color( r, g, b ))
        pixels.show()
        time.sleep(delay)
        

def sonnenuntergang(pixels, delay=0.01):
    for j in range(256):
        for i in range(pixels.count()):
            r, g, b = pixels.get_pixel_rgb(i)
            r = max(0, r - 1)
            g = max(0, g - 1)
            b = max(0, b - 1)
            pixels.set_pixel(i, Adafruit_WS2801.RGB_to_color( r, g, b ))
        pixels.show()
        time.sleep(delay)

def leuchtdauer(pixels, delay=10):
    for x in range(PIXEL_COUNT):                    #ich muss diesen Schritt einfügen
        pixels.set_pixel_rgb(x, 255, 255, 255)      #sonst würden die LED nach dem Aufgang wieder ausgehen,
    pixels.show()                                   #dadurch blinken die LES's kurz auf

    time.sleep(delay) #hiermit kann ich die Zeist Steuern zwischen den Auf- und Untergang  


def main():
    pixels = Adafruit_WS2801.WS2801Pixels(PIXEL_COUNT, spi=SPI.SpiDev(SPI_PORT, SPI_DEVICE))
    clear(pixels)    

    sonnenaufgang(pixels)
    leuchtdauer(pixels)
    sonnenuntergang(pixels)
    
    clear(pixels)
    print('ENDE')
        

if __name__ == "__main__":
    main()
CrazyAndy
User
Beiträge: 4
Registriert: Mittwoch 2. Januar 2019, 16:33

Aber ein Problem in dem Script ist noch - ich muss die LED's mit (255, 255, 255) einschalten, da die LED's sonst bis zum Sonnenuntergang aus bleiben. Was habe ich da noch falsch?
wenn der Zustand der LED's gespeichert werden kann und an der Stelle wieder mit dem Sonnenuntergang begonnen werden kann müsste es kein flackern (blinken) geben. ODER?
Benutzeravatar
__blackjack__
User
Beiträge: 13004
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

Die Frage nach den RGB-Werten am Ende des Sonnenaufgangs war nicht nach was sie sein sollten, oder was Dir reichen würde, sondern was sind das ganz konkret für Werte die da am Ende gesetzt werden. Das musst Du ja irgendwie nachvollziehen wenn zwischen Sonnenauf- und untergang die Lichter kurz ausgehen. Anhand des Codes ist mir das jedenfalls nicht klar. Darum dachte ich vielleicht ich sehe durch blosses ansehen etwas nicht, was man vielleicht mal konkret prüfen sollte. Vielleicht verhält sich `pixels.get_pixel_rgb()` ja nicht so wie ich mir das vorstelle und da kommen aus irgendeinem Grund andere Werte bei heraus.

Was eine Änderung von `n` bewirkt ist klar, aber das macht keinen Sinn da *zwei* Wege für zur Verfügung zu stellen. Und wenn einer der beiden Wege ein Argument ist, und der andere eine magische globale Variable `n`, dann gehört `n` da nicht hin.

Es ist nicht egal ist ob man ``for j in range(int(256 // step)):`` schreibt und immer einen `step`-Wert von 1 verwenden muss, oder ob man einfach ``for j in range(256):`` schreibt und sich `step` damit komplett spart. Das eine ist sinnlos und verwirrend, weil sich der Leser fragt was das soll, und ob das so kompliziert ist, das er einfach nur nicht verstanden hat, warum das da steht, weil es auf den ersten Blick so sinnlos ist. Auf den zweiten halt auch. Also sollte das da nicht stehen.

Und warum sind da immer noch diese komplizierten Schleifen mit auslesen der alten Pixelwerte, die man doch eigentlich kennt, und deswegen gar nicht auslesen bräuchte, und `r`, `g`, und `b` die doch aber alle immer den gleichen Wert haben, und dann `set_pixel()` mit der Umwandlungsfunktion für `r`, `g`, und `b` wo doch im gleichen Skript `set_pixel_rgb()` verwendet, und damit doch bekannt ist? Warum mal `PIXEL_COUNT` und mal `pixel.count()`? Warum eine Schleife mit `set_pixel_rgb()` um alle Pixel einzeln zu setzen, so es doch `set_pixels_rgb()` gibt, um alle auf einmal auf den gleichen Wert zu setzen?

Was die fettgedruckte Nachfrage angeht, wäre halt immer noch interessant wie die Werte die da in `sonnenaufgang()` gesetzt werden, tatsächlich aussehen. Ob da nicht irgendwas unvorhergesehenes passiert.
“Most people find the concept of programming obvious, but the doing impossible.” — Alan J. Perlis
CrazyAndy
User
Beiträge: 4
Registriert: Mittwoch 2. Januar 2019, 16:33

Was die fettgedruckte Nachfrage angeht, wäre halt immer noch interessant wie die Werte die da in `sonnenaufgang()` gesetzt werden, tatsächlich aussehen. Ob da nicht irgendwas unvorhergesehenes passiert.
Ich habe mir die Werte für 'r', 'g' und 'b' während dem durchlaufen anzeigen lassen ..... was soll ich sagen:
"for j in range(256):" gibt ja die Werte von 0 bis 255 aus, weiter in der Schleife lasse ich aber "r = max(0, r + 1)" rechnen und somit übergebe ich am Ende der Schleife in "pixels.set_pixel(i, Adafruit_WS2801.RGB_to_color( r, g, b ))" die Werte für 'r', 'g' und 'b' mit (256, 256, 256) und das ist falsch! (256, 256, 256) ist keine Farbe! Daher gehen die LED's einfach aus.

Richtig muss es heissen:

Code: Alles auswählen

def sonnenaufgang(pixels, delay=0.01):
    for j in range(255):
        for i in range(pixels.count()):
            r, g, b = pixels.get_pixel_rgb(i)
            r = max(0, r + 1)
            g = max(0, g + 1)
            b = max(0, b + 1)
            pixels.set_pixel(i, Adafruit_WS2801.RGB_to_color( r, g, b ))
        pixels.show()
        time.sleep(delay)
Ich benutze die Dokumentation zum Modul "Adafruit_Python_WS2801" von github:
https://github.com/adafruit/Adafruit_Py ... 28b5dab56f

OK, also vielen Dank. Durch Deine Hilfe bin ich darauf gestoßen. Nun ich bin noch lange nicht am Ende aber ein Anfang ist gemacht. Im Versuchsaufbau sieht das schon mal klasse aus.
nezzcarth
User
Beiträge: 1632
Registriert: Samstag 16. April 2011, 12:47

CrazyAndy hat geschrieben: Mittwoch 2. Januar 2019, 22:00 Daran habe ich auch schon gedacht - kann es sein dass wenn ein Script über Wochen läuft es dan zu fehlern kommen kann?
Allein durch das wiederholte Ausführen eines Skripts per Cron-Job kommt es i.d.R. nicht zu Fehlern. Falls es nach einer Weile zu Komplikationen kommt, liegt das entweder an externen Faktoren, oder daran, dass das Skript nicht robust programmiert ist.

Du müsstest dir nur eben darüber Gedanken machen, was z.B. passiert, wenn das Skript zwei mal gleichzeitig ausgeführt wird (und das ggf. verhindern, z.B. über eine Lockdatei), was passiert, wenn während der Ausführung der Stecker gezogen wird, oder inwiefern die beiden Modi von einander abhängig sind (d.h. was passiert, wenn wenn abends "Sonnenuntergang" ausgeführt werden soll, morgens "Sonnenaufgang" aber aus irgendeinem Grund nicht gelaufen ist?).

Die Variante, 9 Stunden lang per 'sleep' zu warten, finde ich konzeptuell nicht so günstig (außer evtl., du willst die Verzögerung dynamisch ändern können, um z.B. Jahreszeiten zu simulieren), weil man da eben einen Prozess hat, der 9 Stunden lang überhaupt nichts tut und mit dem man auch nicht mehr wirklich kommunizieren kann, um ihn zum Beispiel geordnet zu beenden (killen geht natürlich schon). Das lässt sich zwar einbauen (z.B. über Signal(handler) oder Sockets), ist aber vmtl. etwas überdimensioniert.
Benutzeravatar
__blackjack__
User
Beiträge: 13004
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

@nezzcarth: Man muss den Prozess ja nicht killen, SIGTERM oder SIGINT kann man ja auch senden. Insbesondere letzteres lässt sich per ``try``/``except`` leicht behandeln und zum Aufräumen verwenden.
“Most people find the concept of programming obvious, but the doing impossible.” — Alan J. Perlis
Benutzeravatar
__blackjack__
User
Beiträge: 13004
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

@CrazyAndy: Ich verstehe immer noch nicht warum Du das alles so kompliziert machst. Wenn die Funktion `sonnenaufgang()` startet, dann *muss* jedes Pixel den Wert 0,0,0 haben. Sonst macht die Funktion nicht was sie soll. Also warum dann die aktuellen Pixelwerte in jedem Schleifendurchlauf auslesen, wenn man die doch sowieso schon kennt? Das macht keinen Sinn.

Ebenfalls keinen Sinn macht das `max()` hier. Das würde nur etwas bewirken wenn `pixels.get_pixel_rgb()` negative Werte zurückgeben würde, was es aber nie tun sollte, denn dann wäre mit der Bibliothek was kaputt, oder man hat selbst vorher mal falsche Werte gesetzt.

Warum schon wieder/immer noch `set_pixel()` statt `set_pixel_rgb()`?

Unterm Strich bleibt da einfach nur noch das hier übrig:

Code: Alles auswählen

def sonnenaufgang(pixels, delay=0.01):
    for level in range(256):
        pixels.set_pixels_rgb(level, level, level)
        pixels.show()
        time.sleep(delay)
“Most people find the concept of programming obvious, but the doing impossible.” — Alan J. Perlis
nezzcarth
User
Beiträge: 1632
Registriert: Samstag 16. April 2011, 12:47

@__blackjack__:
Stimmt, die Idee mit dem SIGINT finde ich gut. Bei den anderen Signalen muss man sich ja ein bisschen (mehr) mit Signalprogrammierung befassen und einen Handler für (z.B.) SIGTERM registrieren, was vielleicht zum Einstieg etwas viel ist; das war, was in meinem letzten Satz u.a. meinte.
Antworten