Switch Case - Funktion geht nicht

International Deutsch
1

Hi Leute, ich habe folgendes Problem, ich habe eigentlich ein einfaches Programm, bei dem alle Leds erst einmal mit kurzem Delay an gehen, dann kann ich zwei abschalten und am Ende möchte ich ein zufälliges langsames Blinken.

Soweit so gut, mit der Funktion Blinken scheitert es jedoch gerade. Es wird lediglich eine Led abgeschaltet, das wars aber auch schon :confused:

Bin ich blind oder was habe ich übersehen oder vergessen?

Vielen Dank schon mal für eure Hilfe :wink:

int BUTTON_PIN = A1;

int led1 = 2;
int led2 = 3;
int led3 = 4;
int led4 = 5;
int led5 = 6;
int led6 = 7;


int leds[6] = {2,3,4,5,6,7};


bool oldState = LOW;
int showType = 0;

void setup() {
  pinMode(BUTTON_PIN, INPUT_PULLUP);
  pinMode(led1, OUTPUT);
  pinMode(led2, OUTPUT);
  pinMode(led3, OUTPUT);
  pinMode(led4, OUTPUT);
  pinMode(led5, OUTPUT);
  pinMode(led6, OUTPUT);
  
 for (int jj; jj<sizeof(leds)/sizeof(int);jj++){
  pinMode(leds[jj],OUTPUT);
  delay(10);
 }
}

void loop() {
  // Get current button state.
  bool newState = digitalRead(BUTTON_PIN);

  // Check if state changed from high to low (button press).
  if (newState == LOW && oldState == HIGH) {
    // Short delay to debounce button.
    delay(20);
    // Check if button is still low after debounce.
    newState = digitalRead(BUTTON_PIN);
    if (newState == LOW) {
      showType++;
      if (showType > 4)
        showType=0;
      startShow(showType);
    }
  }

  // Set the last button state to the old state.
  oldState = newState;
}

void startShow(int i) {
  switch(i){
    case 0:  
             digitalWrite(led1, LOW);
             delay(100);
             digitalWrite(led2, LOW);
             delay(100);
             digitalWrite(led3, LOW);
             delay(100);
             digitalWrite(led4, LOW);
             delay(100);
             digitalWrite(led5, LOW);
             delay(100);
             digitalWrite(led6, LOW);
            break;
    case 1: 
             digitalWrite(led1, HIGH);
             delay(100);
             digitalWrite(led2, HIGH);
             delay(100);
             digitalWrite(led3, HIGH);
             delay(100);
             digitalWrite(led4, HIGH);
             delay(100);
             digitalWrite(led5, HIGH);
             delay(100);
             digitalWrite(led6, HIGH);
             delay(1000);
            break;
    case 2:               
             digitalWrite(led1, LOW);
             delay(500);
            break;

    case 3:
             digitalWrite(led4, LOW);
             delay(500);
            break; 
    case 4: 
            Blink();
            break; 
  }
}



void Blink(){
  digitalWrite(leds[random(0,sizeof(leds)/sizeof(int))],HIGH);
  delay(500);
  digitalWrite(leds[random(0,sizeof(leds)/sizeof(int))],LOW);
  delay(500);
}
2

bau dir in jeden case sowie in deine blink() Funktion serial.println Ausgaben ein, damit du siehst wo dein Programm sich verfährt.
Wenn du es noch nicht erkennst, gib dir auch die jeweils wichtigen Variablen aus.
Ist imho nur ein Ablauffehler deinerseits. Du löst nur Aktionen mit Tastendruck aus, das Hauptprogramm kommt in weiterer Folge per Zeitablauf nicht bei Case 4 vorbei.

Wenn du aber im Loop bei showType==4 auch dein blink() aufrufst, wirst du vemutlich bemerken, dass deine Taster nur mehr sporadisch funktionieren, weil deine blink() blockierend abläuft. du schickst den Arduino mit delay() quasi schlafen. Mindestens die blink() solltest du umbauen. Nimm dazu das Beispiel "Blink without delay" aus der IDE.

P.S. du setzt im Setup zweimal den Pinmode, das ist so nicht notwendig, die pins hast eh einzeln auf Output gesetzt

3

Was erwartest Du dir von deiner Blink() Funktion? Derzeit wird sie - wie alle anderen cases auch - bei einem Tastendruck genau einmal aufgerufen. Da wird dann eine zufällige Led ein- und eine zufällig bestimmte Led ausgeschaltet. Das wars. Wenn diese zufällig bestimmte Led bereits ein- bzw ausgeschaltet war, passiert garnichts. Damit es blinkt, müsstest Du die Funktion immer wieder aufrufen - da sehe ich aber nichts von.

Und wie schon geschrieben - das mit den delays ist eh nichts gescheites. Das solltest Du ohne delay realiseren (ist auch eine schöne Übung :wink: ).

4

Hi

Warum hast Du die Pin-Nummern sowohl als einzelne int (warum int?), wie auch in einem array (ebenfalls ... warum int?) abgelegt?
Das reicht imho ein Mal - auch ist's einfacher, wenn man die Pin-Nummern nur 1x im Sketch stehen hat und nicht an zwölf Stellen suchen muß.
Konstante Dinge dürfen dem Kompiler auch so übergeben werden, wenn byte reicht (unter 256 Pins reicht Byte) wäre MEIN Array vom Typ byte und hätte zusätzlich ein const (für Konstant, ändert sich nicht während der Laufzeit), wodurch der Kompiler diese Zahlen wirklich als Zahlen im Code einbauen kann und keine langsameren Variablen-Zugriffe dafür braucht.

Delay() wurde ja schon gesagt - zumindest 'unschön' - sobald Du etwas mehr machen willst, als einzelne LEDs nacheinander anzusteuern, fällt Dir Das auf die Füße.

MfG

5 
if (newState == LOW && oldState == HIGH) 
{
    ...
    if (newState == LOW) 
   {
        ...
    }
}

Das zweite if ist nutzlos, da es nur ausgeführt wird, wenn das erste if richtig ist und in diesem Falle immer.

Um das zu programmieren was Du willst brauchst Du millis() für das Timing und einen Endlichen Automaten damit das dauernd gemacht wird was es soll und nicht nur einmal bei Tastendruck.
Das switch case, das Du has,t ist kein endlicher Automat, weil er nur im Falle des Tastendrückens aufgerufen wird. Bei einem endlichen Automaten läuft das Switch case dauernd und die Statusvariablen werden unabhängig davon gesetzt.

Grüße Uwe

6

Wenn wir schon so am Meckern sind:

....

 for (int jj; jj<sizeof(leds)/sizeof(int);jj++){

int jj[b]=0[/b]; ist wesentlich, da das eine lokale variable mit ansonsten undefiniertem und evtl. sogar negativem Anfangswert ist.

Die foreach Konstruktion bei neuerem c++ wie es der Arduino bietet, erspart nicht nur Tipp-Arbeit und ist, wenn man sich erstmal dran gewöhnt hat, leichter lesbar. Sie vermeidet auch unnötige Flüchtigkeitsfehler.

 for (int pin: leds) {
  pinMode(pin,OUTPUT);
  delay(10);  // An der Wartezeit kann man ablesen, wie viele leds es waren  :=)  ?  ?
 }

Statt int schreiben manche sogar auto&, wenn du noch mehr neumodischen Kram lernen willst.
Ein const byte leds[] {...}; wäre aber auch schonmal besser als int leds[] :slight_smile: