Accensione led in sequenza

Ciao,

sto testando un debounce hardware su due pulsanti (momentary switch) che accendono o spengono una fila di 4 led.
Lo sketch dovrebbe essere banale: se premo il tasto 1 i led si accendono in sequenza (da tutti spenti a tutti 4 accesi); se premo il taso 2 i led si spengono in sequenza.

Acceso il quarto led, la sequenza riparte dall’inizio: tutti spenti. (stesso se ho spento anche il primo led: da tutti spenti si riparte poi da tutti e 4 accesi per poi spegnerli in successione)

Il problema che non riesco a risolvere è questo: se premo i pulsanti in modo ripetitivo, tutto funziona. Se tengo premuto il pulsante, il ciclo sembra arrestarsi all’ultima istruzione e poi riparte saltando la prima. Per capirsi, accesi tutti e 4 i led, mi aspetterei che tenendo premuto il pulsante li spegnesse e ripartisse poi ad accendere il primo…invece non li spegne e riparte direttamente con il primo acceso.

Allego lo sketch. E’ molto banale ma non riesco a capire perchè tenendo premuto i tasti non funziona come dovrebbe.

Massimo

// variabili per la lettura dello stato pulsanti
int val1;
int val2;
// assegnazione variabili dei led
int led01 = 31;
int led02 = 33;
int led03 = 35;
int led04 = 37;

int pgm = 0;   // all'avvio parto da pgm = 0 (tutti e 4 i led spenti)

void setup();

void loop();

void setup()
{
  pinMode(2, INPUT);
  pinMode(4, INPUT);

  pinMode(led01, OUTPUT);
  pinMode(led02, OUTPUT);
  pinMode(led03, OUTPUT);
  pinMode(led04, OUTPUT);

}

void loop()

{
// se ho spento tutti i led il successivo step riparte da tutti e 4 i led accesi
  if (pgm < 0)
  {
    pgm = 4;
  }
// se ho acceso tutti i led il successivo step riparte da tutti e 4 i led spenti
  if (pgm > 4)
  {
    pgm = 0;
  }

  
  val1 = digitalRead(2);
  val2 = digitalRead(4);

  if (val1 == HIGH)
  {
   pgm = pgm + 1;
   delay(400);
  }

  if (val2 == HIGH)
  {
   pgm = pgm - 1;
   delay(400);
  }

   
  if (pgm == 0)
  {
    digitalWrite(led01, LOW);
    digitalWrite(led02, LOW);
    digitalWrite(led03, LOW);
    digitalWrite(led04, LOW);
  }

  if (pgm == 1)
  {
    digitalWrite(led01, HIGH);
    digitalWrite(led02, LOW);
    digitalWrite(led03, LOW);
    digitalWrite(led04, LOW);
  }

  if (pgm == 2)
  {
    digitalWrite(led01, HIGH);
    digitalWrite(led02, HIGH);
    digitalWrite(led03, LOW);
    digitalWrite(led04, LOW);
  }

  if (pgm == 3)
  {
    digitalWrite(led01, HIGH);
    digitalWrite(led02, HIGH);
    digitalWrite(led03, HIGH);
    digitalWrite(led04, LOW);
  }

  if (pgm == 4)
  {
    digitalWrite(led01, HIGH);
    digitalWrite(led02, HIGH);
    digitalWrite(led03, HIGH);
    digitalWrite(led04, HIGH);
  }

}

Primo problema, perchè hai inserito "void setup();" e "void loop();" ?

Primo consiglio, direi di assegnare ad una variabile i pin utilizzato dal tasto, un po' come hai fatto per i led e come suggerisco a seguire

// Da inserire nell'area globale

byte pinKey1 = 2;    // n. pin utilizzato per input
byte Key1 = ;        // stato attuale
byte KeyOld1 ;       // stato precedente loop

byte pinKey2 = 4;    // n. pin utilizzato per input
byte Key2 = ;        // stato attuale
byte KeyOld2 ;       // stato precedente loop

Il problema del tuo sketch è dovuto alla gestione dei tasti.

In generale, quando vuoi che un tasto avvii un blocco di codice specifico, non potrai testare solo il suo stato (high o low), ma dovrai intercettare l'evento "premuto" e "rilasciato".
In questo modo il codice sarà avviato una sola volta per ogni pressione del tasto.

// Da inserire nel "void loop"

  KeyOld1 = Key1;              // salva precedente stato
  Key1= digitalRead(pinKey1);  // legge nuovo stato
  KeyOld2 = Key2;              // salva precedente stato
  Key2= digitalRead(pinKey2);  // legge nuovo stato
  
  if (Key1 != KeyOld1) {       // c'è stata un avariazione
    if (Key1==HIGH) {          // è stato premuto il tasto
      pgm = pgm + 1;           // incrementi il contatore
      if (pgm > 4) pgm = 0;    // verifichi il fondo scala 
      delay(70);
    }
  }

Secondo consiglio direi di sostituire quella serie di "if (pgm=1..." con un più semplice switch

  switch (pgm){
    case 0:
      digitalWrite(led01, LOW);
      digitalWrite(led02, LOW);
      digitalWrite(led03, LOW);
      digitalWrite(led04, LOW);
      break;
    case 1:
      digitalWrite(led01, HIGH);
      digitalWrite(led02, LOW);
      digitalWrite(led03, LOW);
      digitalWrite(led04, LOW);
      break;
    case 2:
    case 3:
    case 4:

  }

Volutamente ho lasciato il codice incompleto per lasciarti il modo di studiarlo e completarlo

Grazie lelebum,

Primo problema, perchè hai inserito "void setup();" e "void loop();" ?

Ma non devono essere inserite sempre queste funzioni?

Sul primo e secondo consiglio non posso che ringraziarti.
Sul commento tra i due, pur essendo molto valido, non è però quello che voglio: ok per l’avanzamento ad ogni pressione del tasto ma vorrei altresì che avanzasse in “automatico” tenendo il tasto premuto.
Come vedi dal mio codice mi aspetterei che tenendo premuto il tasto e giunto ad avere accesi i 4 led, il ciclo continuasse ripartendo dai 4 led spenti e andando nuovamente a riaccenderli in sequenza. E così per tutto il tempo durante il quale è premuto il tasto, invece con il tasto premuto mi salta il ciclo dei 4 led tutti spenti.
Inoltre sembra che subito prima (arrivato ad accendere i 4 led) il loop si blocchi per una frazione di secondo.

Devono esserci si, ma solo "una volta", nel tuo caso sono ripetute. Controlla un esempio tra quelli ufficiali, tipo blinkwithoutdelay.

Avete ragione. Erroraccio mio.

Potrebbe essere questo che fa girare la routine non come me l’aspettavo?

Domani provo a correggere questo e vediamo.

Grazie
Massimo

Le hai soltanto prima dichiarate (col loro prototipo) e poi descritte, nulla di male, non è li il problema, non serve farlo con arduino, salvo casi particolari, ma non fa danni

Sono il primo ad affermare che inserire un "delay(400);" su uno sketch non si fa.
Inoltre il suggeriemento che ti invio va un po' contro la filosofia del suggerimento iniziale, ad ogni modo questa è la modifica che propongo

  if (Key1 != KeyOld1) {       // c'è stata una variazione
    if (Key1==HIGH) {          // è stato premuto il tasto
      pgm = pgm + 1;           // incrementi il contatore
      if (pgm > 4) pgm = 0;    // verifichi il fondo scala
      delay(400);
      if(digitalRead(pinKey1)) // tasto ancora premuto
        Key1=LOW;              // forzi al prossimo loop una variazione di stato
    }
  }

Preciso che per realizzare in modo corretto quanto da te cercato, sarebbe quello di ricorrere alla funzione millis().