@thomasgull: 2 Sekunden ist ja der Timeout, also würde ich mal auf das ``read(25)`` tippen. Damit sagst Du: Lies 25 Bytes und warte bis zur Zeitüberschreitung bis die Leseoperation abgebrochen wird, falls weniger als 25 Bytes in der Zeit gelesen werden.
Was erwartest Du denn dort als Antwort über die serielle Schnittstelle? Wie lang ist die? Oder wird die mit einem bestimmten Bytewert abgeschlossen? Vielleicht ist `readline()` hier die bessere Alternative?
Zum Quelltext: Der ist teilweise nicht besonders gut. Die Klasse ist zum Beispiel nicht wirklich so geschrieben wie die Sprache das vorsieht.
Die `init()`-Methode sollte `__init__()` heissen, dann braucht man sie auch nicht extra aufrufen. In dieser Methode sollten alle Attribute auf dem Objekt eingeführt werden. Wenn man in beliebigen anderen Methoden neue Attribute einführt, ist es schwierig sich ein Bild davon zu machen wann die Exemplare welche Attribute haben und wann noch nicht. Das macht Programme unübersichtlicher und Fehleranfälliger. Nach der `__init__()` sollte ein Objekt vollständig initialisiert und in einem konsistenten Zustand sein.
Wenn man das `Serial`-Exemplar schon in der `__init__()` erstellt, kann man für `isOpen` auch *dessen* `isOpen()`-Methode heran ziehen, statt das Flag selber zu verwalten.
Das was als Attribute auf der Klasse definiert wird gehört da nicht wirklich hin, sondern auch eher in die `__init__()`. Dann kann man auch Defaultwerte bei den Argumenten verwenden.
Eigentlich verstehe ich den Sinn der Klasse überhaupt nicht. Im Grunde ist das doch nur ein Wrapper um das Serial-Objekt bei dem man `read()` und `write()` aufrufen kann und dass dann die Verbindung sozusagen On-Demand aufbaut. Nur wird genau das im Quelltext überhaupt gar nicht verwendendet, sondern am Anfang explizit die `start()`-Methode aufgerufen. Damit macht diese Klasse keinen Sinn.
``return`` ohne Rückgabewert an das Ende von Funktionen oder Methoden zu schreiben ist sinnfrei. Ebenso sinnfrei ist es den semantisch nicht vorhandenen Rückgabewert bei den Aufrufen an einen Namen zu binden.
``print`` ist in Python 2 eine Anweisung und keine Funktion, also sollte man es auch nicht so schreiben als wäre es eine. Oder man importiert in aktuellen Python 2er-Versionen die Änderung zu einer `print()`-Funktion aus dem `__future__()`-Modul. Eins von beidem sollte man machen und dann innerhalb eines Moduls konsistent durchziehen.
Auf Moduleben wird Code zum erstellen von Konstanten (Werte, Module, Funktionen, Klassen, …) mit Variablen und Programm vermischt. Das ist unübersichtlich, führt unnötig globale Namen ein, die unbeabsichtigt in Funktionen oder Methoden verwendet werden können oder gar absichtlich, was undurchsichtige Abhängigkeiten zur Folge hat. Der Code sollte in einer Funktion verschwinden und mit dem ``if __name__ == '__main__':``-Idiom aufgerufen werden.
Es werden ein paar Namen im Code gar nicht benutzt. Da wären die importierten Module `io` und `time`; und `SEW.byte`, `SEW.port`, `Result`, `ADR`, `Index`, `SD2`, `Subindex`, `TYP`, und `float1`. Teilweise hängen da transitiv Namen dran, die auch nicht verwendet werden. `text1` wird nur zum berechnen des nicht verwendeten `float1` benutzt, ist also genau so überflüssig. Ausserdem werden Namen ohne Notwendigkeit eingeführt und an vorhandene Objekte mit anderen Namen gebunden. Warum? `ind` und `Index` in `Movilink()` wäre ein Beispiel. Und dann diese Unart jedes noch so kleine Zwischenergebnis an durchnummerierte Namen zu binden. Was soll das? Die 9 Zeilen nach ``# Index aufarbeiten`` bekäme man ohne die ganzen unnötigen Namen auf drei herunter. Wobei das Vorgehen dort ziemlich abenteuerlich ist, für eine Zahlenumwandlung die `repr()`-Form von Gleitkommezahlen und Zeichenkettenoperationen heran zu ziehen. Das ist glaube ich die umständlichste Art einen 16-Bit-Wert in Low- und High-Byte aufzuteilen, die ich bis jetzt gesehen habe. Das geht auch als Einzeiler und in verständlich: ``index_high, index_low = divmod(index, 256)``
Es macht wenig Sinn `int()` auf Werten aufzurufen, von denen man schon weiss, dass es ganze Zahlen sind. Das betrifft sowohl Konstante Werte, als auch Argumente und das Ergebnis der `ord()`-Funktion.
Statt jeden Wert für die CRC hinzuschreiben hätte man besser eine Funktion geschrieben, welche die CRC für eine Zeichenkette oder ein `bytearray` berechnet. Das berechnen von High-/Low-Bytes und zusammensetzen einer Datenstruktur liesse sich mit dem `struct`-Modul aus der Standardbibliothek besser lösen.
Der Präfix `obj*` wie bei `objSEW` macht in Python grundsätzlich keinen Sinn, weil *alles* was man an einen Namen binden kann ein Objekt ist. Durch so einen Präfix hat man also nichts gewonnen, nur einen unnötig längeren Namen.
Listen mit Dummywerten zu erstellen, die man dann durch die tatsächlich verwendeten Werte ersetzt ist in Python genau so ungewöhnlich wie der unnötige Umweg über einen Index, um über Werte in Sequenzen wie beispielsweise Listen zu iterieren. Man hätte die Parameterlisten auch *gleich* mit den passenden Werten füllen können. Ausserdem sollte man nicht zusammengehörige Daten in parallelen Datenstrukturen speichern. Das erhöht die Komplexität des Codes und Fehleranfälligkeit unnötig.
Kommentare sollten dem Leser einen Mehrwert über den Code bieten. Einen `close()`-Aufruf muss man nicht mit einem Kommentar ``# Schliessen`` versehen. Wer das nicht aus dem Code sehen kann, dem hilft auch der Kommentar nicht. In der Regel sollten Kommentare nicht erklären *was* gemacht wird, denn das steht da ja schon im Quelltext, sondern *warum* etwas gemacht wird, wenn das nicht offensichtlich ist und einer Erklärung bedarf. Wenn man einen Kommentar schreibt, kann man sich auch überlegen ob man den Quelltext stattdessen nicht verständlicher hinbekommt. Bei dem Kommentar ``# öffnen`` vor dem Aufruf der `start()`-Methode fällt zum Beispiel auf, dass die Aufgabe der `start()`-Methode tatsächlich einzig das öffnen der Verbindung enthält. Ein Zeichen das der Name `start()` vielleicht eher `open()` sein sollte. Dann ist der Kommentar auch überflüssig.
Die Namen und die Formatierung des Quelltextes halten sich nicht an den
Style Guide for Python Code.
Anstelle der ganzen `print()`-Aufrufe die anscheinend hauptsächlich zum Verfolgen des Programmablaufs dienen, sollte man das `logging`-Modul verwenden. Dann kann man die Ausgaben in verschiedene Stufen einteilen, die man ausgeben lassen, in Protokolldateien schreiben lassen, oder auch ganz unterdrücken kann, ohne dass man überall im Quelltext Änderungen vornehmen muss.
Einiges von den Anmerkungen umgesetzt (ungetestet):
Code: Alles auswählen
#!/usr/bin/env python
# -*- coding: cp1252 -*-
from __future__ import print_function
from contextlib import closing
import serial
class SEW(object):
def __init__(self, port=3, baudrate=9600):
self.serial = serial.Serial(
baudrate=baudrate, parity=serial.PARITY_EVEN, timeout=2
)
self.serial.port = port
self.isOpen = self.serial.isOpen
def start( self,):
if not self.isOpen():
self.serial.open()
print('Opened port.')
print(self.serial.portstr)
def close(self):
self.serial.close()
print('Closed port.')
def write(self, command):
self.start()
self.serial.write(command)
print('{0:10}{1!r}'.format('Sent:', command))
def read(self, byte_count):
self.start()
result = self.serial.read(byte_count)
print('{0:10}{1!r}'.format('Read:', result))
return result
def movilink(adr, index, subindex):
sd1 = 0x02
typ = 0x86
verw = 0x31
pd3h = 0
pd3l = 0
pd4h = 0
pd4l = 0
index_high, index_low = divmod(index, 256)
print(index)
print(index_high)
print(index_low)
print(subindex)
#
# TODO: Write CRC function.
#
crc = (
sd1 ^ adr ^ typ ^ verw ^ subindex ^ index_high ^ index_low ^ pd3h
^ pd3l ^ pd4h ^ pd4l
)
print(crc)
#
# TODO: Use `struct` module.
#
result = str(
bytearray(
[
sd1, adr, typ, verw, subindex, index_high, index_low, pd3h,
pd3l, pd4h, pd4l, crc
]
)
)
print(repr(result))
return result
def main():
sew = SEW(3)
sew.start() # TODO: Either this call or the `SEW` class is unnecessary.
with closing(sew):
parameters = [(8325, 0), (8301, 0)]
for index, subindex in parameters:
telegram = movilink(0, index, subindex)
print(repr(telegram))
sew.write(telegram)
response = sew.read(25)
print(response)
print('Länge: ', len(response))
if response:
print(ord(response[1]))
else:
print('Kein Empfang')
if __name__ == '__main__':
main()
) - alle beide Optionen sind auf Englisch - kann man das locker schaffen. Ich bin nicht der fitteste in Englisch (d.h. nicht schlecht, aber auch nicht gut => ganz okay) , aber dennoch kann man das mit etwas Mühe schaffen.
Wegen diesem Galieleo Drums hätte ich fast aufgegeben, aber das KiddieBuch gibt mir neuen Mut.