Seltsamer Fehler bei einfachem LED-Spiel

Hallo,
ich versuche gerade mit dem Arduino Nano ein kleines Geschicklichkeitsspiel zu bauen. Ist recht simpel. An Pin 2,3 und 4 hängt jeweils über einen 220 Ohm Widerstand eine LED. Die LEDs leuchten immer nacheinander auf (also 2,3,4,3,2,3,4....). Desweiteren hängt ein Taster am Eingang A5, der über einen 1k Widerstand mit GROUND verbunden ist.
Aufgabe des Spielers ist es, den Knopf dann zu drücken, wenn die mittlere LED aufleuchtet (hier an Pin 3).

  • Drückt der Spieler während die mittlere LED aufleuchtet den Taster, leuchtet die mittlere LED fünfmal kurz auf und es verkürzt sich das Intervall, indem die Lampen aufleuchten, um 50 Millisekunden, womit es in der nächsten "Runde" schwerer wird.
  • Drückt der Spieler den Taster, während die linke oder rechte LED aufleuchtet, ist das Spiel verloren, die linke und rechte LED leuchten fünfmal kurz auf und es beginnt wieder von vorne mit dem Intervall 1000 Millisekunden.

Der Fehler liegt darin, dass es beim Probespielen ein paar Mal vorgekommen ist, dass alle 3 LEDs auf einmal leuchten und der Arduino nicht mehr auf den Druck des Tasters reagiert, als ob er in einer Endlosschleife wäre, während alle 3 LEDs leuchten. Leider kann ich den Fehler nicht finden.

Könnt ihr mir helfen ?

Mein Code ist folgender :

unsigned long stime = 0; //Startzeit
unsigned long etime = 0; //Endzeit
int intervall = 1000;
int Error = 0;

void setup() {
  // put your setup code here, to run once:
  pinMode(2, OUTPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(A5, INPUT);
}

void allOff()   //alle LEDs aus
{
  digitalWrite(2, LOW);
  digitalWrite(3, LOW);
  digitalWrite(4, LOW);
}

//Taster wurde im falschen Moment gedrückt und
//und Spiel beginnt von vorne
void ErrorSignal()  //Fehler
{
  allOff();
  delay(1000);
  //kurzes Aufblinken der linken und rechten LEDs
  for (int i=0; i<5; i++)
  {
    digitalWrite(2, HIGH);
    digitalWrite(4, HIGH);
    delay(200);
    digitalWrite(2, LOW);
    digitalWrite(4, LOW);
    delay(200); 
  }
  intervall = 1000;
}

//Taster wurde im richtigen Moment gedrückt und
//die Intervallzeit wird um 50 ms verkürzt
void SuccessSignal()  //Erfolg
{
  allOff();
  delay(1000);
  //kurzes Aufblinken der mittleren LED
  for (int i=0; i<5; i++)
  {
    digitalWrite(3, HIGH);
    delay(200);
    digitalWrite(3, LOW);
    delay(200); 
  }
  intervall = intervall - 50;  //Zeit um 50 ms verringern
}

void loop() {

  //alle LEDs ausschalten 
  allOff();


  //Linke LED aufleuchten
  digitalWrite(2, HIGH);
  stime = millis();
  while (etime - stime < intervall)
  {
    if (analogRead(A5) > 500)
    {
      Error++;
    }
    etime = millis();
  }
  digitalWrite(2, LOW);

  //Mittlere LED aufleuchten  
  digitalWrite(3, HIGH);
  stime = millis();
  while (etime - stime < intervall)
  {
    if (analogRead(A5) > 500)
    {
      SuccessSignal();
    }
    etime = millis();
  }
  digitalWrite(3, LOW);

  //Rechte LED aufleuchten
  digitalWrite(4, HIGH);
  stime = millis();
  while (etime - stime < intervall) 
  {
    if (analogRead(A5) > 500)
    {
      ErrorSignal();
    }
    etime = millis();
  }
  digitalWrite(4, LOW);

  //Mittlere LED aufleuchten  
  digitalWrite(3, HIGH);
  stime = millis();
  while (etime - stime < intervall)
  {
    if (analogRead(A5) > 500)
    {
      SuccessSignal();
    }
    etime = millis();
  }
  digitalWrite(3, LOW);
}

Viele Grüße,
UnoRookie

Keine Ahnung ob das dein Problem ist, aber intervall kann negativ werden. Dann funktioniert dein Vergleich

etime - stime < intervall

nicht wie gedacht.
Und einen Taster würde ich nicht per analogRead lesen

Das ist gar nicht so seltsam.
Weil etime (EndZeit) immer nur innerhalb der while-Loop einen neuen Wert bekommt, die while-Loop aber nur ausgeführt wird wenn etime - stime < intervall
kommt es bald mal vor, dass die while-Loop gar nicht mehr ausgeführt wird. - Überprüf das doch mal mit seriellen Ausgaben.

Das eigentliche Problem ist, dass etime - stime negativ werden kann.
Weil beide aber unsigned sind, kommt es zu einem Überlauf und das ergibt dann eine "sehr große" Zahl, die jedenfalls viel größer als intervall ist. Dadurch werden die while-Loops nicht mehr ausgeführt. Und die LEDs flackern nur noch.

Serielle Debug-Ausgaben - zu sehen ist der Überlauf. Ab diesem Moment flackern die LEDs nur noch:

stime        etime      Differenz (e-s)
-------------------------------------------
...
38389        38389        0
38839        38839        0
41840        41840        0
44841        44840        4294967295
44843        44840        4294967293
...

Du kannst das verhindern indem du etime öfter mal einen "plausiblen" Wert zuweist.
Zum Beispiel so:

//Linke LED aufleuchten
  digitalWrite(2, HIGH);
  stime = millis();
  etime = stime;    //  <--------
  while (etime - stime < intervall)
  {
    if (analogRead(A5) > 500)
    {
      Error++;
    }
    etime = millis();
  }
  digitalWrite(2, LOW);

Aber generell würde ich mir überlegen, das anders zu lösen und auf die vielen while-Loops zu verzichten.
Du hast ja schon eine "wunderbare" Schleife die heißt "loop" und die sollte so flott wie möglich durchlaufen werden... :slight_smile:
Aber das ist natürlich auch irgendwie Geschmackssache.

Auf zusätzliche "Designprobleme" hat michael_x ja bereits hingewiesen.

Vielen Dank. Das mit etime=stime hat funktioniert. Ich versteh nur nicht ganz, warum es nicht funktioniert hat.
Denn das negative Ergebnis von etime-stime kann doch eigentlich erst nach ca. 49 Tagen auftreten, da, soviel ich gelesen hab, erst dann millis wieder bei 0 beginnt und solang hab ich nie gespielt

Das Problem hat mit dem Überlauf nach 49 Tagen nichts zu tun.
Der Stolperstein ist hier:

...
while (etime - stime < intervall)
  {
    if (analogRead(A5) > 500)
    {
      ErrorSignal();
    }
    etime = millis();   // <-------- A) etime wird millis() zugewiesen
  }
  digitalWrite(4, LOW);

  //Mittlere LED aufleuchten
  digitalWrite(3, HIGH);
  stime = millis();  // <------- B) stime wird millis() zugewiesen
  while (etime - stime < intervall)  // <------- C) Rechnung: etime - stime
  {
    ...

Bei A) wird etime millis() zugewiesen, danach gibt es mehrere Anweisungen.
Bei B) wird stime millis() zugewiesen.
Zwischen A) und B) vergeht etwas Zeit. Nicht viel und wohl deutlich weniger als 1 Millisekunde.
Deshalb haben in vielen Fällen etime und stime den gleichen (millis-) Wert und die Differenz (Punkt C) ist dann Null.

Es kann aber passieren, dass Punkt A) gerade zu einem Moment erreicht wird, sehr knapp bevor millis() um 1 weiterzählt. Dann ist etime um eins kleiner als stime und die Rechnung etime minus stime (Punkt C) wird "eigentlich" negativ (genauer: -1), weil das aber unsigned-long-Variablen sind, können diese keine negativen Werte haben. Es kommt zu einem "Überlauf" und statt -1 ergibt sich der Wert 4294967295.
Und weil das deutlich größer als intervall ist werden die while-Loops nicht mehr ausgeführt - nie mehr, weil etime keinen neuen Wert mehr zugewiesen bekommt.
Edit: Erst nach etwa 49 Tagen (nach dem Überlauf von millis) ist stime wieder kleiner als etime.
Aber so lange warten wohl nur die wenigsten Spieler :slight_smile:

Das ist wohl eine Race Condition.

Deswegen speichere ich den aktuellen millis-Stand für jeden loop-Durchlauf und benutze diesen, wenn gebraucht.

void loop() {
unsigned long aktMillis = millis();
...
lastMillis = aktMillis;
eTime = aktMillis;
...

}

Gruß Tommy

Deswegen speichere ich den aktuellen millis-Stand für jeden loop-Durchlauf und benutze diesen, wenn gebraucht.

Und ich isoliere alle Zeitabhandlungen voneinander.
Vollständig.
Dann können sie sich nicht ins Gehege kommen.

Aber, egal wie man es tut ...
Es gilt immer der alte Grundsatz:
Kaum macht man es richtig, schon funktioniert es.