Variable hat einen Wert, den ich ich nicht verstehe

[RLeoP]

Hallo
Bei meinem Code Versuche ich eine LED in Verschiedenen Farben leuchten lassen und das Licht dann später mit einem Schieberegler das Licht zu dimmen.
Zusätzlich soll unter dem Schieberegler die Farbe erscheinen, in der die LED leuchtet.
Jetzt zu meinem Problem, wenn ich eine Farbe eingestellt ist und ich dann die Helligkeit mit dem Regler verändere, passiert es manchmal, das eine der Variablen rfz2, gfz2, bfz2 kleiner als 0 ist, obwohl das vorher kontrolliert wird und wenn nötig auch der Wert verändert wird. Kann mir da bitte jemand weiterhelfen?
Danke schonmal im Vorraus

Code: Alles auswählen

import tkinter as tk
import tkinter.colorchooser as tkc
import pigpio

ColorCode = "#FFFFFF"
rpin = 22
gpin = 23
bpin = 24
r = 0
g = 0
b = 0
rval = 255
gval = 255
bval = 255
rval2 = 255
gval2 = 255
bval2 = 255

pi1 = pigpio.pi('IP-Adresse')        #IP-Adresse des Raspberry Pis
pi1.set_mode(rpin, pigpio.OUTPUT)
pi1.set_mode(gpin, pigpio.OUTPUT)
pi1.set_mode(bpin, pigpio.OUTPUT)
pi1.write(rpin, 1)
pi1.write(gpin, 1)
pi1.write(bpin, 1)


def RGB(rvar, gvar, bvar):
    pi1.set_PWM_dutycycle(rpin, rvar)
    pi1.set_PWM_dutycycle(gpin, gvar)
    pi1.set_PWM_dutycycle(bpin, bvar)
    
def RGB_LED():
    global ColorCode, r, g, b, rval, gval, bval, rval2, gval2, bval2
    color = tkc.askcolor(color=(r, g, b), title="RGB")
    ColorCode = color[1]
    c.create_rectangle(0,0,1000,1000, fill=ColorCode)
    rgb = color[0]
    rval = int((rgb[0]-255)*(-1))
    gval = int((rgb[1]-255)*(-1))
    bval = int((rgb[2]-255)*(-1))
    rval2 = int((rgb[0]-255)*(-1))
    gval2 = int((rgb[1]-255)*(-1))
    bval2 = int((rgb[2]-255)*(-1))
    r = rgb[0]
    g = rgb[1]
    b = rgb[2]
    if rval > 255:
        rval = 255
    if gval > 255:
        gval = 255
    if bval > 255:
        bval = 255
    if rval2 > 255:
        rval2 = 255
    if gval2 > 255:
        gval2 = 255
    if bval2 > 255:
        bval2 = 255
    if r > 255:
        r = 255
    else:
        r = int(round(r))
    if g > 255:
        g = 255
    else:
        g = int(round(g))
    if b > 255:
        b = 255
    else:
        b = int(round(b))
    RGB(rval, gval, bval)
    if r > g and r > b:
        w1.set(r)
    if r == g and r > b:
        w1.set(r)
    if r == b and r > g:
        w1.set(r)
    if r == g and r == b:
        w1.set(r)
    if g > r and g > b:
        w1.set(g)
    if g == b and g > r:
        w1.set(g)
    if b > r and b > g:
        w1.set(b)

ButCount1 = 0
def LEDOff():
    global ButCount1
    ButCount1 += 1
    if ButCount1 == 1:
        RGB(255,255,255)
        b2["text"]="LED on"
    if ButCount1 == 2:
        RGB(rval, gval, bval)
        b2["text"]="LED off"
        ButCount1 -= 2

def setBrightness(x):
    global r, g, b, ColorCode
    x = int(x)
    y = int((x-255)*(-1))
    lab1["text"]=x
    rz = int(rval2)
    gz = int(gval2)
    bz = int(bval2)
    rd = rz - y
    gd = gz - y
    bd = bz - y
    rz2 = int(r)
    gz2 = int(g)
    bz2 = int(b)
    rd2 = rz2 - x
    gd2 = gz2 - x
    bd2 = bz2 - x
    if r > g and r > b:
        rf = rz - rd
        gf = gz - rd
        bf = bz - rd
        rf2 = rz2 - rd2
        gf2 = gz2 - rd2
        bf2 = bz2 - rd2
    if r == g and r > b:
        rf = rz - rd
        gf = gz - rd
        bf = bz - rd
        rf2 = rz2 - rd2
        gf2 = gz2 - rd2
        bf2 = bz2 - rd2
    if r == b and r > g:
        rf = rz - rd
        gf = gz - rd
        bf = bz - rd
        rf2 = rz2 - rd2
        gf2 = gz2 - rd2
        bf2 = bz2 - rd2
    if r == g and r == b:
        rf = rz - rd
        gf = gz - rd
        bf = bz - rd
        rf2 = rz2 - rd2
        gf2 = gz2 - rd2
        bf2 = bz2 - rd2
    if g > r and g > b:
        rf = rz - gd
        gf = gz - gd
        bf = bz - gd
        rf2 = rz2 - gd2
        gf2 = gz2 - gd2
        bf2 = bz2 - gd2
    if g == b and g > r:
        rf = rz - gd
        gf = gz - gd
        bf = bz - gd
        rf2 = rz2 - gd2
        gf2 = gz2 - gd2
        bf2 = bz2 - gd2
    if b > r and b > g:
        rf = rz - bd
        gf = gz - bd
        bf = bz - bd
        rf2 = rz2 - bd2
        gf2 = gz2 - bd2
        bf2 = bz2 - bd2
    if rf < 0:
        rfz = 0
    else:
        rfz = rf
    if gf < 0:
        gfz = 0
    else:
        gfz = gf
    if bf < 0:
        bfz = 0
    else:
        bfz = bf
    if rf > 255:
        rfz = 255
    else:
        rfz = rf
    if gf > 255:
        gfz = 255
    else:
        gfz = gf
    if bf > 255:
        bfz = 255
    else:
        bfz = bf
    if rf2 < 0:
        rfz2 = 0
    else:
        rfz2 = rf2
    if gf2 < 0:
        gfz2 = 0
    else:
        gfz2 = gf2
    if bf2 < 0:
        bfz2 = 0
    else:
        bfz2 = bf2
    if rf2 > 255:
        rfz2 = 255
    else:
        rfz2 = rf2
    if gf2 > 255:
        gfz2 = 255
    else:
        gfz2 = gf2
    if bf2 > 255:
        bfz2 = 255
    else:
        bfz2 = bf2
    RGB(rfz, gfz, bfz)
    r = rfz2
    g = gfz2
    b = bfz2
    ccc(rfz2, gfz2, bfz2)
    c.create_rectangle(0,0,1000,1000, fill=ColorCode)
    
def ccc(r, g, b):
    global ColorCode
    x = str(hex(r))
    y = str(hex(g))
    z = str(hex(b))
    if len(x) == 3:
        x1 = "0" + str(x[2])
    else:
        x1 = str(x[2])+str(x[3])
    if len(y) == 3:
        y1 = "0" + str(y[2])
    else:
        y1 = str(y[2])+str(y[3])
    if len(z) == 3:
        z1 = "0" + str(z[2])
    else:
        z1 = str(z[2])+str(z[3])
    ColorCode = "#"+x1+y1+z1

master = tk.Tk()
master.title("LED Strip")
master.geometry("100x350")

lab1 = tk.Label(master)
lab1.pack()
w1 = tk.Scale(master, from_=255, to=0,orient=tk.VERTICAL, showvalue=0, length = 100, width = 30, command=setBrightness)
w1.pack()
b1 = tk.Button(master, text="RGB LED", command=RGB_LED)
b1.pack()
b2 = tk.Button(master, text="LED off", command=LEDOff)
b2.pack()

c = tk.Canvas(master, width=50, height=20)
c.pack()
c.create_rectangle(0,0,1000,1000, fill="#ffffff")
lab1["text"]=w1.get()

tk.mainloop()

[__blackjack__]

@RLeoP: Werd erst mal die globalen Variablen los. ``global`` hat in einem übersichtlichen Programm nix zu suchen. Dann die ganzen Code-Kopien mit den vielen abgezürzten Variablen. Das könnte beispielsweise eine Fehlerquelle sein, dass da irgendwo ein Tippfehler ist und gar nicht der Variablenwert geprüft wird der danach verändert wird.

Am besten trennt man auch Programmlogik und GUI. Und da man für die GUI sowieso mit Klassen arbeiten muss, kann man sich beispielsweise auch gleich mal eine Klasse schreiben die so eine RGB-Farbe modelliert und Operationen darauf bereit stellt.

`ccc()` sieht extrem umständlich aus. Ich vermute man da wird letztlich nichts anderes als das hier gemacht (ungetestet):

Code: Alles auswählen

def rgb_to_string(red, green, blue):
    return f"#{red:02x}{green:02x}{blue:02x}"

[RLeoP]

@RLeoP: Werd erst mal die globalen Variablen los. ``global`` hat in einem übersichtlichen Programm nix zu suchen. Dann die ganzen Code-Kopien mit den vielen abgezürzten Variablen. Das könnte beispielsweise eine Fehlerquelle sein, dass da irgendwo ein Tippfehler ist und gar nicht der Variablenwert geprüft wird der danach verändert wird.

Am besten trennt man auch Programmlogik und GUI. Und da man für die GUI sowieso mit Klassen arbeiten muss, kann man sich beispielsweise auch gleich mal eine Klasse schreiben die so eine RGB-Farbe modelliert und Operationen darauf bereit stellt.

`ccc()` sieht extrem umständlich aus. Ich vermute man da wird letztlich nichts anderes als das hier gemacht (ungetestet):

Code: Alles auswählen

def rgb_to_string(red, green, blue):
    return f"#{red:02x}{green:02x}{blue:02x}"

@__blackjack__: Danke für die Hilfe. Der Code den du geschrieben hast macht genau das, was ich will und den Vorschlag mit den Klassen versuche ich umzusetzen.

[Sirius3]

Statt dreimal den gleichen Code zu wiederholen, solltest Du mit Listen mit 3 Werten arbeiten.
Bei

Code: Alles auswählen

    if r > g and r > b:
        w1.set(r)
    if r == g and r > b:
        w1.set(r)
    if r == b and r > g:
        w1.set(r)
    if r == g and r == b:
        w1.set(r)
    if g > r and g > b:
        w1.set(g)
    if g == b and g > r:
        w1.set(g)
    if b > r and b > g:
        w1.set(b)

kann man die meisten Bedingungen zusammenfassen, z.B. mit >=, wenn sich die Bedingungen gegenseitig ausschließen benutzt man elif.

Code: Alles auswählen

    if r >= g and r >= b:
        w1.set(r)
    elif g >= b:
        w1.set(g)
    else:
        w1.set(b)

Wenn man das aber liest, dann steht da nichts anderes als „nimm den größten Wert von r, g oder b”:

Code: Alles auswählen

w1.set(max(r, g, b))

Aus RGB_LED wird dann sowas:

Code: Alles auswählen

RGB_PINS = [22, 23, 24]

def set_rgb(rgb_color):
    for pin, value in zip(RGB_PINS, rgb_color):
        pi1.set_PWM_dutycycle(pin, value)

def set_rgb_led(canvas, scale, rgb_color):
    rgb_color, color_code = tkc.askcolor(color=rgb_color, title="RGB")
    canvas.create_rectangle(0,0,1000,1000, fill=color_code)
    rgb_color_inverse = [min(255, int(255-v)) for v in rgb_color]
    rgb_color = [min(255, int(round(v))) for v in rgb_color]
    set_rgb(rgb_color)
    scale.set(max(rgb_color))

[RLeoP]

Statt dreimal den gleichen Code zu wiederholen, solltest Du mit Listen mit 3 Werten arbeiten.
Bei

Code: Alles auswählen

    if r > g and r > b:
        w1.set(r)
    if r == g and r > b:
        w1.set(r)
    if r == b and r > g:
        w1.set(r)
    if r == g and r == b:
        w1.set(r)
    if g > r and g > b:
        w1.set(g)
    if g == b and g > r:
        w1.set(g)
    if b > r and b > g:
        w1.set(b)

kann man die meisten Bedingungen zusammenfassen, z.B. mit >=, wenn sich die Bedingungen gegenseitig ausschließen benutzt man elif.

Code: Alles auswählen

    if r >= g and r >= b:
        w1.set(r)
    elif g >= b:
        w1.set(g)
    else:
        w1.set(b)

Wenn man das aber liest, dann steht da nichts anderes als „nimm den größten Wert von r, g oder b”:

Code: Alles auswählen

w1.set(max(r, g, b))

Aus RGB_LED wird dann sowas:

Code: Alles auswählen

RGB_PINS = [22, 23, 24]

def set_rgb(rgb_color):
    for pin, value in zip(RGB_PINS, rgb_color):
        pi1.set_PWM_dutycycle(pin, value)

def set_rgb_led(canvas, scale, rgb_color):
    rgb_color, color_code = tkc.askcolor(color=rgb_color, title="RGB")
    canvas.create_rectangle(0,0,1000,1000, fill=color_code)
    rgb_color_inverse = [min(255, int(255-v)) for v in rgb_color]
    rgb_color = [min(255, int(round(v))) for v in rgb_color]
    set_rgb(rgb_color)
    scale.set(max(rgb_color))

Danke für die vielen Tipps! Es hat jetzt endlich geklappt.

Code: Alles auswählen

import pigpio
import tkinter as tk
import tkinter.colorchooser as tkc

RGB_PINS = [22, 23, 24]
pi = pigpio.pi('IP-Adresse')
for pin in RGB_PINS:
    pi.set_mode(pin, pigpio.OUTPUT)
    pi.write(pin, 1)

master = tk.Tk()
master.title("RGB LED")
master.geometry("100x300")
lab1 = tk.Label(master)
lab1.pack()
w1 = tk.Scale(master, from_=255, to=0,orient=tk.VERTICAL, showvalue=0, length = 100, width = 30)
w1.pack()
b1 = tk.Button(master, text="RGB LED")
b1.pack()
b2 = tk.Button(master, text="LED off")
b2.pack()
c1 = tk.Canvas(master, width=50, height=20)
c1.pack()
lab2 = tk.Label(master)
lab2.pack()

scaleValx = [0, 0, 0]

class Color:
    def __init__(self, canvas, label, scale, colorCode_start):
        self.canvas = canvas
        self.label = label
        self.scale = scale
        self.color_code = colorCode_start
        self.rgb_color = [0, 0, 0]
    def dec_to_hexCode(self, red, green, blue):
        return f"#{red:02x}{green:02x}{blue:02x}"
    def size_adjustment_0_255(self, Value):
        self.rgb_color = []
        for i in Value:
            if i > 255:
                i = 255
                self.rgb_color.append(i)
            elif i < 0:
                i = 0
                self.rgb_color.append(i)
            else:
                self.rgb_color.append(i)
    def set_rgb_led(self):
        global scaleValx
        rgb_color, self.color_code = tkc.askcolor(color=self.color_code, title="RGB")
        self.setCanvas(self.color_code)
        self.size_adjustment_0_255(rgb_color)
        self.set_rgb(self.rgb_color)
        self.scale.set(max(self.rgb_color))
        self.label["text"]=max(self.rgb_color)
        scaleValx = self.rgb_color
        lab2["text"] = self.color_code
    def set_rgb(self, rgb_color):
        #for pin, value in zip(RGB_PINS, rgb_color):
        #    pi.set_PWM_dutycycle(pin, value)
        x = rgb_color
    def LED_on(self):
        self.set_rgb(self.rgb_color)
    def LED_off(self):
        for pin in RGB_PINS:
            pi.write(pin, 1)
        x = 1
    def setCanvas(self, color):
        self.canvas.create_rectangle(0, 0, 1000, 1000, fill=color)
    def Canvas_on(self):
        self.canvas.create_rectangle(0, 0, 1000, 1000, fill=self.color_code)
    def Canvas_off(self):
        self.canvas.create_rectangle(0, 0, 1000, 1000, fill="#000000")

color = Color(c1, lab1, w1, "#000000")
ButCount1 = 0
def LEDOffOn():
    global ButCount1
    ButCount1 += 1
    if ButCount1 == 1:
        color.Canvas_off()
        color.LED_off()
        b2["text"]="LED on"
    if ButCount1 == 2:
        color.Canvas_on()
        color.LED_on()
        b2["text"]="LED off"
        ButCount1 -= 2
def scaleChange(Value):
    global scaleValx, scaleValz
    lab1["text"]=Value
    dif = int(Value) - int(scaleValz)
    x = []
    for i in scaleValx:
        x.append(int(i)+int(dif))
    color.size_adjustment_0_255(x)
    color.set_rgb(color.rgb_color)
    y = color.dec_to_hexCode(color.rgb_color[0], color.rgb_color[1], color.rgb_color[2])
    color.setCanvas(y)
    color.color_code = y
    lab2["text"] = color.color_code
    scaleValx = x
    scaleValz = Value

#TKinter window command
w1["command"]=scaleChange
b1["command"]=color.set_rgb_led
b2["command"]=LEDOffOn

lab1["text"]=w1.get()
lab2["text"]=color.color_code
scaleValz = w1.get()
color.setCanvas("#000000")
tk.mainloop()
pi.stop()

Was haltet ihr von der Lösung und was könnte ich noch besser machen?

[__deets__]

Wurde alles schon erzaehlt, aber nicht beachtet: keine globalen Variablen benutzen, keinen Code auf Modul-ebene haben, auf objektorientierte Programmierung wechseln.

[RLeoP]

Wurde alles schon erzaehlt, aber nicht beachtet: keine globalen Variablen benutzen, keinen Code auf Modul-ebene haben, auf objektorientierte Programmierung wechseln.

Was meinst du mit "keinen Code auf Modul-ebene"?

[__deets__]

Code: Alles auswählen

print("das hier ist code auf modul ebene")

def eine_funktion():
    print("das hier ist KEIN code auf modul ebene")

# und hierdurch ist sichergestellt, dass das nur ausgefuehrt wird, wenn
# man will.
if __name__ == "__main__":
     eine_funktion()

[Sirius3]

In `size_adjustment_0_255` kann man wieder mit min und max einiges zusammenfassen. Variablen schreibt man klein. `i` ist ein schlechter Name für einen Wert, der kein Index ist. Einbuchstabige Variablennamen oder welche mit Nummern sind ebenso schlecht.

[__blackjack__]

@RLeoP: Das mit dem Code auf Modulebene wird auch umso unübersichtlicher je mehr man den zwischen Funktions- und Klassendefinitionen den verteilt. Man muss dann erst einmal den ganzen Code durchgehen um zu sehen was da eigentlich für globale Variablen existieren und wo und wie sie definiert werden. Also zusätzlich zu dem Problem das es überhaupt globale Variablen gibt, man also auch *in* allen Funktionen und Methoden den Code durchgehen muss wenn man wissen will von wo aus die wie manipuliert werden. Funktionen und Methoden sollten alles was sie ausser Konstanten benötigen als Argument(e) übergeben bekommen und keine globalen Variablen verändern. Denn nur dann kann man sich eine Funktion/Methode einzeln anschauen ohne zu viel Wissen über das gesamte Programm haben zu müssen.

Namen werden in Python klein_mit_unterstrichen geschrieben. Ausnahmen sind Konstanten (KOMPLETT_GROSS) und Klassen (MixedCase).

Die Klasse macht IMHO so keinen Sinn. Der Name passt nicht weil so ein Objekt nicht (nur) eine Farbe repräsentiert sondern auch einen Teil der GUI. Eine Klasse für eine Farbe mit Operationen auf dieser Farbe und ohne irgendeinen direkten Bezug zur GUI würde Sinn machen. Also die Programmlogik getrennt von der GUI. Was bei so einem Objekt Sinn machen würde, wäre eine `__str__()`-Methode, welche die Zeichenkette im "#rrggbb"-Format darstellt, weil man diese Objekte dann Tk einfach direkt als Farbe übergeben kann, denn `tkinter` wandelt alles in Zeichenketten auf dem Weg zu Tk, denn Tcl/Tk kennt im Grunde nur Zeichenketten. Sozusagen „stringly typed“.

Die Klasse enthält ”Methoden” die keine sind, sondern nur Funktionen die in die Klasse gesteckt wurden: `dec_to_hexCode()`, `set_rgb()`, `LED_off()`. Wobei die letzten beiden `pi` als Argument benötigen. Hier würde es sich anbieten die LED als Klasse zu modellieren die Pinnummern und das `pi`-Objekt kapselt.

Redundante Daten in verschiedenen Formaten vorzuhalten ist keine gute Idee weil das fehleranfällig ist. `color_code` und `rgb_color` sind Attribute die beide immer die gleiche Farbe beschreiben (sollten). Also braucht man nur eines davon speichern weil man das andere jederzeit daraus berechnen kann wenn man es benötigt. Wenn man eine Farbe als Klasse modelliert ist das auch kein Problem. Intern arbeitet man mit RGB-Werten und wie schon gesagt: die Zeichenkettendarstellung von so einem Objekt bietet sich für den Farbcode an. So eine Farbklasse würde ich als Werttyp schreiben, also einen wo die Operationen das Objekt nicht verändern sondern jeweils ein neues verändertes Objekt liefern. Dann bietet es sich an mit Operatorüberladung zu arbeiten und beispielsweise ``+`` und ``-`` zu implementieren.

Wenn man `size_adjustment_0_255()` nicht auf einer Liste mit mehreren Elementen sondern auf nur einem Wert definiert, ist der übliche Name für diese Operation `clamp()`. Die Zahlen sollten auch eher nicht im Namen stehen sondern in der Dokumentation der Funktion und/oder Argumente sein.

`Canvas_on` und `Canvas_off` haben fast den gleichen Code wie `setCanvas()`. Die beiden ersten Methoden kann man durch den Aufruf von `setCanvas()` implementieren um die Codewiederholung zu vermeiden. Hier liegt aber auch ein Programmierfehler vor: Es werden immer wieder neue Rechtecke erzeugt die alle übereinander ”gemalt” werden. `Canvas` ist aber Vektorgrafik und keine Pixelgrafik, das heisst jedes neue Rechteck verändert nicht einfach nur die Pixel die es abdeckt sondern alle vorherigen Rechtecke werden auch immer wieder und wieder übereinander ”gemalt” und das werden immer mehr und mehr und das Programm damit mit der Zeit immer langsamer. Man würde nur *ein* Recheck erstellen, sich dessen ID merken und von dem *einen* Rechteck dann immer nur die Farbe ändern. Die Kombination `Canvas`/Rechteck ist aber auch unnötig komplex, denn eigentlich würde hier auch ein `Frame` ausreichen dessen Hintergrundfarbe man entsprechend setzt/ändert.