Un tasto, due led indipendenti

buongiorno a tutti
Dopo aver risolto grazie ai vostri consigli la parte hardware del mio progetto, ho cominciato a scrivere la parte di codice.
Sono riuscito a far funzionare tutto tranne un pezzo.

In pratica mentre premo un tasto si deve accendere un led dopo un tot di tempo, aspetta ancora un po di tempo e fa accendere il secondo led.
al rilascio del tasto il primo led deve spegnersi e rimanere acceso solo il secondo.

Se premo nuovamente il tasto deve spegnersi il secondo led.

La prima parte sono più o meno riuscito a farla funzionare, non riesco a far spegnere il secondo led in nessun modo, saranno 15 giorni che ci sbatto la testa senza risultati.

#define rele 12
#define valv 11
#define start 9
long pausa =1000;
 long tempotasto = 3000;
 long tempospegnimento = 350;
 long iniziotasto = 0;
int statorele = LOW;
int statovalvola = LOW;
int inmoto = LOW;

long statopausa = 0;
long statopausa1 =0;





void setup()
{
  pinMode(rele, OUTPUT);
  pinMode (valv, OUTPUT);
  pinMode(9, INPUT);
  Serial.begin(9600); 
  
}



void loop(){
 
 while((digitalRead(9) == HIGH)&& (inmoto== LOW)){                              //comincia il primo rele ad accendersi
  
if(digitalRead(9) == HIGH){
statopausa=millis();
  if(millis()>= pausa){
    statovalvola=!statovalvola;
    digitalWrite(valv,HIGH);
    Serial.println("valvola");

  } 
  
 }

 else{
  digitalWrite(valv,LOW);
  }
 
  if ((digitalRead(9) == HIGH) && (statovalvola == HIGH) && (statorele == LOW)){        // si accende anche il secondo
  
   
  iniziotasto= millis();
  if(millis() >= tempotasto){
    
    
      
      digitalWrite(rele,HIGH);
      
      statorele = !statorele;
      
     Serial.println("RELEoooooooON");3
       Serial.println(inmoto);
      Serial.println(statomotorino);
      Serial.println(statorele);

      
     
      
         
         
            }
      
   
 }

 }
 if (digitalRead(9) == LOW){
 
  digitalWrite(valv,LOW);
  }

if((digitalRead(9) == HIGH) && (statorele == HIGH)){
    
    
    digitalWrite(rele,LOW);
    statorele = !statorele;

il codice è un pò pasticciato visto le innumerevoli prove.
altra piccola cosa, se premo il tasto senza lasciarlo fa tutto diciamo giusto, se lo lascio e lo ripremo, non riparte a contare da 0 ma è come se tiene in memoria il tempo precedente e non fa tutta la routine di attesa.

Perdonatemi perchè avrò scritto un sacco di castronerie ma da autodidatta non sono riuscito a fare di meglio.

grazie

Per cortesia, sistema un po' il codice togliendo le righe vuote e facendo un CTRL+T sull'IDE!
Inoltre, le variabili che devono copiare millis() devono essere uint32_t (unsigned long) e quelle che devono contenere solo true/false o HIGH/LOW devono essere bool.
Per i nomi delle variabili, poi, usa nomi comprensibili! Io per le variabili che copiano millis() uso sempre nomi che iniziano con t_, per esempio t_press_tasto, t_rilascio_tasto.
Per i nomi composti si usa separare le parole con _ oppure mettendo lettere maiuscole all'inizio delle parole esclusa la prima, per esempio in_moto oppure inMoto; stato_valvola oppure statoValvola.

stavo per scriverlo io

il fatto di essere un neofita non ti esime dallo scrivere bene i programmi
anzi, devi proprio scriverli bene, se non altro per aiutarti a trovare i problemi

quindi fai un bel Ctrl-T e sopratutto:
mettilo tutto il programma, che adesso è tronco

e comunque ci sono altri 2 grossi errori
1: non hai specificato cosa deve succedere nei casi anomali:

  • se rilasci prima il pulsante
  • lo lo rilasci prima E lo ripreni prima cha passi il tempo

2: stai riutilizzando pezzi di codice che non c'entrano nulla
posso capire per una prova, ma se sono due settimane che ci sbatti contro, non ti è mai venuto in mente di ripartire da capo buttando via tutti i casini che avevi scritto?
avresti fatto meno tempo

3 domanda vitale
ma il dbounce lo hai considerato?

Alle cose che giustamente ti hanno consigliato, aggiungo anche una descrizione, che riporto da un mio precedente post in altro thread.

Per gestire correttamente cose di questo tipo (ma sono molti i casi, visto che parliamo di un microcontrollore di "qualcosa") evitando cicli bloccanti è quello di iniziare disegnando una macchina a stati finiti, o FSM, ed una volta fatto si deve implementare nel codice usando almeno una variabile che conservi lo stato corrente del sistema ed in base a questo operare

Ovviamente potresti anche ignorare questa definizione, perché un programma relativamente semplice come quello che hai descritto può anche farne a meno, ma ti assicuro che questo è il modo migliore di gestire eventi e controlli, ed il tempo che impiegherai a cercare di implementare una macchina a stati finiti sarà ben speso perché lo userai parecchie volte.

Nel tuo caso ad esempio, un elenco di stati potrebbe essere questo:

S_OFF: tutto spento
S_LED1_ON: accedo il primo led
S_LED12_ON: accendo anche il secondo
S_LED2_ON: acceso solo il secondo

Quindi in genere o si fa un disegno, o almeno uno schema di transizione di stato ossia righe nel formato:

stato corrente: SE condizione ALLORA fai questa azione PASSA a questo stato

Queste condizioni puoi derivarle direttamente dalla tua descrizione (S_OFF sarà lo stato iniziale), un esempio da cui partire (devi eventualmente verificare tu questi passaggi):

S_OFF: SE premo il tasto ALLORA accendi LED1 e PASSA a S_LED_1_ON
S_LED1_ON: SE rilascio il tasto ALLORA spegni LED1 e PASSA a S_OFF
S_LED1_ON: SE sono passati "x" secondi ALLORA accendi LED2 e PASSA a S_LED12_ON
S_LED12_ON: SE rilascio il tasto ALLORA spegni LED1 e PASSA a S_LED2_ON
S_LED2_ON: SE premo il tasto ALLORA spegni LED2, attendi che venga rilasciato, e PASSA a S_OFF

Comprendo che per chi inizia ad affacciarsi a questo mondo (soprattutto per la programmazione) possa sembrare complicato, ma, come dicevo prima, fidati che se vuoi iniziare ad affrontare questo divertente moldo dei microcontrollori è molto utile e ti servirà sicuramente quando vorrai creare cose più complesse.

Aggiungerei un paio di stati

Beh, si ma intanto quando ho scritto "attendi che venga rilasciato" ho voluto risparmiargli uno stato. :wink:
Poi ho anche scritto "(devi eventualmente verificare tu questi passaggi)" proprio per non dargli proprio tuttotutto già fatto, e vedere se ha capito il concetto (e quindi capire cosa manca)... :wink:

Certo, ma all'inizio è un muro verticale anche avendo gli stati completi. Tra l'altro su un PLC questo specifico caso si risolverebbe molto più sinteticamente in ladder con solo quattro rung... che equivalgono a quattro righe equivalenti di C.

Rimane non chiarito che comportamento deve avere il sistema se il tasto viene rilasciato in un qualsiasi momento intermedio tra l'inizio e l'accensione di LED2.

Grazie a tutti per le risposte e le "bacchettate".
In questi giorni mi rimetto d'impegno e riparto da zero con i vostri consigli e vediamo cosa esce, vi tengo aggiornati.
Grazie ancora

allora come promesso ho resettato tutto e ripensato anche come macchina a stati.
in effetti sono riuscito a fare tutto quello che volevo ma mi perdo probabilmente in usa stupidata.
il "maledetto uso del millis()"...

vi spiego il mio problema:
al primo giro di utilizzo va tutto bene perchè vado veloce a premere il tasto,
se aspetto come dovrebbe essere nella realtà, quando ripremo nuovamente il tasto per far spegnere il rele, si spegne all'istante senza aspettare il tempo richiesto.
potete gentilmente aiutarmi a capire dove sbaglio con i millis()?
Vi ricordo che sono un manutentore e non un programmatore, se ho cappellato di brutto abbiate pietà di me

#define rele 12
#define valv 11
#define start 9

int statorele = LOW;
int statovalv = LOW;
int accesa = LOW;
int statostart = 0;
int stato = 0;

unsigned long tspegnimento;
unsigned long statopausa;
unsigned long statopausa1;
unsigned long t;
unsigned long ta;

void setup() {
  pinMode(9, INPUT);
  pinMode(12, OUTPUT); 
  pinMode (11, OUTPUT); 
  Serial.begin(9600);
  statopausa = millis();
  stato = 0;
}

void loop() {
  
  switch (stato) {
    case 0:
      loop0();
    break; 
    case 1:
      loop1();
    break; 
    case 2:
      loop2();
    break; 
    case 3:
      loop3();
    break;
    case 4:
      loop4();
    break;
   
   
   
  }
}

void loop0() {  
  
 if((digitalRead(9) == HIGH) && (statorele == LOW)){                                 // comincio ad aprire la valvola                   
    statopausa = millis();
  
  if(statopausa-t >= 1500){
    statovalv=HIGH;
    digitalWrite(valv,HIGH);
    t = millis();
   stato = 1;
   } 
   }
  
   else{
  digitalWrite(valv,LOW);
  }
 }
  
void loop1() {
  
  
  if ((digitalRead(9) == HIGH) && (accesa == LOW) && (statovalv == HIGH)){      // dopo un tot di tempo si inserisce il rele
  statopausa1 = millis();
  if(statopausa1-t >= 2000){    
      Serial.println("RELE ON");
      digitalWrite(rele,HIGH);
      accesa = !accesa;
      t = millis(); 
      stato = 2;   
 }
 }
 }

  void loop2(){
    if(digitalRead(9) == LOW){                                                // al rilascio del tasto la valvola si chiude e rimane eccitato solo il rele
     
      digitalWrite(valv, LOW);
      digitalWrite(rele,HIGH);
      statorele = HIGH;
      statovalv = LOW;
      statostart = 1;     
      stato = 3;
      ta = millis();
 }
 }

  void loop3(){                                                            // passato un tot di tempo dalla pressione del tasto si spegne anche il rele
 
   if(digitalRead(9)==HIGH){
   
    tspegnimento = millis();
    if(tspegnimento-ta >= 2000){
    digitalWrite(rele,LOW);
    statorele = LOW;
    t = millis();
    stato = 4;
    accesa = !accesa;
    
    }
   }
  } 

  void loop4(){
    if(digitalRead(9)==LOW){
      
      stato = 0;
      }
    }


Io mi occupo di manutenzione da più di 20 anni ormai e ne avrei di cose da dire a presunti programmatori, specialmente quelli tedeschi che sono molto sopravvalutati :rofl: :rofl:

Scherzi a parte, qualche consiglio:

  • evita sempre (!) di usare nomi di variabili e funzioni che non sono significativi. t, ta, loop1, loop2 etc etc non significano nulla... in questo momento per te è chiaro il loro scopo, ma se rimetti mani a questo software tra qualche tempo ti troverai esattamente nello stato in cui mi trovo adesso io nel cercare di comprendere cosa volevi realizzare: ovvero brancolerai nel buio.

  • stessa cosa per gli stati: ad esempio stato == 0 è poco rappresentativo. Ma se usi un'enumerazione diventa tutto molto più leggibile.

  • cura di più l'indentazione del software e commenta i passaggi più significativi per le stesse ragioni di prima. Possono sembrare cose di poco conto, ma fanno la differenza tra un programma funzionante ed uno che non lo è (perché non si riesce a capire cosa non va!).

  • io non amo molto le macchine a stati implementate con lo switch/case perché quando sono più complesse della tua è un attimo perdere di vista il modello stesso della macchina a stati e se vuoi modificarne il funzionamento devi rivedere tutto il software-
    Per questa ragione secondo me è preferibile cambiare lo stato principalmente nello switch/case invece che nelle funzioni sparpagliate in giro.

Ad ogni modo, la tua è piuttosto semplice da modellare quindi va bene cosi.
Il consiglio è quello di trattare anche i momenti di "attesa" come degli stati intermedi: se nel momento in cui premi il pulsante la seconda volta devi aspettare 1 secondo, metti la tua macchina nello stato di attesa intermedio ed aspetti 1 secondo prima di passare al successivo.

Chiaramente questo esempio non ha nulla a che vedere con il tuo sketch, è solo per rendere esplicito il concetto che ho cercato di descrivere.

unsigned long t_passaggioStato;
enum { ATTIVA_RELE=0, ATTENDI_1 , ACCENDI_LED, ATTENDI_2, SPEGNI_TUTTO};
.....
switch (stato) {
    case ATTIVA_RELE:
      unaFunzioneCheAttivaRele();
      t_passaggioStato = millis();
      stato = ATTENDI_1;
      break;

    case ATTENDI_1 :
      if (millis() - t_passaggioStato > 1000) {
        stato = ACCENDI_LED;
      }
      break;

    case ACCENDI_LED :
      unaFunzioneCheAccendeLed();
      t_passaggioStato = millis();
      stato = ATTENDI_2;
      break;
    
    case ATTENDI_2 :
      if (millis() - t_passaggioStato > 5000) {
        stato = SPEGNI_TUTTO;
      }
      break;

    case SPEGNI_TUTTO:
      spengoTutto();
      break;
  }

Perché nello stato 0 controlli un timeout? Non basta controllare il tasto di partenza?

Il problema del tempo sballato al secondo giro dipende dal fatto che misuri il tempo trascorso dall'ultima volta che la variabile t è stata aggiornata. E questo errore è consequenziale al fatto che nel primo stato non dovresti controllare nessun timeout, ma solo caricare la variabile col tempo attuale nel momento in cui il tasto viene premuto.

Invece, ancora più grave, In loop1 non è stato gestito il caso di rilascio del pulsante.

AGGIUNTA: ho capito il perché del primo controllo tempo, manca uno stato intermedio.... Il primo stato deve solo controllare la pressione, il secondo (mancante) la pausa iniziale, e al terzo deve iniziare ad accendersi qualcosa.

Ho capito, perciò devo fare più loop e non racchiudere più funzioni in un loop solo.
Però scusatemi non ho capito dove sbaglio con la millis.
In questi giorni mi rimetto sotto e riprovo ancora
Grazie mille

Si, se vuoi chiamarli così, sono stati. Ci sono un sacco di variabili che non servono a niente e si possono completamente eliminare: statorele, statovalv, accesa, statostart, tspegnimento, statopausa, statopausa1, ta

Per misurare il tempo basta la variabile t
E per il calcolo del trascorso: millis()-t

Le condizioni da verificare in ogni stato sono molto più semplici, per questo è importante prima di ogni altra cosa disegnarsi il diagramma degli stati e le condizioni di transizione. Riferendomi al diagramma del post #5, e con la speranza che questa non sia pappa pronta ma un modo per comprendere il metodo:

L'uso di millis è semplice, in uno stato calcoli il tempo trascorso da quando il valore di millis è stato salvato nello stato precedente nel momento di cambio stato. Dal diagramma si vede che solo S_START e S_LED1_ON hanno come condizioni di uscita dei timeout, quindi solo in S_OFF e S_START (che sono i rispettivi stati precedenti) andrà salvato il valore di millis in t

Gli avevo consigliato infatti di ripartire da capo per spazzare via tutta quella polvere vecchia

Non lo ha capito, pazienza

Come regola generale, devi essere ordinato e devi stare attento ai nomi che dai e ai tipi delle variabili:

int statorele = LOW;
deve essere bool:
bool stato_rele=LOW;

unsigned long statopausa;
Se è uno stato, perché è UL?...
bool stato_pausa;

statopausa = millis();
Allora non è uno stato!
uint32_t t_pausa=millis();
Io uso sempre t_nome per le variabili a 32 bit che uso per memorizzare il tempo di millis(), così le riconosco subito.

loop0();
Il loop è uno! I case dello switch devono passare in un attimo! Non sono loop. Inoltre, loop0 non spiega che cosa fa quella funzione:
apre_valvola();

Tecnicamente ogni stato dovrebbe corrispondere a una situazione in cui si attende qualcosa, per cui più che consiglio direi che è proprio da fare. L'alternativa sarebbe usare una variabile sottostato per distinguere diverse situazioni dentro uno stesso stato, che è più complicato. Avrebbe senso solo per sottostati tra loro strettamente "imparentati", come la modifica di diversi campi, ad esempio giorno mese e anno in uno stato di regolazione data.

In settimana appena ho tempo ricomincio.
Grazie soprattutto perché piano piano sto capendo gli errori

Perfetto, bravissimo :clap:, ottima spiegazione e soprattutto per il disegno, del quale ho accennato nel post #4 ma per pigrizia ho preferito descrivere la cosa come passi "descritti" più che "disegnati":wink: .
Tra l'altro confermo, è quello che in genere si fa, anche solo a penna su un foglio, quando si affrontano situazioni come questa (che comunque nel mondo dei microcontrollori è frequentissima...).

grazie ancora a tutti per i consigli (sto imparando moltissimo);
ho rifatto tutto un'altra volta e diciamo che ci sono quasi..
mi perdo con i millis(), non riesco a capire dove sbaglio

#define rele 12
#define valv 11
#define start 9

unsigned long t = 0;


bool statorele = LOW;

int stato = 0;

enum { INIZIO=0, ATTESA_TASTO, APRO_VALVOLA, TEMPO_CONTROLLO_RELE, ATTIVO_RELE, SOLO_RELE, SPEGNI_TUTTO, ATTESA_SPEGNIMENTO, STOP};


void setup() {
  pinMode(9, INPUT);
  pinMode(rele, OUTPUT); 
  pinMode (valv, OUTPUT); 
  
  
}

void loop() {
  
  switch (stato) {
    
    case INIZIO:                                                            // quando premo il pulsante verifico tramite lo stato rele in che case sono
    if ((digitalRead (9) == HIGH) && (statorele == LOW)){
        stato = ATTESA_TASTO;
    }
    break; 

    
   case ATTESA_TASTO:                                                          // aspetto 500 ms per evitare falsi contatti se no ricomincio da capo
   if (millis()-t >=500){
   stato = APRO_VALVOLA; 
   
   }
   else {
    stato = INIZIO;
    }   
    break; 

    
    case APRO_VALVOLA:                                                         // apro l'elettrovalvola
    if (digitalRead(9) == HIGH){
    digitalWrite (valv,HIGH);

    stato = TEMPO_CONTROLLO_RELE;
    }
    else {
      stato = INIZIO;
    }
    
    break; 


    
    case TEMPO_CONTROLLO_RELE:                                          // aspetto per l'attivazione del rele se no ricomincio da capo
      if(millis()-t >=2000){
     stato = ATTIVO_RELE;
     }
     else {
      stato = INIZIO;
      }
    break;

    
    case ATTIVO_RELE:                                                      // attivo il rele e cambia stato 
      digitalWrite (rele,HIGH);
      statorele = HIGH;
      stato = SOLO_RELE;
    break;

    case SOLO_RELE:                                                     // spengo la valvola e lascio attivo il rele quando mollo il tasto
    if(digitalRead (9) == LOW){
      digitalWrite (rele, HIGH);
      digitalWrite (valv, LOW);
      stato = SPEGNI_TUTTO;
      }
      break;


    case SPEGNI_TUTTO:                                                     // controllo il tasto per spegnere
    if (digitalRead (9) == HIGH){                                    
      stato = ATTESA_SPEGNIMENTO;
      t = millis();
      }
     break;

    case ATTESA_SPEGNIMENTO:                                               // aspetto 350ms
    if (millis()-t >= 350){
      stato = STOP;
      }
   else {
    stato = SPEGNI_TUTTO;
    }
   break;

   case STOP:
   digitalWrite (rele, LOW);                                          // spengo tutto
   statorele = LOW;
   stato = INIZIO;
   break;
   
   
  }
}
grazie ancora per l'aiuto e soprattutto per la pazienza nei miei confronti

È più facile ripartire da zero. Inoltre questa richiesta è molto più importante di quanto ti possa sembrare...

...il codice non correttamente incolonnato (indentato) per gli altri è illeggibile, equi valea scri veresen zaris pett areg lisp azi.


Iniziamo: definiamo i pin da usare tramite nomi di comodo, maiuscoli visto che sono costanti, e da usare in tutto il resto del programma al posto dei numeri corrispondenti (altrimenti è del tutto inutile definire dei nomi di comodo).
#define RELE 12
#define VALV 11
#define START 9

void setup() {
  pinMode(START, INPUT);
  pinMode(RELE, OUTPUT);
  pinMode(VALV, OUTPUT);
}

Definiamo un certo insieme di nomi di comodo per identificare gli stati, e alla variabile di stato assegniamo quello che vogliamo sia l'iniziale.

enum { 
    INIZIO,
    ATTESA_STABILITA
};

int stato = INIZIO;

Dobbiamo anche tenere conto del trascorrere del tempo tramite una variabile temporale 't'.

unsigned long t = 0;

Se millis crea confusione, basta un pò di syntactic sugar, due funzioni di comodo che ci aiutano a non pensare a millis.

void start_tempo() { t = millis(); }

bool trascorso(unsigned long dt) { return millis() - t  >=  dt; }

Si chiama 'start_tempo' nel momento da cui si vuole iniziare a misurare il tempo, e si chiama 'trascorso' per sapere se è trascorso oppure no.

Anche per la lettura dell'ingresso (che se è un contatto diamo per scontato che sia già filtrato in hardware con un condensatore contro i rimbalzi) ci creiamo una funzione di comodo, in modo che il resto del programma sia molto più leggibile e comprensibile:

bool premuto() { return digitalRead(START) == HIGH; }

E arriviamo all'elaborazione, un ciclo (il loop) con un selettore di stato attivo (switch):

void loop() {
    switch(stato)
    {
        case INIZIO:
        break;    
    
        case ATTESA_STABILITA:
        break;    
        
        ecc ecc ecc
    }
}

Ricordando che in una macchina a stati scritta correttamente ogni stato rappresenta l'attesa di qualcosa (anche più di una)... cosa fa lo stato iniziale? Attende la pressione del pulsante. Basta, non gli interessa altro. E cosa deve succedere quando si preme? Che si deve passare a una fase di attesa stabilità della durata di 500 ms dal momento della pressione, per cui, più schematicamente:

INIZIO:
    SE premuto:
        start tempo
        passa a ATTESA_STABILITA

Tradotto:

case INIZIO:
    if (premuto()) {
        start_tempo();
        stato = ATTESA_STABILITA;
    }
break;

E avanti, in ATTESA_STABILITA cosa attendiamo? O il rilascio del pulsante, e si torna a INIZIO, o il timeout di 500 ms, per cui si attiva la valvola, si restarta il tempo per la fase seguente e si passa alla fase seguente. Anche qui più schematicamente:

ATTESA_STABILITA:
    SE non premuto:
        stato = INIZIO
    ALTRIMENTI SE trascorsi 500 ms:
        apro valvola
        start tempo
        passa a ....

Tradotto:

case ATTESA_STABILITA:
    if (!premuto()) {
        stato = INIZIO
    }else if (trascorso(500)){
        apri_valvola();
        start_tempo();
        stato = ...
    }
break;

E abbiamo fatto i primi due cerchi del diagramma del post #13.

Avanti con gli altri :wink:

2 Likes