Frage zur tkinter Multithreading Wertuebergabe

[Tylon]

Hallo zusammen,

ich bin neu im Forum und blutiger Programmiereinsteiger. Derzeit versuche ich mir eine Klasse zu erstellen welche es mir ermoeglicht
ein anderes Programm durch ein kleines Kontrollwidget zu erweitern. Derzeit scheitere ich daran das ich ueber eine Pingabfrage, welche in endlosschleife in einem gesonderten
Threat laeuft, im Mainthread die Farbe eines Labels zu wechseln. Quasi als eine art Statusabfrage. Leider will mir das ueberschreiben der Methode "__wgd_label_client_status" nicht gelingen. Vielleicht kann mir ja wer einen Tipp geben. Leider waren andere Forenbeitraege nicht sehr hilfreich fuer mich da ich mangels Erfahrung noch nicht so gut adaptieren kann.
Ich bedanke mich schon mal im vorraus.
Gruesse!

P.S.: Amerikanische Tastatur, daher das Umlaut geschreibe.

Code: Alles auswählen

from time import sleep
import tkinter as tk
import os
import time
import threading

iplist = ["192.168.100.1", "192.168.100.2"]

class widget_ui(threading.Thread):

    def __init__(self,space,xcoord,ycoord,ip):
        #super().__init__(ip)
        self.space = space
        self.xcoord = xcoord
        self.ycoord = ycoord
        self.ip = ip
        self.bgclient = self.run()
        
    def wgd_rahmen(self):
        self.wgd_rahmen = tk.Frame(self.space, width=130, height=390, highlightthickness=1, highlightbackground="white", bg="#313335").place(x=self.xcoord, y=self.ycoord)
        self.colorvar = ""
        self.__clientname()
        self.__wgd_label_client_beschriftung()
        self.__wgd_label_client_status()
        self.__wgd_label_content_beschriftung()
        self.__wgd_label_content_status()
        self.__wgd_button()

    def __clientname(self):
        self.__clientname = tk.Label(self.space, text ="Clientname", bg="#313335", fg="white", font=("Segoe ui",13)).place(x=self.xcoord+8, y=self.ycoord+15)

    def __wgd_button(self):
        self.__wgd_button = tk.Button(self.space, width=11, fg="white", height=5, text="Reboot", bg="#444441").place(x=self.xcoord+7, y=self.ycoord+285)

    def __wgd_label_client_beschriftung(self):
        self.__wgd_label_client_beschriftung = tk.Label(self.space, text ="Clientstatus", bg="#313335", fg="white", font=("Segoe ui",11)).place(x=self.xcoord+19, y=self.ycoord+125)

    def run(self):
        while True:
            if os.system("ping -c 1 " + self.ip) == 0:
                return True
            else:
                return False
            time.sleep(10)
    
    def __wgd_label_client_status(self):
        
        self.__wgd_label_client_status = tk.Label(gui, width=14, height=2, command = threading.Thread(target=self.run).start())
        self.__wgd_label_client_status.place(x=self.xcoord+7, y=self.ycoord+80)
        self.__wgd_label_client_status
        if self.bg_client == True:
            self.__wgd_label_client_status.config(bg="green")
        else:
            self.__wgd_label_client_status.config(bg="red")

    def __wgd_label_content_beschriftung(self):
        self.__wgd_label_content_beschriftung = tk.Label(self.space, text ="Contentstatus", bg="#313335", fg="white", font=("Segoe ui",11)).place(x=self.xcoord+10, y=self.ycoord+205)

    def __wgd_label_content_status(self):
        self.__wgd_label_content = tk.Label(gui, width=14, height=2,bg="red").place(x=self.xcoord+7, y=self.ycoord+160)

gui = tk.Tk()
gui.geometry("1600x900")

frame = tk.Frame(width = 1400, height = 800, bg="#2b2b2b").place(x = 50, y = 50 )

r = widget_ui(gui,55,55,iplist[0])

r.wgd_rahmen()

gui.mainloop()

[__blackjack__]

@Tylon: Ähm, also Du hast ja echt ein Talent Dir Deine Attribute zu zerschiessen. So ziemlich jede Methode überschreibt sich da selbst mit einem anderen Wert.

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

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

Die doppelten führenden Unterstriche sollten *einfache* führende Unterstriche sein. Doppelte braucht man selten bis gar nicht.

Die Klasse ist aber insgesamt falsch. Ein Widget ist kein Thread, die Vererbung ist falsch. Die `__init__()` der Basisklasse wird nicht aufgerufen. Die `start()`-Methode wird nie aufgerufen. Dass das Objekt ein `Thread` ist, wird also überhaupt gar nicht benutzt.

Die ``while``-Schleife in der `run()`-Methode ist sinnlos weil die auf jeden Fall gleich im ersten Durchlauf durch ein ``return`` abgebrochen wird. Der Rückgabewert landet aber im ”Nichts”. Der Code der `run()` Aufruft weiss nix damit anzufangen und kann den selbst ja auch gar nicht irgend wohin zurück geben, weil da von Dir aus ja ein asynchroner Aufruf dahinter steckt der sofort zurück kehrt. Selbst wenn das weitergereicht werden könnte: Aufgerufen wird das von der GUI-Hauptschleife, die auch mit keinem Rückgabewert rechnet und damit einfach nichts machen würde.

`os.system()` sollte man nicht verwenden. Die Dokumentation verweist auf das `subprocess`-Modul.

Das `time.sleep()` ist toter Code, der kann niemals ausgeführt werden.

Das `bg_client` wird nirgends definiert.

Attribute sollte nach Ablauf der `__init__()` definiert sein. Man führt keine neuen Attribute in irgendwelchen Methoden ein.

Funktions- und Methodennamen beschreiben üblicherweise die Tätigkeit die durchgeführt wird.

Namen sollten keine Grunddatentypen enthalten und keine kryptischen Abkürzungen. Oder gar nur aus einer kryptischen Abkürzung bestehen. Negativrekord ist hier `r` wird ein `WidgetUi`-Objekt. Warum `r`? Da kann man ja nicht mal raten wie man *darauf* kommen soll.

`place()` und `geometry()` sollte man nicht verwenden. Das macht alles viel unnötige Arbeit und funktioniert dann am Ende doch nicht wirklich portabel.

[Tylon]

@ Blackjack Danke erstmal fuer die Hinweise. place() und geometry() habe ich nur verwendet weil das Programm in welches dieser Code implementiert werden soll schon so gestaltet worden ist. ich verstehe aber natuerlich das Argument. Es ist nur ein Testfile welches irgendwann mal in den Originalcode implementiert werden soll. Der Grundgedanke der Klasse sollte sein das man dass Widget einfach mit einem Befehl aufrufen kann und es komplett erstellt wird weil man es in meinem Fall zwanzig mal braucht. Ich habe die Geschichte mit dem Threading als einzelne Klasse schon probiert und getestet ich krieg sie halt leider nur nicht in die Widget Klasse integriert. Ich werde deine hinweise Morgen mal in aller Ruhe durcharbeiten. Danke schon mal fuer die Tipps!

[Tylon]

Hallo zusammen,

hab das Script jetzt mal ueberarbeitet und hoffe es ist so richtiger. Aufgrund deiner Hinweise _blackjack_ hab ich jetzt auch die Pingabfrage zum laufen bekommen. jetzt versuche ich eine Abfrage zu implementieren die den Clieneten ( in dem Fall ein Raspberry Pi ) abfragt ob dessen omxplayer prozess laeuft. leider wieder ohne Erfolg. Das sei aber ersteinmal dahin gestellt, ich finde schon einen weg das es funktioniert. Viel mehr wuerde mich interessieren wie ich das Script im allgemeinen noch verbessern kann. Wie oben schon erwaehnt muss ich .place() nehmen wenn ich das Originalprogramm nicht komplett umschreiben mochte.

Danke fuer euren Input!

Code: Alles auswählen

rom subprocess import STDOUT
from time import sleep
import tkinter as tk
import subprocess
import time
import threading
import paramiko

iplist = ["192.168.100.1", "192.168.100.2"]

class Widget_Ui(threading.Thread):

    def __init__(self,space,xcoord,ycoord,ip):
        self.space = space
        self.xcoord = xcoord
        self.ycoord = ycoord
        self.ip = ip
        self.ping = ['ping', "-c", '1', self.ip]
        
        ssh = paramiko.SSHClient()
        ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
        ssh.connect(hostname=self.ip, username='pi', password='pi', port=22)
        self.clientabfrage = ssh.exec_command('ps -fC omxplayer')

        self.wgd_rahmen = tk.Frame(self.space, width=130, height=390, highlightthickness=1, highlightbackground="white", bg="#313335")
        self.wgd_rahmen.place(x=self.xcoord, y=self.ycoord)
        
        self.clientname = tk.Label(self.space, text ="Clientname", bg="#313335", fg="white", font=("Segoe ui",13))
        self.clientname.place(x=self.xcoord+8, y=self.ycoord+15)

        self.wgd_button = tk.Button(self.space, width=11, fg="white", height=5, text="Reboot", bg="#444441")
        self.wgd_button.place(x=self.xcoord+7, y=self.ycoord+285)

        self.wgd_label_client_beschriftung = tk.Label(self.space, text ="Clientstatus", bg="#313335", fg="white", font=("Segoe ui",11))
        self.wgd_label_client_beschriftung.place(x=self.xcoord+19, y=self.ycoord+125)

        self.wgd_label_client_status = tk.Label(self.space, width=14, height=2, command = threading.Thread(target=self.run).start())
        self.wgd_label_client_status.place(x=self.xcoord+7, y=self.ycoord+80)

        self.wgd_label_content_beschriftung = tk.Label(self.space, text ="Contentstatus", bg="#313335", fg="white", font=("Segoe ui",11))
        self.wgd_label_content_beschriftung.place(x=self.xcoord+10, y=self.ycoord+205)

        self.wgd_label_content = tk.Label(self.space, width=14, height=2, command = threading.Thread(target=self.call_process_on_client).start())
        self.wgd_label_content.place(x=self.xcoord+7, y=self.ycoord+160)

    
    def run(self):
        while True:
            if subprocess.call(self.ping) == 0:
                self.wgd_label_client_status.config(bg="green")
            else:
                self.wgd_label_client_status.config(bg="red")
            time.sleep(10)

    def call_process_on_client(self):
        while True:
            if self.clientabfrage == True:
                self.wgd_label_content.config(bg='green')
            else:
                self.wgd_label_content.config(bg='red')
            time.sleep(10)


def main():
    gui = tk.Tk()
    gui.geometry("1600x900")

    frame = tk.Frame(width = 1400, height = 800, bg="#2b2b2b").place(x = 50, y = 50 )

    widget_one = Widget_Ui(gui,55,55,iplist[0])
    #widget_two = Widget_Ui(gui,200,55,iplist[1])


    gui.mainloop()



main()

[__blackjack__]

@Tylon: Die Klasse erbt ja immer noch von `Thread`, was immer noch keinen Sinn macht.

Die `run()`-Methode wird nicht erst auf Knopfdruck gestartet sondern sofort, bedingungslos. Ausserdem ist der Name schlecht und auch noch irreführend im Zusammenhang mit dem erben von `Thread`. Das hat ja nichts mit der `Thread.run()`-Methode zu tun, sieht aber extrem danach aus.

In dem Thread veränderst Du die GUI, was falsch ist. Die GUI darf man nur von dem Thread aus verändern in dem die `mainloop()` läuft. Zwischen Threads kommuniziert man dann am besten mit einer `Queue` und die Fragt man im GUI-Thread regelmässig mit der `Widget.after()`-Methode auf ab.

Das gleiche gilt für `call_process_on_client()`, wobei das als Thread ziemlich sinnfrei ist, weil sich der Wert von dem Attribut ja nie ändert.

Was soll eigentlich dieser kryptische `wdg_*`-Präfix bedeuten? Und man muss da wohl auch nicht alles an Attribute binden.

`frame` in der `main()`-Funktion wird an den Wert `None` gebunden. Was wenig Sinn macht. Wird ja auch nirgends verwendet.

[Sirius3]

Was soll denn das wgd bedeuten? Benutze keine kryptischen Abkürzungen.
Dass Du von threading.Thread erbst, ist immer noch unsinnig.
Nicht alles muß man an Argumente binden, vor allem nicht Labels und Buttons, auf die man nie wieder zugreifen muß. Labels haben kein command-Argument. Und Thread.start liefert auch kein Funktionsobjekt zurück. Aus Threads heraus darf man keine GUI-Elemente verändern, dazu braucht man Queues und after.
Also statt etwas zu "verbessern" wäre der erste Schritt, ein funktionierendes Skript, das auch läuft.

[Tylon]

@ Sirius 3 wgd ist eine Abkuerzung fuer Widget. wenn ihr jetzt alle sagt das es unverstaenlich ist werde ich das aendern. Was ich hingegen nicht verstehe ist die Kritik mit dem laufenden Script denn mal abgesehen von der call_process_on_client() Methode (von der ich ja schrieb das ich damit aktuell noch ein Verstaendnissproblem habe.) tut es das ja und zwar genau so wie es soll. Wieso darf ich aus einem Thread heraus keine GUI Elemente veraendern? Wie gesagt ich bin Programmier noob allerdings erschliesst sich mir jetzt erstmal nicht wie ich es anders realiesieren soll. Auf jeden Fall werde ich mich jetzt ersteinmal mit Queues und after beschaeftigen. danke fuer den Tipp!


P.S.: Danke fuer die ausfuehrliche Antwort @ _blackjack_

[__blackjack__]

@Tylon: Nicht alles was ”funktioniert” tut das auch weil das so garantiert ist. Gerade bei nebenläufiger Programmierung kann ein Programm auch ”funktionieren” bis es halt doch irgendwann mal auf die Nase fällt. Und das nichtdeterministisch; also alles ist genau so wie bei den letzten Läufen wo es ”funktioniert” hat, und trotzdem kracht es dieses mal. GUI-Rahmenwerke sind in der Regel nicht threadsicher und man darf nur von einem Thread aus auf die GUI zugreifen. Sonst kann es irgendwann einmal passieren das zwei Threads so ungünstig etwas machen, dass sie sich gegenseitig in die Queue kommen. Und dann kann *alles* passieren. Von komischem Verhalten bis hin zu harten Abstürzen.