@tryanderror: Anmerkungen zum Code:
Das `test_` bei `test_window` ist nicht wirklich nötig. Es ist das einzige Fenster in dem Namensraum und `Test` ist bereits Bestandteil des Namens der Klasse.
Objekthierarchien wie man sie bei GUIs verwendet sollten in der Regel nur in eine Richtig traversiert werden: vom Meister zu den Kindern. Wenn Kinder auf die Meister oder gar auf den Meister vom Meister zugreifen wird das unübersichtlich und unflexibel und damit fehleranfälliger und schwerer zu verändern. Kinder haben nichts bei den Meistern herum zu pfuschen sondern bieten die Möglichkeit Rückruffunktionen für Informationen in die Gegenrichtung. Ich würde das auch nur bei GUI-Widgets machen, denn bei einem Objekt in einem `tkinter` benutzenden Programm bedeutet für die meisten Leser ein `master`-Attribut, dass es sich um ein `Widget`-Objekt handelt.
Python hat einen Typ für Wahrheitswerte (`bool`) mit den Werten `True` und `False`. Dazu sollte man nicht die Zahlen 1 und 0 missbrauchen.
`tkinter` hat Konstanten für Zeichenketten die für Tk eine besondere Bedeutung haben wie "end", "normal" oder "disabled". Dann weiss der Leser das es sich nicht um eine beliebige Zeichenkette handelt und man bekommt von Python einen `NameError` wenn man sich vertippt bevor der falsche Wert an Tk übergeben wird.
Die beiden `Button`-Attribute hat anscheinend Yoda benannt.
Ein Objekt sollte nach Ablauf der `__init__()`-Methode vollständig initialisiert sein, also insbesondere alle Attribute sollten existieren. Die `Queue` gehört also auch in die `__init__()`.
Der Start-Knopf sollte nach dem Start deaktiviert werden sonst kann der Benutzer den mehrfach betätigen und immer neue Threads starten.
Dem `Thread` würde ich noch das Argument ``deamon=True`` mitgeben, sonst endet das Programm erst wenn dieser Thread auch beendet ist.
`can_thread` ist keine Methode sondern eine normale Funktion die in die Klasse gesteckt wurde. Warum? Falls das Absicht und kein Versehen war, sollte man da wenigstens eine `staticmethod()` draus machen, damit der Leser sieht, dass es Absicht war.
Die ``for``-Schleife über einen Laufindex um den dann für den Zugriff auf die einzelnen Elemente einer Sequenz zu verwenden ist in Python ein „anti pattern“. Man *direkt* über die Elemente von Sequenzen iterieren, ohne den Umweg über einen Index. Falls man *zusätzlich* eine laufende Zahl benötigt, gibt es die `enumerate()`-Funktion. Allerdings wird der Index hier gar nicht wirklich benötigt. Der wird zwar mit in die Queue gesteckt, aber an der Stelle wo aus der Queue gelesen wird, ignoriert der Code den Index einfach.
`item` ist kein guter Name für einen Laufindex.
Statt etwas durch einen gannzahligen Wert zu teilen und das Ergebnis in `int()` umzuwandeln, böte sich ganzzahlige Teilung an.
Falls der `stop_CAN()`-Aufruf wichtig ist, sollte man den vielleicht in einem ``finally``-Block stecken.
`string` in der `update()`-Methode stimmt als Name nicht, weil da ein Tupel dran gebunden wird und keine Zeichenkette.
Code: Alles auswählen
#!/usr/bin/env python3
import threading
import tkinter as tk
from functools import partial
from queue import Queue
import can_com
...
def can_thread(queue, commands):
can_com.start_BUS()
can_com.set_bus_filter(0x680, 0x710)
for command in commands:
data = command.SID + command.DID
response = can_com.talk_with_device(
command.CANID, int(command.Node, 16), len(data) // 2, data, 0
)
queue.put(f"{data} => {response}")
#
# TODO If this call is important, maybe ensure it with a ``finally`` block.
#
can_com.stop_CAN()
class TestGui:
def __init__(self, master, commands, on_close):
self.commands = commands
self.on_close = on_close
self.queue = Queue()
self.window = tk.Toplevel(master)
self.window.geometry("500x700")
self.window.resizable(False, False)
self.window.protocol("WM_DELETE_WINDOW", self.close_window)
self.listbox = tk.Listbox(self.window, state=tk.NORMAL)
self.listbox.pack(fill=tk.BOTH, expand=True)
self.start_button = tk.Button(
self.window, text="START", command=self.start_test
)
self.start_button.pack()
self.check_button = tk.Button(
self.window, text="CHECK", state=tk.DISABLED
)
self.check_button.pack()
def update(self):
while not self.queue.empty():
self.listbox.insert(tk.END, self.queue.get())
self.listbox.see(tk.END)
self.listbox.after(10, self.update)
def start_test(self):
#
# TODO Re-Enable the button after the thread did its work.
#
self.start_button["state"] = tk.DISABLED
threading.Thread(
target=partial(can_thread, self.queue, self.commands), daemon=True
).start()
self.update()
def close_window(self):
self.on_close()
self.window.destroy()
