@marouensassi: Warum ist das denn so komisch eingerückt? Da ist zu viel Einrückung vor jeder Zeile und eingerückt wird mit vier Leerzeichen pro Ebene.
Namen sollten nicht kryptisch abgekürzt werden und einbuchstabige Namen sind in aller Regel gar nicht gut. Das geht wenn der Gültigkeitsbereich auf einen Ausdruck beschränkt ist, oder für `i`, `j`, `k` wenn das ganzzahlige Indexwerte sind, oder `x`, `y`, `z` für Koordinaten. Aber sicher nicht für das `time`-Modul das modulweit sichtbar ist.
Für den Zweck würde man auch nicht die `time.time()`-Funktion verwenden, sondern `time.monotonic()`.
Sternchen-Importe sind Böse™. Bei `tkinter` holt man sich so fast 200 Namen ins Modul von denen nur ein kleiner Bruchteil verwendet wird. Nicht nur Namen die in `tkinter` definiert werden, sondern auch welche die `tkinter` selbst von woanders importiert.
Auf Modulebene gehört nur Code der Konstanten, Funktionen, und Klassen definiert. Das Hauptprogramm steht üblicherweise in einer Funktion die `main()` heisst.
Literale Zeichenketten sind keine Kommentare, dafür ist ``#`` da. Diese komischen Linienkommentare sollte man sich auch sparen. Insgesamt sind die meisten Kommentare in dem Quelltext überflüssig. Einige sogar irreführend oder gar falsch. Faustregel: Kommentare erklären nicht was der Code macht, denn das steht da ja bereits als Code, sondern warum er das so macht, sofern das nicht offensichtlich ist.
Kommentare die einen schlechten oder unpassenden Namen erklären sollte man durch eine bessere Namensgebung beseitigen.
Bei Funktionsdefinitionen kommt zwischen Funktionsname und öffnender Klammer der Argumentliste kein Leerzeichen. Insgesamt ist das mit Leerzeichen und auch -zeilen ziemlich uneinheitlich.
Man sollte keine Warnungen unterdrücken sondern deren Ursache beseitigen. Das heisst vor dem Ende des Programms `gpio.cleanup()` aufrufen. Was man am besten mit einem ``try``/``finally`` sicherstellt.
Für magische Zahlen wie Pinnummern definiert man am besten eine Konstante, damit man die a) leicht ändern kann, und b) überall statt einer nichtssagenden Zahl ein verständlicher Name steht.
Namen werden in Python klein_mit_unterstrichen geschrieben. Ausnahmen sind Konstanten (KOMPLETT_GROSS) und Klassen (MixedCase).
`Funktion_knopf` ist nirgends definiert. Das sollte `GPIO_TASTER` sein‽
Threads sollte man in der Regel die `daemon`-Option mitgeben, sonst lässt sich das Programm nicht so leicht beenden.
Wenn man Namen durchnummeriert will man sich entweder bessere Namen ausdenken, oder gar keine Einzelwerte sondern eine Datenstruktur. Oft eine Liste.
Ein Grid-Layout macht hier nicht wirklich Sinn wenn alle Elemente in Spalte 2 untereinander landen. Man sollte auch keine Spalten oder Zeilen frei lassen.
Alles was eine Funktion oder Methode ausser Konstanten benötigt, wird als Argument(e) übergeben. ``global`` hat in einem normalen Programm nichts zu suchen. Wenn man sich Zustand über Funktionsaufrufe hinweg merken möchte und das nicht über Rückgabewerte geregelt werden kann, braucht man objektorientierte Programmierung (OOP). Bei `tkinter` kann man sich bis zu einem gewissen Grad noch mit `tkinter.Variable`-Objekten und `functools.partial()` durchmogeln, aber letztlich braucht man für jede nicht-triviele GUI OOP.
Mich machen ja diese ganzen `notaus1`, `notaus2`, `diff`, `diff2` wuschig. Und dann gibt es auch noch `diff2` und `diff_2` in *einer* Funktion. Da besteht hochgradig Verwechslungsgefahr beziehungsweise vermute ich mal ganz stark das `diff_2` eigentlich `diff2` heissen sollte‽ Und die beiden `diff`\s müssen die wirklich global sein oder als Argumente übergeben werden? Im Moment gehen die beiden Aktionen die man mit den Buttons anstossen kann ja nur *einmal*, beziehungsweise bis `diff` und `diff2` ihren jeweiligen höchstwert erreicht haben, denn die werden ja dann nie wieder unter diesen Wert gesetzt.
Das aus Python-Tupeln in Tcl Listen werden und die wiederrum als Zeichenkette interpretiert zu den einzelnen Bestandteilen durch ein Leerzeichen getrennt werden ist für Python-Programmierer eher undurchsichtig. Da sollte man auf Python-Seite die Zeichenkette mit Python-Mitteln formatieren.
Die beiden `Run()`-Funktionen sollten nicht verschachtelt definiert sein, und dann auch bessere Namen als `Run()` und `Run2()` haben. Zudem wurde ja schon angemerkt, dass man die GUI nicht von anderen Threads aus manipulieren darf, also sollten hier keine Threads sondern auch `after()` verwendet werden.
Die Frage ist auch ob das überhaupt zwei Funktionen sein sollten, denn die sehen nahezu gleich aus. Man könnte also auch *eine* Funktion schreiben die entsprechend parametriert wird.
Im Grunde sind die beiden Startfunktionen auch sehr symmetrisch und unterscheiden sich nur durch Werte, aber es gibt so schon genug/zu viele Argumente. Das will man wirklich sinnvoll auf Klassen aufteilen.
Was auch ziemlich schräg ist sind die Werte die `notaus1` und `notaus2` annehmen können. `notaus1` kann die Werte 0, 0.1, und 1 annehmen, und `notaus2` die Werte 0, 0.2, und 1. Mit den Zahlen wird nirgends gerechnet und die werden nirgends angezeigt und es wird nur auf Ungleichheit gegen 0.1 beziehungsweise 0.2 getestet. Was soll das denn bedeuten? Kann es sein, dass das einfach nur Wahrheitswerte sein sollten statt so komischer Zahlen‽
Zwischenstand:
Code: Alles auswählen
#!/usr/bin/env python3
import tkinter as tk
from functools import partial
from time import monotonic
from RPi import GPIO as gpio
BUTTON_PIN = 16
def update_status_bar(statusbar):
#
# FIXME This does not work in a GUI as a thread. Use callbacks from `gpio`,
# a `queue.Queue` and `after()` to communicate with the main thread.
#
while gpio.input(BUTTON_PIN):
statusbar["text"] = "Button gedrückt"
def _run(
time_label,
notaus,
diff,
start_time,
notaus_value,
max_diff,
diff_offset,
label_prefix,
):
if notaus.get() == notaus_value and diff.get() < max_diff:
end_time = monotonic()
diff.set(int(end_time - start_time) + diff_offset)
time_label["text"] = f"{label_prefix}: {diff.get()}"
time_label.after(
100,
_run,
time_label,
notaus,
diff,
start_time,
notaus_value,
max_diff,
diff_offset,
label_prefix,
)
def start_sensor_test(notaus1, notaus2, diff, statusbar, time_label):
notaus1.set(0.1)
statusbar["text"] = "Start Test"
if notaus2.get() != 0.2:
_run(
time_label, notaus1, diff, monotonic(), 0.1, 15, 0, "Test time is"
)
def start_wischen(notaus1, notaus2, diff, statusbar, time_label):
notaus2.set(0.2)
statusbar["text"] = "Start Wischen"
if notaus1.get() != 0.1:
_run(
time_label,
notaus2,
diff,
monotonic(),
0.2,
23,
1000,
"Time wischen is",
)
def on_stop(notaus1, notaus2, statusbar):
notaus1.set(1)
notaus2.set(1)
statusbar["text"] = "STOP"
def main():
try:
gpio.setmode(gpio.BCM)
gpio.setup(BUTTON_PIN, gpio.IN, pull_up_down=gpio.PUD_DOWN)
window = tk.Tk()
window.title("GUI")
diff = tk.IntVar(value=0)
diff2 = tk.IntVar(value=0)
notaus1 = tk.DoubleVar(value=0)
notaus2 = tk.DoubleVar(value=0)
pack_options = {"padx": (100, 5), "pady": 5}
sensor_test_button = tk.Button(window, text="Sensor Test")
sensor_test_button.pack(**pack_options)
start_wischen_button = tk.Button(window, text="Start wischen")
start_wischen_button.pack(**pack_options)
cancel_button = tk.Button(window, text="Abbrechen")
cancel_button.pack(**pack_options)
label_options = {
"width": 25,
"height": 5,
"borderwidth": 15,
"relief": tk.SUNKEN,
}
statusbar = tk.Label(window, text="auf start drücken", **label_options)
statusbar.pack()
time_label = tk.Label(window, text="Time", **label_options)
time_label.pack()
sensor_test_button["command"] = partial(
start_sensor_test, notaus1, notaus2, diff, statusbar, time_label
)
start_wischen_button["command"] = partial(
start_wischen, notaus1, notaus2, diff2, statusbar, time_label
)
cancel_button["command"] = partial(
on_stop, notaus1, notaus2, statusbar
)
window.mainloop()
finally:
gpio.cleanup()
if __name__ == "__main__":
main()
