Threading funktioniert nicht richtig

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
Antworten
hanmey
User
Beiträge: 6
Registriert: Freitag 3. Oktober 2014, 11:19

Ich habe ein Problem mit dem Multithreading.

Code: Alles auswählen

#!/usr/bin/python

import spidev
import time
import os
import thread
import threading
# Open SPI bus
spi = spidev.SpiDev()
spi.open(0,0)

# Function to read SPI data from MCP3008 chip
# Channel must be an integer 0-7
def ReadChannel(channel):
  adc = spi.xfer2([1,(8+channel)<<4,0])
  data = ((adc[1]&3) << 8) + adc[2]
  return data

class thread1 (threading.Thread):
 def RundenBahn1():
  f1 = open('runden1.txt', 'r+')# just put 'w' if you want to write to the file
  x1 = f1.readline() #this command will read file lines
  f1.close()
  open("runden1.txt","w").close()
  y1 = int(x1)+1
  z1 = str(y1) #making data as string to avoid buffer error
  #print ("Light1: ",z1)
  f1 = open('runden1.txt', 'r+')# just put 'w' if you want to write to the file
  f1.write(z1)
  f1.close()
  time.sleep(10)
  print ("thread1 fertig")
  return
 RundenBahn1()

class thread2 (threading.Thread):
 def RundenBahn2():
  f2 = open('runden2.txt', 'r+')# just put 'w' if you want to write to the file
  x2 = f2.readline() #this command will read file lines
  f2.close()
  open("runden2.txt","w").close()
  y2 = int(x2)+1
  z2 = str(y2) #making data as string to avoid buffer error
  #print ("Light2: ",z2)
  f2 = open('runden2.txt', 'r+')# just put 'w' if you want to write to the file
  f2.write(z2)
  f2.close()
  time.sleep(5)
  print ("thread2 fertig")
  return
 RundenBahn2()

# Define sensor channels
light1_channel = 0
light2_channel = 1
schwellwert = 600
runden1 = 0
runden2 = 0
# Define delay between readings
delay = 5
runden = 0
thread1 = thread1()
thread2 = thread2()
while True:

  # Read the light sensor data
  light1_level = float(ReadChannel(light1_channel))
  light2_level = float(ReadChannel(light2_channel))
  if light1_level <= schwellwert:
   if not thread1.is_alive():
    thread1.start()

  if light2_level <= schwellwert:
   if not thread2.is_alive():
    thread2.start()
Eine kleine Erklärung zum code:
Readchannel() ist zum auslesen von alalogen sensoren mit dem raspberry pi.
für mein Problem eher uninteressant.
Die beiden Classes sollen unabhängig von der while-schleife laufen.
in den classes wird eine txt datei geöffnet und die zahl von dort ausgelesen und +1 gerechnet und wieder gespeichert.
und danach wird eine bestimmte zeit gewartet.
In der while-schleife wird der analoge wert ausgelesen und wenn dieser einen bestimmten schwellwert unterschreitet, soll der thread gestartet werden. die while-schleife soll aber nebenbei weiterlaufen.
Wenn aber der eine thread noch läuft soll er nicht noch einmal gestartet werden. das habe ich mit der If-schleife versucht.
wenn ich das script ausführe kommt folgender Fehler:
Traceback (most recent call last):
File "analogread.py", line 71, in <module>
thread1.start()
File "/usr/lib/python2.7/threading.py", line 489, in start
raise RuntimeError("threads can only be started once")
RuntimeError: threads can only be started once

Kann mir einer helfen?
Benutzeravatar
snafu
User
Beiträge: 6740
Registriert: Donnerstag 21. Februar 2008, 17:31
Wohnort: Gelsenkirchen

Threads können nur einmal gestartet werden, wie die Fehlermeldung ja schon sagt. Du versuchst aber, in deiner ``while``-Schleife einen Thread mehrfach zu starten. Zwar nicht im selben Schleifendurchlauf, aber eben dann, wenn dein Thread einmalig abgearbeitet wurde. Denn dann ist der Test auf ``not thread.is_alive()`` ja positiv und es käme zum zweiten Start.

Wenn du mehrfach einen Thread das selbe tun lassen willst, dann musst du jedes Mal den Thread neu erstellen und dann starten. Deutlich sinnvoller ist es jedoch, dem einmal erstellten Thread bei Bedarf mitzuteilen, dass er nochmal starten soll. Es gibt nämlich durchaus Möglichkeiten, damit Code "von außen" mit dem Thread kommunizieren kann. Und das geht auch weitaus eleganter, als es über temporäre Dateien zu regeln, wie du es ja anscheinend im Moment (zumindest in Ansätzen) machst. Für Details schlage ich die Python-Dokumentation für das ``threading``-Modul vor, sowie ggf 1-2 Tutorials zu dem Thema zum besseren Verständnis.
hanmey
User
Beiträge: 6
Registriert: Freitag 3. Oktober 2014, 11:19

Also die Text-Dateien habe ich erstellt, weil die auf einer Website eingebunden werden.

Einen Thread kann man nicht 2 mal benutzen? Wie kann ich den denn immer wieder neu erstellen?
Und es war bisher immer so dass erst der erste Thread durchgelaufen ist und dann der zweite sollte die schleife nicht weiterlaufen wenn die thread gestartet ist?
BlackJack

@hanmey: Du machst das ganz grundsätzlich falsch mit den Threads. Du erzeugst da nicht wirklich welche, denn die machen gar nichts weil die keine `run()`-Methode haben. Das Du die einzige andere Methode während der definition der Klasse wie eine Funktion aufrufst ist auch *sehr* merkwürdig und unsinnig. Was Du da mit den Dateien anstellst ist auch total umständlich und „falsch”.

`RundenBahn1()` und `RundenBahn2()` machen auch beide das selbe, nur mit einer anderen Datei. Da schreibt man nicht zweimal fast das selbe hin und nummeriert die Namen, sondern schreibt eine Funktion der man die Unterschiede, also den Dateinamen als Argument übergeben kann. Noch unsinniger ist es *lokale* Namen zu nummerieren um sie von *lokalen* Namen in einer *anderen* Funktion/Methode abzugrenzen.

'r+' ist ein Dateimodus den man selten bis gar nicht braucht und insbesondere als Anfänger besser meiden sollte. Ich sehe den Sinn hier auch gar nicht.

Ich denke Du bist insgesamt noch nicht so weit mit nebenläufiger Programmierung zu arbeiten solange die sequentiellen Teile noch so wirr aussehen. Letztendlich braucht man hier IMHO auch gar keine Threads. Man könnte sich auch einfach merken wann das letzte mal der Zähler in der Datei hochgezählt wurde und neben dem Schwellwert auch einfach prüfen ob das schon lange genug her ist um erneut hochzuzählen. Beziehungsweise könnte man davon auch abhängig machen ob man den entsprechenden Sensor überhaupt erst ausliest.

Zur Namensgebung und Quelltextformatierung könnte ein Blick in den Style Guide for Python Code nicht schaden.
hanmey
User
Beiträge: 6
Registriert: Freitag 3. Oktober 2014, 11:19

Hmm...
Das soll ein Rundenzähler für eine Carrerabahn werden. Die Sensoren sind Fotowiderstände, die messen wenn ein auto dazwischen ist. mein Problem war, dass die Schleife öfter durchläuft, wenn das auto dazwischen ist. Dadurch werden zu viele Runden gezählt. Ich habe RundenBahn1() und RundenBahn2(), weil die unabhängig voneinander laufen sollten.
Ich habe mir gedacht, dass ich einfach ein Delay einbauen kann, sodass die Funktion auf jeden Fall länger läuft, als das auto im Zähler ist.
r+ habe ich gelesen, dass dieser zum lesen und schreiben ist damit kann ich doch nichts falsch machen.
Ich versuche nochmal ein bisschen Ordnung hinein zu bringen.
BlackJack

@hanmey: Die Erklärung zu `RundenBahn1()` und `RundenBahn2()` ist keine denn jeder Funktionsaufruf ist ”unabhängig”, man kann also auch *eine* Funktion verwenden und die mit verschiedenen Argumenten aufrufen. Wenn es unbedingt sein muss auch asynchron in jeweils einem eigenen Thread. Es gibt keinen Grund dafür zwei fast identische Funktionen zu schreiben, und schon gar keinen Grund die lokalen Namen in Funktionen so zu wählen das ein lokaler Name in einer Funktion nicht in einer anderen vorkommt obwohl der Name die gleiche Bedeutung hat.

'r+' ist zum lesen und schreiben und das braucht man in 99% der Fälle nicht und man kann damit natürlich auch etwas falsch machen. Machst Du ja. Das ist vom Effekt her nicht falsch, aber unsinnig was Du da machst. Du verwendest den Umstand das Du lesen *und* schreiben kannst gar nicht, denn entweder liest Du nur oder Du schreibst nur, und wegen dem 'r+' bist Du auch ”gezwungen” zwischendurch die Datei mal eben mit dem 'w'-Modus zu öffnen, nur um sie *sofort* wieder zu schliessen. 'r+' macht eigentlich nur bei Binärdateien Sinn bei denen man gezielt bestimmte Stellen ansteuern und lesen oder schreiben möchte. Diese Fälle sind eher selten. Ich weiss gar nicht wann ich dass das letzte mal gebraucht habe. Wenn Du mit 'r' gelesen hättest und mit 'w' geschrieben, dann hättest Du das ”mittlere” `open()` nicht gebraucht. An der Stelle sollte man vielleicht auch erst in eine temporäre Datei schreiben und die dann nach dem schreiben umbenennen, damit der Wechsel des Wertes für den Webserver atomar aussieht. Sonst kann es passieren dass Du ausgerechnet während des schreibens, aber bevor das fertig ist, den Wert ausliest und keinen bekommst weil der neue Wert noch nicht geschrieben wurde.

Edit: Wenn die Signalübergänge eindeutig sind und nicht „flackern”, kann man übrigens auch die Flanken zählen, also eine der beiden.
hanmey
User
Beiträge: 6
Registriert: Freitag 3. Oktober 2014, 11:19

Ok
ich habe jetzt die Funktion zum Hochzählen geändert, sodass ich nur noch eine brauche. Das r+ habe ich jetzt auch nicht mehr.
Wie mache ich das jetzt am besten, dass ich überprüfe ob nochmal hoch gezählt werden muss oder das auto noch zwischen ist?

Code: Alles auswählen

 def NeueRunde(bahn_nummer):
  f = open("runden" + bahn_nummer + ".txt", "r")# just put 'w' if you want to write to the file
  x = f.readline() #this command will read file lines
  f.close()
  open("runden" + bahn_nummer + ".txt","w").close()
  y = int(x)+1
  z = str(y) #making data as string to avoid buffer error
  f = open("runden" + bahn_nummer + ".txt", "w")# just put 'w' if you want to write to the file
  f.write(z)
  f.close()
  time.sleep(10)
  print ("runden" + bahn_nummer + " fertig")
  return
BlackJack

@hanmey: Einrückung ist per Konvention vier Leerzeichen pro Ebene. Selbst wenn man sich nicht daran halten möchte, was ich *nicht* empfehle, ist *ein* Leerzeichen definitiv zu wenig. Da verliert man zu leicht die Übersicht.

Dateien sollte man wenn möglich zusammen mit der ``with``-Anweisung öffnen, dann wird sie beim verlassen des Blocks auch garantiert geschlossen.

Ich hatte ja schon mal auf den Style Guide for Python Code hingewiesen. `NeueRunde()` würde eigentlich `neue_runde()` heissen. Wobei man für Funktionen (und Methoden) in der Regel Tätigkeiten als Namen verwendet, eben weil die etwas tun. Dann kann man sie besser von Namen für andere Werte unterscheiden.

Namen sollten auch aussagekräftig sein und dem Leser vermitteln was der Wert dahinter im Kontext des Programms bedeutet. Einbuchstabige Namen sind das nur sehr selten.

Kommentare sollten nicht das beschreiben was noch einmal ganz offensichtlich im Code steht. In der Regel sind Kommentare nicht dazu da zu beschreiben *was* passiert, das steht da ja schon als Code, sondern *warum* es so passiert — falls das nicht klar sein sollte. Wert bei ``some_file.readline()`` nicht erkennt das dort eine Zeile aus einer Datei gelesen wird, dem wird auch ein Kommentar nicht weiterhelfen der genau das *nochmal* aussagt.

Man sollte keinen Code (oder Daten) wiederholen. Wenn man beispielsweise einen aus Zeichenketten und Werten zusammengesetzten Dateinamen mehr als einmal benötigt, dann setzt man den nur *einmal* zusammen und nicht mehrfach.

Die Zeile 5 macht keinen Sinn mehr.

Weder das Warten noch die Ausgabe gehören in diese Funktion. Funktionen sollten *eine* Aufgabe erledigen und Dateinein- und Ausgabe gehören nicht zusätzlich in Funktionen die schon eine andere Aufgabe erledigen.

Ein nacktes ``return`` am Ende einer Funktion hat keinen Effekt. Die Funktion kehrt am Ende auch ohne diese Anweisung zum Aufrufer zurück, mit genau dem selben Rückgabewert.

Damit Deine Funktion funktioniert muss man `bahn_nummer` als Zeichenkette übergeben, was bei *dem* Namen sehr überraschend ist.

Die Funktion sollte eher so aussehen (ungetestet):

Code: Alles auswählen

def update_round_file(track_number):
    filename = 'round_{0}.txt'.format(track_number)

    with open(filename, 'r') as round_file:
        round_number = int(round_file.read()) + 1
    
    temporary_filename = filename + '.tmp'
    with open(temporary_filename, 'w') as round_file:
        round_file.write(str(round_number))
    
    os.rename(temporary_filename, filename)
    
    return round_number
Wie snafu schon angedeutet hat ist die Datenhaltung in den externen Dateien aber nicht wirklich schön. Die Rundenzähler würde man eher im Programm haben und dann halt für den Webserver auch in die Dateien schreiben, aber nicht den Wert selber dort immer wieder auslesen.

Ansätze die Wartezeit ohne Threads zu machen oder ganz loszuwerden habe ich doch schon gegeben. Ich würde den Signalverlauf nehmen und nur bei aufsteigenden Flanken zählen. Falls das Signal an den Übergängen nicht sauber ist, würde ich erst einmal versuchen durch zwei verschiedene Schwellwerte für die beiden Flanken zu entprellen und nur falls das nicht ausreicht eine Wartezeit beim Übergang einzubauen. Und zwar nicht mit `sleep()` sondern über Zeitstempel. Dazu muss man entweder objektorientierte Programmierung drauf haben oder Generatorfunktionen schreiben können. Beides zu können schadet natürlich auch nicht. :-)
hanmey
User
Beiträge: 6
Registriert: Freitag 3. Oktober 2014, 11:19

Ich weiß nicht genau ob ich die Zeile:

filename = 'round_{0}.txt'.format(track_number)

Richtig verstanden habe, wird für die 0 nun track_number benutzt?
Ich habe die Anzahl der Runden nun im Programm gespeichert. Mit den Variablen: rounds_track_1 und rounds_track_2

Ich habe dein Code noch etwas geändert:

Code: Alles auswählen

def update_round_file(track_number):
    filename = 'round_{0}.txt'.format(track_number)
    rounds_yet = 'rounds_track_{0}'.format(track_number)

    round_number = int(rounds_yet) + 1
	
    temporary_filename = filename + '.tmp'
    with open(temporary_filename, 'w') as round_file:
        round_file.write(str(round_number))
   
    os.rename(temporary_filename, filename)
   
    return round_number
Und in der schleife kann ich ja abfragen ob in den vorherigen Durchlauf der Schwellwert auch schon unterschritten wurde mit einer Variable in der nur true oder false steht.
Den Namen weiß ich noch nicht. :?
BlackJack

@hanmey: Die `format()`-Methode ist dokumentiert genau wie die ”Minisprache” für die Platzhalter. Ausserdem könnte man das auch einfach mal in einer Python-Shell ausprobieren.

Apropos ausprobieren: Das hast Du mit der geänderten Funktion offensichtlich nicht getan, denn die versucht eine Zeichenkette mit Buchstaben in eine Zahl umzuwandeln. Was ganz sicher nicht funktionieren wird.

Solange Du Dich um Generatorfunktionen oder OOP drückst wird der Code in dem die Hauptschleife steht immer unübersichtlicher werden und immer mehr einzelne durchnummerierte Namen enthalten die eigentlich in eine Datenstruktur zusammengefasst werden sollten. Also einmal die zusammengehörigen Namen, und das dann auch noch mal zum Beispiel in eine Liste um die Nummernzusätze loszuwerden.

Man sollte das Problem für *einen* Rundenzähler lösen und dann den *gleichen* Code für weitere Rundenzähler verwenden können. Denn der Code an sich ist ja der selbe für beide Rundenzähler, es ändern sich nur ein paar Werte wie Kanalnummer für die Hardware.
BlackJack

Ungetestet:

Code: Alles auswählen

#!/usr/bin/env python
# coding: utf8
from __future__ import absolute_import, division, print_function
import os
from contextlib import contextmanager
from functools import partial

from spidev import SpiDev


@contextmanager
def opened_spi_device(bus, device):
    spi_device = SpiDev()
    spi_device.open(bus, device)
    try:
        yield spi_device
    finally:
        spi_device.close()


def read_channel(spi_device, channel_number):
    if not 0 <= channel_number <= 7:
        raise ValueError('channel number out of range')
    values = spi_device.xfer2([1, (8 + channel_number) << 4, 0])
    return ((values[1] & 0b11) << 8) + values[2]


def iter_falling_egdes(read_function, threshold):
    previous_result = None
    while True:
        result = read_function() < threshold
        yield result and previous_result != result
        previous_result = result


class Track(object):
    def __init__(self, number, egdes):
        self.number = number
        self.egdes = egdes
        self.round_number = 0

    def check_round(self):
        result = next(self.egdes)
        if result:
            self.round_number += 1
        return result

    def save_round(self, filename_template='round_track_{0}.txt'):
        filename = filename_template.format(self.number)
        temporary_filename = filename + '.tmp'
        with open(temporary_filename, 'w') as round_file:
            round_file.write('{0}\n'.format(self.round_number))
        os.rename(temporary_filename, filename)


def main():
    threshold = 600
    channel_numbers = [0, 1]
    with opened_spi_device(0, 0) as spi_device:
        tracks = [
            Track(
                i,
                iter_falling_egdes(
                    partial(read_channel, spi_device, channel_number), threshold
                )
            )
            for i, channel_number in enumerate(channel_numbers, 1)
        ]
        try:
            while True:
                for track in tracks:
                    if track.check_round():
                        track.save_round()
        except KeyboardInterrupt:
            pass  # Intentionally ignored.


if __name__ == '__main__':
    main()
Zuletzt geändert von BlackJack am Samstag 4. Oktober 2014, 18:53, insgesamt 1-mal geändert.
Grund: try/finally hinzugefügt.
hanmey
User
Beiträge: 6
Registriert: Freitag 3. Oktober 2014, 11:19

Super! Danke :D
Es funktioniert einwandfrei.
Obwohl ich noch nicht genau weiß wie, das muss ich mir dann nochmal ansehen.
DasIch
User
Beiträge: 2718
Registriert: Montag 19. Mai 2008, 04:21
Wohnort: Berlin

@BlackJack: Wenn eine Exception auftritt schliesst opened_spi_device() das Device nicht. Wäre es da nicht sinnvoll ein try..finally bzw. contextlib.closing zu nutzen?
BlackJack

@DasIch: Verdammt, ich ging davon aus das `contextmanager()` das für mich erledigt. Habe ich oben ausgebessert. Danke für den Hinweis.
Benutzeravatar
snafu
User
Beiträge: 6740
Registriert: Donnerstag 21. Februar 2008, 17:31
Wohnort: Gelsenkirchen

Was spricht gegen ein einfaches ``with closing(SpiDev().open(bus, device)) as spi_device`` anstelle einer eigenen Funktion?

EDIT: Oder gibt die ``.open``-Methode kein passendes Objekt zurück?
BlackJack

@snafu: Die Methode sollte eigentlich `None` zurück geben. Die öffnet/erzeugt ja kein neues Objekt, sondern verändert bloss den Zustand des Objekts auf dem sie aufgerufen wurde.
Antworten