Parallelität/IO Error

[iuvabo]

Moin moin zusammen,
ich habe vor kurzem mit Python angefangen ein wenig zu programmieren.

Ich arbeite mit einem Arduino Board und einem Raspberry PI, das Python Script läuft auf dem RaspberryPi und kommuniziert mittels SMBus mit dem Arduino.

Das Script macht folgendes:
In einem Thread wird regelmäßig ein Ping Request auf mein Handy losgeschickt. Wenn das Handy im WLAN ist, dann wird die Funktion writeNumber ausgeführt und eine Zahl an das Arduino Board geschickt, wenn nicht, dann eine andere. Das lief bis vor kurzem auch stabil und fehlerfrei.

Nun habe ich den Code erweitert und einen Webserver aufgesetzt, über den ich weitere Befehle das Arduino Board schicken kann. Dieser Server läuft in einem Thread, der Ping-Vorgang in einem anderen.

Seitdem läuft das ganze nicht mehr stabil, ich erhalte nach einiger Zeit diesen Fehler:

Code: Alles auswählen

Sat Oct 25 16:30:41 2014 Server Starts - 192.168.178.9:8080
Traceback (most recent call last):
  File "new.py", line 56, in pingRequest
    writeNumber(1);
  File "new.py", line 27, in writeNumber
    bus.write_byte(address, value)
IOError: [Errno 5] Input/output error

Der Fehler scheint aufzutreten, wenn der Ping-Vorgang und ein Web-Aufruf fast zur gleichen Zeit aufgerufen werden.

Nun ist die Frage, wo ich da den Fehler habe? Ich habe den Thread mittels lock-Variable im kritischen Bereich großzügig gesichert und zusätzlich um die Funktion While-Schleife gesetzt, die solange wartet, bis der letzte Aufruf abgeschlossen war.

Wie kann ich den Fehler abfangen und verhindern? Ich dachte eigentlich, dass der Try/Catch Block den Fehler abfängt, aber das ist nicht der Fall.

Der Code ist ein wenig eingekürzt, ich hoffe alle wichtigen Teile sind drin.

Code: Alles auswählen

bus = smbus.SMBus(1)
offlineCounter = 0
lock = thread.allocate_lock() 
lastSuccessful = 1
lastRelaiseState = 0

def doNoting():
    return -1
    
def writeNumber(value):
    try:
        global lock
        lock.acquire() 
        global lastSuccessful
        while lastSuccessful == 0:
             sleep(0.1)
        lastSuccessful = 0
        address = 0x04
        bus.write_byte(address, value)
        sleep(0.25)
        lastSuccessful = 1
        lock.release()
        # bus.write_byte_data(address, 0, value)
        return 1
    except:
        raise
    return 0


def pingRequest(): 
    global lastRelaiseState    
    global offlineCounter    
    while True:
        try:
            hostname = "android-588e9d891161229e" #example
            response = os.system("ping -c 1 " + hostname)
            time.sleep(10)
            if response == 0:
                offlineCounter=0
            else:
                offlineCounter=offlineCounter+1
            if offlineCounter <= 10: 
                writeNumber(1);
                if lastRelaiseState == 0: #Anlage anschalten bei erstem Start
                    lastRelaiseState = 1
                    writeNumber(6);
            else:
                writeNumber(0)
                lastRelaiseState = 0
        except:
            os.system("rm /home/pi/log");    
            raise



class MyHandler(BaseHTTPServer.BaseHTTPRequestHandler):
    def do_HEAD(s):
        s.send_response(200)
        s.send_header("Content-type", "text/html")
        s.end_headers()
    def do_GET(s):
        
        #gekürzt
            
            
        """Respond to a GET request."""
        s.send_response(200)
        s.send_header("Content-type", "text/html")
        s.end_headers()
        f = open("index.html") 
        s.wfile.write(f.read())
        f.close()
        # If someone went to "http://something.somewhere.net/foo/bar/",	
        # then s.path equals "/foo/bar/".
        s.wfile.write("<p>You accessed path: %s</p>" % s.path[5:])
        s.wfile.write("</body></html>")
	
        
def httpServer():
    PORT_NUMBER = 8080
    HOST_NAME = '192.168.178.9'  
    server_class = BaseHTTPServer.HTTPServer
    httpd = server_class((HOST_NAME, PORT_NUMBER), MyHandler)
    print time.asctime(), "Server Starts - %s:%s" % (HOST_NAME, PORT_NUMBER)
    try:
        httpd.serve_forever ()
    except KeyboardInterrupt:
        raise
    httpd.server_close()
    print time.asctime(), "Server Stops - %s:%s" % (HOST_NAME, PORT_NUMBER)




if __name__ == '__main__':
    try:
        thread.start_new_thread(pingRequest,())   
        httpServer()   
    except:
        raise
       

Für jeden Tipp/Hinweis bin ich Dankbar.

Viele Grüße
Joachim

[BlackJack]

@iuvabo: Ich denke es sollten erst einmal folgende drei Punkte abgearbeitet werden:

• Nicht das `thread`-Modul verwenden, das ist schon lange ”deprecated” und durch das `threading`-Modul abgelöst.
• Keine nackten ``except:`` verwenden sondern immer konkrete Ausnahmen behandeln die man auch erwartet. Und ein ``except: raise`` macht keinen Sinn, da passiert genau das selbe als wenn es gar kein ``try``/``except`` gegeben hätte. Was soll das?
• Kein ``global`` verwenden. Davon abgesehen dass es an einigen Stellen sinnfrei ist weil es keinen Effekt hat, bei `lock` zum Beispiel, macht es das ganze unnötig unübersichtlich und fehleranfällig.

`lastSuccessful` verstehe ich nicht. Das wird immer nur innerhalb eines durch ein Lock geschützten Abschnitts verändert und ist nach diesem Abschnitt immer 1 und wird im Abschnitt eigentlich nicht wirklich verwendet. Ausser das „busy waiting” am Anfang solange der Wert 0 ist, was er zumindest beim gezeigten Quelltext sowieso niemals sein kann. Selbst wenn ist „busy waiting” schlecht. Dafür gibt es `Lock`-, `Event`-, oder `Condition`-Objekte im `threading`-Modul.

Falls eine Ausnahme auftritt während das Lock gehalten wird, gibt es in `writeNumber()` eine potentielle Verklemmung weil das Lock dann nicht wieder freigegeben wird. Das sollte man in ein ``try``/``finally`` setzen oder die ``with``-Anweisung mit dem `Lock`-Exemplar verwenden.

Die Funktion hat einen Rückgabewert der nie verwendet wird. Und der ist *immer* 1. Das ``return 0`` kann niemals erreicht und ausgeführt werden.

Wenn eine Variable ein Flag darstellen soll, dann sind `True` und `False` verständlicher als 1 und 0. Dann weiss man nämlich das es ein Flag ist und nicht auch 3 oder 42 als Werte in Frage kommen.

`os.system()` sollte man nicht verwenden. Dafür gibt es das `subprocess`-Modul. Für das Löschen einer Datei braucht man keinen externen Prozess starten, das gibt's als Funktion im `os`-Modul.

Die Namensschreibweisen entsprechen nicht dem Style Guide for Python Code. Und auch wenn man das erste Argument von Methoden nennen kann wie man will kann man es nicht anders als `self` nennen. Echt nicht.

[iuvabo]

Vielen Dank für dein schnelles Feedback!

• Nicht das `thread`-Modul verwenden, das ist schon lange ”deprecated” und durch das `threading`-Modul abgelöst.
• Keine nackten ``except:`` verwenden sondern immer konkrete Ausnahmen behandeln die man auch erwartet. Und ein ``except: raise`` macht keinen Sinn, da passiert genau das selbe als wenn es gar kein ``try``/``except`` gegeben hätte. Was soll das?
• Kein ``global`` verwenden. Davon abgesehen dass es an einigen Stellen sinnfrei ist weil es keinen Effekt hat, bei `lock` zum Beispiel, macht es das ganze unnötig unübersichtlich und fehleranfällig.

Ist umgeändert. In einem Fall konnte ich das "except" nicht genaue spezifizieren, aber ansonsten entfernt

`lastSuccessful` verstehe ich nicht. Das wird immer nur innerhalb eines durch ein Lock geschützten Abschnitts verändert und ist nach diesem Abschnitt immer 1 und wird im Abschnitt eigentlich nicht wirklich verwendet. Ausser das „busy waiting” am Anfang solange der Wert 0 ist, was er zumindest beim gezeigten Quelltext sowieso niemals sein kann. Selbst wenn ist „busy waiting” schlecht. Dafür gibt es `Lock`-, `Event`-, oder `Condition`-Objekte im `threading`-Modul.

Danke, das war ein überflüssiges Überbleibsel. Ist entfernt.

Falls eine Ausnahme auftritt während das Lock gehalten wird, gibt es in `writeNumber()` eine potentielle Verklemmung weil das Lock dann nicht wieder freigegeben wird. Das sollte man in ein ``try``/``finally`` setzen oder die ``with``-Anweisung mit dem `Lock`-Exemplar verwenden.

Danke, hab die Freigabe in den finally Block geschoben.

Die Funktion hat einen Rückgabewert der nie verwendet wird. Und der ist *immer* 1. Das ``return 0`` kann niemals erreicht und ausgeführt werden.

Wenn eine Variable ein Flag darstellen soll, dann sind `True` und `False` verständlicher als 1 und 0. Dann weiss man nämlich das es ein Flag ist und nicht auch 3 oder 42 als Werte in Frage kommen.

Ist auf True gesetzt und gekürzt

`os.system()` sollte man nicht verwenden. Dafür gibt es das `subprocess`-Modul. Für das Löschen einer Datei braucht man keinen externen Prozess starten, das gibt's als Funktion im `os`-Modul.

Habe sämtliche os.system auf subprocess umgeändert. Die Logdatei wird ohnehin nicht mehr benötigt, ist komplett entfernt

Die Namensschreibweisen entsprechen nicht dem Style Guide for Python Code. Und auch wenn man das erste Argument von Methoden nennen kann wie man will kann man es nicht anders als `self` nennen. Echt nicht.

Sollte nun soweit korrigiert sein.


Hier der aktuelle Code:

Code: Alles auswählen

import time
import BaseHTTPServer
import subprocess
import smbus
import time
import threading
from threading import Thread
from time import sleep


bus = smbus.SMBus(1)

def do_noting():
    return -1
    
def write_number(value):
    lock = threading.Lock() 
    lock.acquire()
    try:
        address = 0x04
        bus.write_byte(address, value)
        sleep(0.25)
        # bus.write_byte_data(address, 0, value)
    except IOError:
        print "IO Error"
    finally:
        lock.release() 
      
    return True
"""
def readNumber():
    number = bus.read_byte(address)
    # number = bus.read_byte_data(address, 1)
    return number"""

class PingClass():
    def __init__(self):
        self.last_relay_state = 0
        self.offline_counter = 0
    #Pingt den Hostname für immer an und sendet Befehl an das Arduino Board
    def ping_request(self,hostname): 
        while True:
            try:
                response = subprocess.call("ping -c 1 " + hostname,shell=True)
                time.sleep(10)
                if response == 0:
                    self.offline_counter=0
                else:
                    self.offline_counter=self.offline_counter+1
                if self.offline_counter <= 10: 
                    write_number(1);
                    if self.last_relay_state == 0: #Anlage anschalten bei erstem Start
                        self.last_relay_state = 1
                        write_number(6);
                    subprocess.call("echo 'online'",shell=True)
                else:
                    subprocess.call("umount -a",shell=True)
                    time.sleep(5)
                    write_number(0)
                    self.last_relay_state = 0
                    subprocess.call("echo 'offline'",shell=True)
            except:
                print "Unknown Error"
                


class MyHandler(BaseHTTPServer.BaseHTTPRequestHandler):
    def do_HEAD(s):
        s.send_response(200)
        s.send_header("Content-type", "text/html")
        s.end_headers()
    def do_GET(s):
        #gekürzt
        s.send_response(200)
        s.send_header("Content-type", "text/html")
        s.end_headers()
        f = open("index.html") 
        s.wfile.write(f.read())
        f.close()
        s.wfile.write("<p>You accessed path: %s</p>" % s.path[5:])
        s.wfile.write("</body></html>")
	
#startet den HTTP Server        
def http_server():
    PORT_NUMBER = 8080
    HOST_NAME = '192.168.178.9'  
    server_class = BaseHTTPServer.HTTPServer
    httpd = server_class((HOST_NAME, PORT_NUMBER), MyHandler)
    print time.asctime(), "Server Starts - %s:%s" % (HOST_NAME, PORT_NUMBER)
    try:
        httpd.serve_forever ()
    except KeyboardInterrupt:
        print "Keyboard Interrupt"
    httpd.server_close()
    print time.asctime(), "Server Stops - %s:%s" % (HOST_NAME, PORT_NUMBER)

#Thread fürs Pingen starten und Webserver starten
if __name__ == '__main__':
    pingClass = PingClass()
    hostname = 'android-588e9d891161229e';
    t = Thread(target=pingClass.ping_request, args=(hostname,))
    t.start()
    http_server()   

[mieszko]

BlackJack has recht ueber dein Skript, aber das ist nicht die Quelle deines Problems.
Ich denke dass das Problem ist nicht in dein Skript. Die Funktion write_byte nimmt andere funktion - i2c_smbus_write_byte,

Code: Alles auswählen

   
 @validate(addr=int, val=int)
    def write_byte(self, addr, val):
        """write_byte(addr, val)

        Perform SMBus Write Byte transaction.
        """
        self._set_addr(addr)
        if SMBUS.i2c_smbus_write_byte(self._fd, ffi.cast("__u8", val)) == -1:
            raise IOError(ffi.errno)

quelle: https://github.com/bivab/smbus-cffi/blo ... s/smbus.py
und diese i2c_smbus_write_byte ist verantwortlich fuer die WRITE Prozess und liefert als Ergebnis -1 wann sie kann nicht es tun, das ist wann es gibt ein Fehler und 0 wann es gibt ein Erfolg. Wann i2c_smbus_write_byte liefert -1, dann write_byte liefert ein Error. Also das Problem ist nicht in dein Python Skript aber - wahrscheinlich - mit Gerät, aber das ist nur meine Hypothese
Sorry fuer mein Deutsch!
English version below:
BlackJack is absolutely right in his remarks, but that's not the source of your problem.
I don't think that the problem is in your code. You call the write_byte function, which in turn calls i2c_smbus_write_byte function. That i2c_smbus_write_byte performs actual write operation and returns negative integer (-1) when the operation is unsuccessful (God knows why!) nad 0 when it's a success. When SMBUS.i2c_smbus_write_byte returns -1, write_byte returns an error. It looks like the source of your problem is outside that script, probably with the device itself, but I'm just guessing.
I'm deeply sorry for my German - I hope that you could make head and tails from what I've written.

[Sirius3]

@iuvabo: jetzt ist der Lock ja noch unsinniger. Welcher weitere Thread soll denn auf die lokale Variable zugreifen können?

subprocess mit shell=True ist kaum besser als os.system. Zumal, es gibt 'echo' schon als Python-Befehl: print. self sollte wirklich nie als s abgekürzt werden.

[BlackJack]

@iuvabo: Wogegen soll das Lock denn überhaupt schützen? In dem gezeigten Quelltext wird doch nur von *einem* Thread aus auf `bus` zugegriffen.

Die Fehlerbehandung dort ist keine — man kann nicht einfach 'IO Error' ausgeben und so weitermachen als sei nichts passiert. So ein Bus muss sich von einem E/A-Fehler ja nicht wieder erholen und wenn die Zahl nicht übertragen werden konnte und man das einfach ignoriert ist das Programm/System doch sehr wahrscheinlich in einem ungültigen Zustand.

Das ``return True`` am Ende der Funktion macht keinen Sinn.

Das eine Klasse eine Klasse ist muss man nicht in den Namen schreiben. Das sieht man ja schon daran dass der Name mit einem Grossbuchstaben anfängt. Zumindest in dem Quelltext wie er da steht muss das auch gar keine Klasse sein sondern könnte auch einfach(er) als Funktion ausgedrückt werden. Klassen die nur aus einer `__init__()` und *einer* weiteren Methode bestehen sind sehr selten tatsächlich Klassen sondern eigentlich Funktionen die unnötigerweise als Klasse geschrieben wurden.

Da ist immer noch ein ``except:`` ohne konkrete Ausnahme. Auch hier gilt wieder: Wenn da etwas schief gelaufen ist mit dem man nicht gerechnet hat, kann man nicht einfach weitermachen als wäre nichts passiert. Man sollte mindestens die Ausnahme und den Traceback irgendwie protokollieren damit man solchen Fehlern auf die Spur kommen kann.

Zum Thema protokollieren: Es gibt in der Standardbibliothek ein `logging`-Modul. Das würde ich manuellen ``print``\s mit selbst erzeugten Zeitstempeln vorziehen.

Auf Modulebene sollten möglichst nur Konstanten, Funktionen, und Klassen definiert werden. Also auch das Hauptprogramm sollte in einer Funktion stehen und nicht Namen auf Modulebene definieren.