Tool Box mit Qt Designer und Python

[astloch]

Hallo zusammen,
ich möchte gerne eine Toolbox in meinen Plot integrieren um darin navigieren zu können.
Die GUI habe ich mit Qt Designer gebastelt und den Code in Python.
Wäre super wenn mir jemand einen Tipp geben kann.
Hier mal mein kompletter Code:

Code: Alles auswählen

from PyQt4 import QtCore, QtGui, uic
import os
import serial
import time
import thread

import numpy
import pylab


class UiDemo(QtGui.QDialog):
    # constructor
    def __init__(self):
        QtGui.QDialog.__init__(self) #Aufruf des Konstruktors der Basisklasse
        # Set up the user interface from Designer.
        self.ui = uic.loadUi('rec_ui_motor_threading.ui')
        self.ui.show()
        self.ui.my_plot.axes.set_ylim(0,4100)
            
        # Connect up the buttons to functions
        self.connect(self.ui.pb_canc, QtCore.SIGNAL('clicked()'), 
                     QtGui.qApp, QtCore.SLOT('quit()'))
       
         
        self.connect(self.ui.pb_rec, QtCore.SIGNAL("clicked()"),
                     self.run_and_rec)             
                     
        self.connect(self.ui.pb_run,QtCore.SIGNAL("clicked()"),
                     self.run_motor)
                     
        # take spinbox values
        self.connect(self.ui.dauer, QtCore.SIGNAL('valueChanged(int)'),
                     self.change_motor_values)
        self.connect(self.ui.drehzahl, QtCore.SIGNAL('valueChanged(QString)'),
                     self.change_motor_values)
                     
        #Variablen definieren
        self.dauer = 1
        self.drehzahl = -180
       
    def change_motor_values(self):
        self.dauer = self.ui.dauer.value()
        self.drehzahl = self.ui.drehzahl.value()*-1 #spinning direction as in GUI
    
    def run_motor(self):##########################
        
        ser = serial.Serial(0)
        ser.baudrate = 57600
        ser.timeout = 1
        ser.bytesize = 8
        ser.write("VR;")
        ser.write("EO=1;")
        ser.write("MO=1;")
        my_string = "JV = %d;"%self.drehzahl
        ser.write(my_string)
        ser.write("BG;")
        time.sleep(self.dauer)
        ser.write("MO=0;")
        ser.close()
     
    def run_and_rec(self):
       thread.start_new_thread(self.get_values,(()))
       thread.start_new_thread(self.run_motor,(()))
      
        
                
    def get_values(self):
        #os.system("C:\\Users\\witt_bo\\Programmieren\\workspace_c\\recording_get_encoder_data\\Debug\\recording_get_encoder_data.exe")
        my_filename = open("C:\\Users\\witt_bo\\Dokumente\\Messdaten_Heidenhain\\letzter_Dateiname\\last_string.txt","r")#Dokument oeffnen
        
        for zeilen in my_filename:
            filename1 = zeilen
        filename = "C:\Users\witt_bo\Dokumente\\Messdaten_Heidenhain\\Position\\" + filename1
        filename2 = "C:\Users\witt_bo\Dokumente\\Messdaten_Heidenhain\\PositionAB\\" + filename1
        my_print_file = open(filename2,"w")
        my_obj_name = open(filename, "r")
    
        encoder1 = []
        encoder2 = []
        
        a = 0
        b = 0
        cnt = 1  # number of pitches in total (55 pitches/360 deg)
        cnt2 = 1
        neu, neu2 = 0, 0
        alt, alt2 = 0, 0
        zaehler, zaehler2 = 0, 0
        diff = 0
        min_diff = 100
        max_diff = 0
        start_cnt = 0
        zaehlerstand = 0
        for zeilen in my_obj_name:
            a = float(zeilen[46:56]) # encoder 1
            b = float(zeilen[78:88]) # encoder 3
            if a > 4096:
                cnt = int(zeilen[46:56])/4096
                a = a - 4096*cnt
                neu = a
                if neu > alt:
                    alt = neu
                if neu < alt:
                    zaehler += 1
                    alt = 0
            if b > 4096:
                cnt2 = int(zeilen[78:88])/4096
                b = b - 4096*cnt2
                neu2 = b
                if neu2 > alt2:
                    alt2 = neu2
                if neu2 < alt2:
                    zaehler2 += 1
                    alt2 = 0
                    
            encoder1.append(a)
            encoder2.append(b)
            my_print_file.write("%010d %010d %d %d\n" % (a, b, zaehler, zaehler2))
        
       
        for i in range(len(encoder1)): #gets the minimum difference between a and b
            if encoder1[i] > encoder2[i]:
                diff = encoder1[i]-encoder2[i]
            else:
                diff = encoder2[i]-encoder1[i]
            if diff < min_diff:
                min_diff = diff
                zaehlerstand = i
        print "kleinste Differez: ",min_diff," Zaehlerstand ",zaehlerstand
        
        my_print_file.close
        my_obj_name.close 
        y = numpy.array(encoder1) #55 pitches
        z = numpy.array(encoder2) #56 pitches
        x = z - y
        t = numpy.arange(0,len(encoder1),1)
        
        
        
        self.ui.my_plot.axes.hold(False)
        uiDemo.ui.setWindowTitle("hallo du raeuber")
       

        #plotHandle1 = self.ui.my_plot.axes.plot(t,y,"k",t,z,"c",t,x,"r") 
        plotHandle1 = self.ui.my_plot.axes.plot(t,z,"b", t,y,"r") 
        #plotHandle1 = self.ui.my_plot.axes.plot(t,x,"black",linewidth=2) 
        
              
        
        self.ui.my_plot.axes.legend([plotHandle1],["55 Zaehne","56 Zaehne", "absolut"])
        self.ui.my_plot.axes.set_xlim(500,540)
        self.ui.my_plot.draw()
    
if __name__ == '__main__':
    import sys
    app = QtGui.QApplication(sys.argv)
    uiDemo = UiDemo()#neue Instanz der Klasse UiDemo
    sys.exit(app.exec_())

PS.
ich weiß auch nicht wie ich die Legende so verschieben kann das sie nicht mitten im Plot hängt.
Bin über jede Hilfestellung und jeden Tipp dankbar.
Schöne Grüße aus Bayern

[BlackJack]

@astloch: Zum eigentlichen Problem kann ich nichts sagen, hätte aber ein paar Anmerkungen zum Quelltext.

Es gibt seit geraumer Zeit eine kürzere Variante im Signale und Slots zu verbinden als über die `connect()`-Methode auf Widgets. Die erkennt zusätzlich auch schneller Fehler, wenn man versucht Signale zu verwenden, die es nicht gibt. Beispiele:

Code: Alles auswählen

        self.connect(self.ui.pb_run,QtCore.SIGNAL("clicked()"),
                     self.run_motor)
        
        # ->
        
        self.ui.pb_run.clicked.connect(self.run_motor)

        self.connect(self.ui.dauer, QtCore.SIGNAL('valueChanged(int)'),
                     self.change_motor_values)
        
        # ->
        
        self.ui.dauer.valueChanged(int).connect(self.change_motor_values)

Für das schliessen vom seriellen Port böte sich die ``with``-Anweisung im Zusammenhang mit `contextlib.closing()` an. Damit ist, wie bei ``with`` und Dateien, sichergestellt, dass `close()` auf jeden Fall beim Verlassen des ``with``-Blocks aufgerufen wird. Auch wenn zum Beispiel eine Ausnahme auftritt.

Code: Alles auswählen

    def run_motor(self):
        with closing(Serial(0, 57600, 8, timeout=1)) as port:
            port.write('VR;')
            port.write('EO=1;')
            port.write('MO=1;')
            port.write('JV = %d;' % self.drehzahl)
            port.write('BG;')
            time.sleep(self.dauer)
            port.write('MO=0;')

Das `thread`-Modul ist „deprecated”. Es wurde schon seit längerem durch das `threading`-Modul abgelöst. In Python 3 ist `thread` nicht mehr importierbar und damit nicht mehr Teil der öffentlichen API. Wie lange läuft eigentlich `run_motor()`? Lohnt sich das überhaupt, beziehungsweise ist es denn nötig das in einem eigenen Thread zu starten?

`get_values()` ist zu lang und zu unübersichtlich. Da gibt es 31 lokale und zum Teil sehr schlechte Namen. Zwei davon werden nach der Wertzuweisung überhaupt nicht verwendet — auch nicht im auskommentierten Quelltext. Wenn man anfängt Namen durch zu nummerieren oder so etwas wie `my_*` als Vorsilbe dran zu hängen, damit sie nicht mit anderen Namen kollidieren, macht man etwas falsch. Der schlechteste Name ist `my_obj_name`. Generischer und nichtssagender geht es kaum.

In zwei Schleifen werden *einzelne* Zeilen an den Namen `zeilen` gebunden.

Man sollte sich wiederholende Daten, wie zum Beispiel den gemeinsamen Teil von verschiedenen Pfaden nicht mehrfach in den Quelltext schreiben, sondern einmal an einen Namen binden und dann wiederverwenden. Pfade sollte man nicht mit ``+`` sondern mit `os.path.join()` zusammen setzen.

Die beiden Dateien werden nicht wieder geschlossen. Einfach nur ``datei_objekt.close`` zu schreiben reicht nicht, man muss die Methode auch *aufrufen*. Oder das weiter oben schon erwähnte ``with`` verwenden.

Namen sollte man erst an Werte binden wenn man sie wirklich braucht und auch nicht an Werte die garantiert niemals verwendet werden. Das betrifft zum Beispiel das binden von `a`, `b`, `neu`, `neu2`, und `diff` vor der Schleife an 0 oder `cnt` und `cnt2` an 1. In der Schleife werden die Namen sofort an andere Werte gebunden und nach der Schleife werden sie nicht mehr verwendet. Damit ist diese „Initialisierung” vollkommen überflüssig. `diff`, `min_diff`, und `zaehler_stand` werden auch eine Schleife zu früh „Initialisiert”. Das macht den Quelltext unübersichtlicher und es ist schwerer einzelne Teile als Funktion heraus zu lösen.

`cnt` und `cnt2` sind überflüssig wenn man Modulo-Rechnung verwendet. `neu` und `neu2` sind überflüssig, weil das einfach nur Namen sind, die immer an die gleichen Werte wie `a` und `b` gebunden sind.

Die beiden ``if``-Bedingungen gegen den alten Wert lassen den Fall wenn der alte und der neue Wert gleich sind, offen. Der Effekt ist der gleiche als wenn im ersten ``if`` auf ≥ gebrüft würde. Wenn man das tut schliessen sich die beiden Bedingungen in den ``if``\s gegenseitig aus und man könnte es als ``if``/``else`` schreiben, was IMHO klarer ist.

Was soll dieser Abschnitt semantisch eigentlich bewirken? Ist die Eingangsprüfung ``a > 4096`` tatsächlich richtig? Ich habe den Verdacht, dass hier eigentlich ``>=`` stehen sollte, wenn ich mir die Modulo-Rechnung in dem Zweig anschaue. Und muss der alte Wert im Fall vom Erhöhen des Zählers auf 0 gesetzt werden? Ginge nicht auch den aktuellen Wert zum alten zu machen, wie im anderen ``if`-Zweig? Welchen Wertebereich haben die Zahlen in den Zeilen? Sind das überhaupt Gleitkommazahlen?

Die Schleife enthält für `a` und `b` von der Struktur identischen Quelltext. Wenn man anfängt zu kopieren sollte man darüber nachdenken eine Funktion oder eine Klasse aus den Gemeinsamkeiten zu machen. In diesem Fall eine Klasse wegen dem Zähler:

Code: Alles auswählen

class ValueParser(object):
    def __init__(self, index, length=10, max_value=4096):
        self.index = index
        self.length = length
        self.max_value = max_value
        self.old_value = 0
        self.count = 0
    
    def __call__(self, line):
        result = float(line[self.index:self.index + self.length])
        if result > self.max_value:  # TODO Or ``>=``!?
            result %= self.max_value
            if result >= self.old_value:
                self.old_value = result
            else:
                self.count += 1
                self.old_value = 0
        return result

Verwenden kann man das dann so (ungetestet):

Code: Alles auswählen

                a_parser, b_parser = map(ValueParser, [46, 78])
                for line in position_file:
                    a, b = (f(line) for f in [a_parser, b_parser])
                    encoder1.append(a)
                    encoder2.append(b)
                    position_ab_file.write(
                        '%010d %010d %d %d\n' % (
                            a, b, a_parser.count, b_parser.count
                        )
                    )

Was daran noch unschön ist: Die beiden Werte `a` und `b` gehören eigentlich zusammen, werden später auch gegeneinander verrechnet, aber sie werden trotzdem in zwei verschiedene Datenstrukturen gesteckt. Denn die Berechnung der Differenz ist selbst in Python schon sehr umständlich gelöst. ``for index in range(len(sequence))`` um mit dem `index` auf die Elemente von `sequence` zuzugreifen ist in Python ein Anti-Pattern. Man kann direkt über Elemente von Sequenzen iterieren, ohne dem Umweg über einen Index gehen zu müssen. Wenn man den *zusätzlich* benötigt, gibt es `enumerate()`. Und wenn man über zwei ”parallele” Sequenzen iterieren möchte gibt es `zip()` beziehungsweise `itertools.izip()`. Beides kann man kombinieren. Oder man steckt die Werte gleich in *eine* Datenstruktur, also zum Beispiel eine verschachtelte Liste. Dann lässt sich das sehr einfach mit `min()`, `enumerate()` und einem Generatorausdruck umsetzen. Und statt des ersten ``if``/``else``-Konstruktes in der Schleife könnte man die `abs()`-Funktion verwenden.

Code: Alles auswählen

        min_diff, zaehlerstand = min(
            (abs(a - b), i) for i, (a, b) in enumerate(encoder)
        )
        min_diff = min(min_diff, 100)  # TODO Really!?
        print 'kleinste Differenz:', min_diff, ' Zaehlerstand', zaehlerstand

Die zweite Verwendung von `min()` habe ich mal mit einem fragenden Kommentar versehen. Wenn `min_diff` vorher nämlich grösser als 100 war, dann ist `zaehlerstand` sehr wahrscheinlich auf einem falschen Wert. Aber das ist das was Dein Quelltext macht.

Alternativ hätte man diese Rechnung auch schon mit `numpy` erledigen können, denn auch dort wird die Differenz zwischen den beiden Wertereihen berechnet. Diese doppelte Berechnung könnte man also einsparen (ungetestet):

Code: Alles auswählen

        y, z = numpy.array(encoder).transpose()
        x = z - y
        t = numpy.arange(0, len(encoder), 1)
        
        zaehlerstand = numpy.absolute(x).argmin()
        min_diff = x[zaehlerstand]
        min_diff = min(min_diff, 100)  # TODO Really!?
        print 'kleinste Differez:', min_diff, ' Zaehlerstand', zaehlerstand

Auch hier wieder die Frage ob das mit `min_diff` und der 100 tatsächlich so sein soll.

Mit den Änderungen kommt man dann von 31 auf 19 lokale Namen runter. Was immer noch ein bisschen viel ist:

Code: Alles auswählen

    def get_values(self):
        root_path = r'C:\Users\witt_bo\Dokumente\Messdaten_Heidenhain'
        with open(
            os.path.join(root_path, 'letzter_Dateiname', 'last_string.txt')
        ) as filenames_file:
            for line in filenames_file:
                filename = line

        with open(
            os.path.join(root_path, 'Position', filename)
        ) as position_file:
            with open(
                os.path.join(root_path, 'PositionAB', filename)
            ) as position_ab_file:
                
                encoder = list()
                a_parser, b_parser = map(ValueParser, [46, 78])
                for line in position_file:
                    a, b = (f(line) for f in [a_parser, b_parser])
                    encoder.append([a, b])
                    position_ab_file.write(
                        '%010d %010d %d %d\n' % (
                            a, b, a_parser.count, b_parser.count
                        )
                    )

        y, z = numpy.array(encoder).transpose()
        x = z - y
        t = numpy.arange(0, len(encoder), 1)
        
        zaehlerstand = numpy.absolute(x).argmin()
        min_diff = x[zaehlerstand]
        min_diff = min(min_diff, 100)  # TODO Really!?
        print 'kleinste Differez:', min_diff, ' Zaehlerstand', zaehlerstand
        
        self.ui.my_plot.axes.hold(False)
        self.ui.setWindowTitle("hallo du raeuber")
       
        #plot = self.ui.my_plot.axes.plot(t, y, 'k', t, z, 'c', t, x, 'r')
        plot = self.ui.my_plot.axes.plot(t, z, 'b', t, y, 'r')
        #plot = self.ui.my_plot.axes.plot(t, x, 'black', linewidth=2)

        self.ui.my_plot.axes.legend(
            [plot], ['55 Zaehne', '56 Zaehne', 'absolut']
        )
        self.ui.my_plot.axes.set_xlim(500, 540)
        self.ui.my_plot.draw()

Den Hauptcode sollte man nicht nur durch das ``if __name__``-Idiom „schützen” sondern auch in eine eigene Funktion stecken. Dann reduziert man die Namen auf Modulebene und es fällt auch sofort auf wenn im restlichen Code auf Objekte zugegriffen wird, auf die man so überhaupt nicht zugreifen sollte. Aus `get_values()` wurde an einer Stelle nämlich auf das globale `uiDemo` zugegriffen.

[astloch]

@BlackJack:
Oh Gott oh Gott.
Danke für die ganzen Hinweise. Ich werde den Code jetzt erst mal ein Bisschen überarbeiten.
Mir kam das mit der Methode get_values auch schon etwas lang vor, ich weiß aber nicht wie ich es ändern kann. Ich versuche jetzt mal mein Bestes um alles zu verstehen und umzusetzen was du geschrieben hast.
Kann sich nur um Jahr dauern.

Schönen Gruß

[astloch]

@BlackJack:
so, jetzt bin ich schon ein gutes Stück weiter gekommen. Paar Fragen habe ich aber noch.
+Warum soll ich für die eine Funktion eine Klasse machen? Du sagtes wegen dem Zähler. Ich verstehe nicht was du damit meinst.
+Wenn ich das hier:

Code: Alles auswählen

self.connect(self.ui.dauer, QtCore.SIGNAL('valueChanged(int)'),
self.change_motor_values)
 

in das hier:

Code: Alles auswählen

self.ui.dauer.valueChanged(int).connect(self.change_motor_values)

ändere, bekomme ich folgende Fehlermeldung:

Traceback (most recent call last):
File "C:\Users\witt_bo\Programmieren\Python_sensor\GUI\recording_UI_plot_motor\rec_ui_absolut_360deg.py", line 193, in <module>
uiDemo = UiDemo()#neue Instanz der Klasse UiDemo
File "C:\Users\witt_bo\Programmieren\Python_sensor\GUI\recording_UI_plot_motor\rec_ui_absolut_360deg.py", line 41, in __init__
self.ui.dauer.valueChanged(int).connect(self.change_motor_values)
TypeError: native Qt signal is not callable

Der Motor läuft übrigens ein paar Sekunden und während der Motor läuft muss ich ein paar Messungen machen die dann in die Datei geschrieben werden. Welche Werte- wie du richtig erkannt hast- keine float Werte sind.

Seruvs

[BlackJack]

@astloch: Wenn man den Code heraus zieht, der bei der Verarbeitung der beiden Werte strukturell gleich ist, dann ist der zustandsbehaftet. Die alten Werte und den Zähler muss man sich merken, man bräuchte also eine Funktion die sich über Aufrufe hinweg einen Zustand merkt. Und dafür sind Klassen da — die Kapseln einen Zustand und Funktionen die darauf operieren in einem Objekt. Nur mit einer Funktion könnte man das sauber lösen in dem man bei jedem Aufruf den alten Zustand hinein gibt, und neben dem eigentlichen Ergebnis auch den neuen Zustand zurück gibt. Als Funktion könnte das dann so aussehen, mit dem Problem, dass man den Zustand dann ausserhalb der Funktion hat, also keine Kapselung mehr:

Code: Alles auswählen

def parse_value(index, line, old_value, count, length=10, max_value=4096):
    result = float(line[index:index + length])
    if result > max_value:  # TODO Or ``>=``!?
        result %= max_value
        if result >= old_value:
            old_value = result
        else:
            count += 1
            old_value = 0
    return result, old_value, count

Da Python keine „vollwertigen” Closures hat, wie es funktionale(re) Programmiersprachen wie JavaScript, Lisp, oder Scheme bieten, kann man diesen Zustand auch dort nicht sauber Kapseln. Zumindest IMHO sind dafür Listen oder ``nonlocal`` in aktuellen Python-Versionen eher Hacks. In Python sind dafür nun einmal Klassen vorgesehen. Selbst mit Closure hätte man noch das Problem, dass man auch `count` ausserhalb sichtbar machen möchte. Man müsste also immer zwei Ergebnisse zurück liefern — den Wert und den Zähler. In JavaScript könnte man die Klasse zum Beispiel so als Closure formulieren (ungetestet):

Code: Alles auswählen

  function createValueParser(index, length, maxValue) {
    if (length == null) length = 10;
    if (maxValue == null) maxValue = 4096;
    var oldValue = 0;
    var count = 0;
    return function(line) {
      var result = parseInt(line.slice(index, index + length));
      if (result > maxValue) {
        result %= maxValue;
        if (result >= oldValue) {
          oldValue = result;
        } else {
          count++;
          oldValue = 0;
        }
      }
      return [result, count];
    };
  };

Verwendung dann ungefähr so:

Code: Alles auswählen

  function f(lines) {
    var a, b, aCount, bCount, tmp;
    var aParser = createValueParser(46);
    var bParser = createValueParser(78);
    var encoder = [];
    for (var i = 0; i < lines.length; i++) {
      var line = lines[i];
      tmp = aParser(line);
      a = tmp[0];
      aCount = tmp[1];
      tmp = bParser(line);
      b = tmp[0];
      bCount = tmp[1];
      encoder.push([a, b]);
      console.log(a, b, aCount, bCount);
    }
  };

Was natürlich unschön ist mit dem Array. Man würde in JavaScript selbst bei einem Closure natürlich ein Objekt zurück geben, weil einem das syntaktisch so einfach gemacht wird. Beziehungsweise würde man bei `createValueParser()` schon `count` zu einem Attribut vom Rückgabewert machen.

----

Das kommt davon wenn man aus dem Gedächtnis APIs zitiert. Um bei Signalen überladene Varianten anzusprechen muss man das Objekt in Python nicht aufrufen, sondern den Index-Operator verwenden:

Code: Alles auswählen

self.ui.dauer.valueChanged[int].connect(self.change_motor_values)

Wobei man auch in der Dokumentation nachsehen könnte ob das `valueChanged`-Signal überhaupt überladen ist, beziehungsweise was der „default overload” ist — dann könnte man das eventuell auch weglassen.