Proper to change delay to millis, program flow

EXTRACT;
Currently working program uses delays functions, I set up Watchdow timmer, and is trigger by the delays, I read Nick Gammon and make a couple of examples in using millis, to avoid delay, however Im not quite sure how to proceed in this particular problem.

The problem;
I will post a small snip of the code… and latter on, the entire code.
All the functions and switches are working properly, just I need to adapt the code to a non blocking type

/*Pseudocode just to simplify the problem*/

void loop(){
        wdt_reset ();
        read.SW1
        read.SW2
        read.SW3
        printscreen(); 

if( SW == defrost)
{
  defrostON
}

defrostON(){
 
for(byte a = 0; a< 4; a++){ ← Actual function blocking code  
digitalWrite(rele[a], HIGH); 
delay(1000);

Know… Im really new to avoid delays, I make a coupe of examples using Nick, blink leds without delay.

In order to proceed in this particular problem, can I do something like; ?

void defrostON()
{
byte past_delay_seconds = 0; // at global scope 
byte delay_seconds      = 0;   //global
byte a                  = 0;          //global

rele_timer = millis();

if(rele_timer - millis(); >= rele_delay)
{
   delay_seconds++;
   a++;
}

if(past_delay_seconds != delay_seconds)
{
   digitalWrite(rele[a], HIGH);
}

Im trying to do like this because, I need to read the main switch witch is manually opperated, and I like to have a second between each rele to start… So will be; Rele 1- ON - delay 1000 - rele 2-ON etc.

Here is the entire code… Warning… is a litte bit messy. Well actually a lot messy , kumensai work in progress.

//===============================LIBRERIAS===============================//

#include <avr/wdt.h>          // watch dog library 
#include "Arduino.h"          // call need for current arduino ide 
#include <Switch.h>           //to read switchs
#include <MemoryFree.h>       // to test memory usage 


//=============================HARDWARE PINS=============================//
const byte sol = 13;                 //pin para el control de la solenoide 
const byte comp =12;                 //Pin para control del compresor 
const byte motores[4] = {8,9,10,11}; //Pines controlar motores del difusor  
const byte focoRef= 5 ;              //Pin para encender el foco de la refrigeracion 
const byte focoDes= 6 ;              //Pin para encender el foco deshielo 
const byte term = 4;                 //Pin para leer el estado del termostato 
const byte SW2 = 3;                  //pin para leer el switch principal
const byte SW1 = 2;                  //pin para leer el switch principal

//===========================GLOBAL VARIABLES===========================//

byte sw =     0;              //variable para almacenar el estado del switch 
byte caso =   1;              //variable de 1 a 4 para alternar los motores difussor

boolean encendido = false;    //Variable para comparar si se ha encendio o apagado el termostato

//            fila / columna                 
byte ArrayMotores[6][4] ={{HIGH, HIGH, HIGH,  HIGH  }, //Primer arreglo de encendio de motores
                         {LOW,  HIGH, HIGH,  HIGH  } , // Segundo... cada 1 y 0 corresponden a un motor
                         {HIGH, LOW , HIGH,  HIGH  } , //encendido o apagado. 
                         {HIGH, HIGH, LOW ,  HIGH  } , 
                         {HIGH, HIGH, HIGH,   LOW }  ,
                         {LOW,  LOW,  LOW,   LOW }   ,  
                          };


unsigned long  global_timer = 0;  //var to control delays 

unsigned long  one_sec_delay = 1000; 
unsigned long  defrost_time = 0; 

unsigned long current_time = 0; 
unsigned long end_loop_time = 0; 
unsigned long loop_time = 0; 

//==============================CONSTRUCTORS==============================//
Switch EdoSW1 = Switch (SW1, INPUT, HIGH); 
Switch EdoSW2 = Switch (SW2, INPUT, HIGH); 
Switch EdoTerm = Switch(term, INPUT, HIGH); 

//===============================VOID SETUP===============================//

void setup() {
  delay(500);                      //Delay para prevenir ruido al arranque del arduino
  Serial.begin(9600);
  pinMode(comp, OUTPUT); 
  pinMode(sol, OUTPUT); 
  pinMode(focoRef, OUTPUT); 
  pinMode(focoDes, OUTPUT); 
  delay(250); 
  for(int b; b<4; b++){               //inicia contador de 1 a 4
    pinMode(motores[b], OUTPUT);      //declara cada motor como pin de salida 
    delay(500);
                                      //digitalWrite(motores[b], HIGH); //Apaga cada motor 
   }
/* for(int z; z<5; z++){
   pinMode(pinesNoUsados[z], OUTPUT); //declara cada pin sin usar como salida 
   pinMode(pinesNoUsados[z], HIGH);   //pone cada pin no usado a 5V
   }
 */
  delay(500); 
  wdt_enable (WDTO_4S);  // reset after one second, if no "pat the dog" received
  Serial.println("==================Start program==================="); 
}





void loop() {
  
  unsigned long start_loop_time = millis(); //para debug 
  global_timer = millis();


  wdt_reset ();  // give me another second to do stuff (pat the dog)
  
  EdoSW1.poll(); 
  EdoSW2.poll(); 
  EdoTerm.poll(); 

  Serial.print("freeMemory()="); Serial.println(freeMemory());
    //Serial.println("Inicia Lectura de SW"); 
    
    //Serial.print("SW1 = ");   //Serial.println(EdoSW1); 
    //Serial.print("SW2 = ");   //Serial.println(EdoSW2); 


         
  if(EdoSW1.on() && !EdoSW2.on()){
        sw=1; 
     }
     else if ( !EdoSW1.on() && EdoSW2.on()){
        sw=2;
      } 
     else if (!EdoSW1.on() && !EdoSW2.on()){
        sw =0; 
        }
  
//  Serial.println("Termina lectura de SW"); 
  //delay(100); 

  switch(sw){
  case 0:
    Serial.println("SW apagado"); 
    //delay(100); //remover en la verson final 
    digitalWrite(focoRef, HIGH); 
    digitalWrite(focoDes, HIGH); 
    AllOff();
    break;

  case 1:
    Serial.println("SW en refrigeracion"); 
    //delay(100); //remover en la version final 
    digitalWrite(focoRef, HIGH); 
    digitalWrite(focoDes, LOW); 
    refrigeracion();
    break; 
    
  case 2:
    Serial.println("SW en deshielo");   
    //delay(100);// remover en la version final 
    digitalWrite(focoRef, LOW); 
    digitalWrite(focoDes, HIGH); 
    deshielo();  
    break;
  }
  //delay(150);
  Serial.flush();

  end_loop_time = millis();
  loop_time = end_loop_time -start_loop_time;

  Serial.print("Loop Time"); Serial.println(loop_time); 
}

// subrutina de deshielo, deja encendido solo los motores del evaporador. 
void deshielo(){ 
  defrost_time = millis(); 
  digitalWrite(sol,LOW);  //enciende la solenoide 

  if(defrost_time - global_timer => one_sec_delay  ){ //remover en version final); 
    digitalWrite(comp, HIGH);} //apaga el compresor
    

  if(defrost_time - global_timer => one_sec_delay*2 ){
    for(int a; a<4 ; a++){
    digitalWrite(motores[a], ArrayMotores[5][a]); 
  
    delay(1000); 
  }
}


// subrutina para apagar todo; compresor, solenoide, evaporador. 
void AllOff(){
  digitalWrite(sol,HIGH);  //Apaga  la solenoide 
  //delay(500);           //remover en version final); 
  digitalWrite(comp, HIGH); //apaga el compresor
  //delay(200); //remover en version final
  for(int u; u<4; u++){
    digitalWrite(motores[u], ArrayMotores[0][u]);
  }  
}

//subrutina que encendera la refrigeracion y alternara los motores del 
//difusor.

void refrigeracion(){
          
            if(caso >= 5){caso=1;}       
  
        if(EdoTerm.on()){
           digitalWrite(sol, HIGH);   //Apaga la solenoide, deja pasar liquido
           for(byte d=0; d<4; d++){
              digitalWrite(motores[d], ArrayMotores[5][d]);
              delay(1000); 
              }
              delay(1000);               //retraso en la entrada del compresor
              digitalWrite(comp, LOW);  //enciende el compresor
              Serial.print("Termostato encendido  "); Serial.println(caso);
              encendido = true; 
              }
           
      if(!EdoTerm.on()){
             if(encendido == true){
             caso++;}
             Serial.print("Termostato apagado ");
             Serial.print("Comienza subrutina "); Serial.println(caso);
             digitalWrite(sol, LOW);  //Enciende solenoide, no pasa liquido
             delay(1000);              //retraso en la entrada del compresor
             digitalWrite(comp, HIGH);  //Apaga el compresor
             for(byte e=0; e<4; e++){
             digitalWrite(motores[e], ArrayMotores[caso][e]);
            }      
            encendido = false; 
            }
}

Thanks in advance for any help.

-Alex.

Read this : http://forum.arduino.cc/index.php?topic=223286.0 That might perhaps be a little overwhelming.

There is a specific way to use millis() to prevent the rollover problem. The previous millis() is remembered and the if-statement is like this:

if ( millis() - prevMillis >= interval )

or like this:

currentMillis = millis();
if ( currentMillis - prevMillis >= interval )

Can you tell what you want to do ? The goal is to let the loop() run as fast as possible, and only at some intervals or after some delays do a task. Assume that the loop() runs indefinitely fast, and the normal tasks should be done 100 times a second or once in an hour. How many times a second should something be written to the serial monitor ? How many times a second should the switches be checked ?

Why do you use a Serial.flush() ?

void defrostON()
{
byte past_delay_seconds = 0; // at global scope 
byte delay_seconds      = 0;   //global
byte a                  = 0;          //global

They are not at global scope, so your comments are really confusing.

  wdt_enable (WDTO_4S);  // reset after one second, if no "pat the dog" received

If the comments don't agree with the code, fix them or omit them.

if(rele_timer - millis(); >= rele_delay)

No, you can't do that.

Hi peter, hi Nick thanks for the reply.

What I want to do, is to read the main switch in the loop, depending of its position will be 3 subrutines to launch, one of them is defrost.

Currently defrost start one of four relays in a one second apart manner... I know this might be pretty easy just dont see at the time.

So... I was using delay... And a for statmen to accomplish the function.

I understand the previous millis- millis >= delay. Can be called sof delay?.

But... How I make each relay one second apart from each other? Let's say I write something like;

If (switch == defrost){ CurrentMillis = mill is() If(pastMillis-currentMillis => 1000){ DigitalWrite(evap[1] , HIGH)

But.... I will need another 3unsigned long Variables, each for one releay?

Or can be done like a second counter ?

If (switch ==defrost){ CurrentMillis = millis() If(pastMillis-CurrentMillis => 1000) { SecCounter ++ Relaycounter ++ }

If(secCounter != pastSecCounter) \ means we have elapsed a second

{ DigitalWrite(rele[relay counter], HIGH) }

I'm on the right track?

Sorry for the commets Nick latter on I will make a more clear and way less hard to read example.

Thanks. Alex.

AlexLPD:

If (switch ==defrost){

CurrentMillis = millis()
If(pastMillis-CurrentMillis => 1000)
{
SecCounter ++
Relaycounter ++
}

If(secCounter != pastSecCounter) \ means we have elapsed a second

{
DigitalWrite(rele[relay counter], HIGH)
}

Yes, you’re on the right track.

A single counter is enough, no? Why not:

if (switch == defrost) {
 if (RelayCounter < sizeof(rele)  && millis()-pastMillis >= 1000) {
    pastMillis = millis();
    DigitalWrite(rele[RelayCounter], HIGH)
    Relaycounter++;
  }
} else {
  // reset some stuff since we've released the defrost switch
  Relaycounter = 0;
  pastMillis = millis();
}

Wow thaths amazibg And simple as well thanks so much for the clarification .... I will make a couple of test and comeback with results.

Thanks again!!!

-Alex.

Hi and thanks @arduinodlb… I think this line will never executes…

if (RelayCounter < sizeof(rele) && millis()-pastMillis >= 1000) {

because the timmer is in the if statment and never will be true…

In the Nick example… there was a timmer setting at the setup;

void setup() {

pinMode(led1, OUTPUT);
pinMode(led2, OUTPUT);
pinMode(led3, OUTPUT);
pinMode(led4, OUTPUT);
pinMode(led5, OUTPUT);
pinMode(led6, OUTPUT);
pinMode(led7, OUTPUT);

pinMode(ldr , INPUT );

led1timer = millis();
led2timer = millis();
led3timer = millis();
led4timer = millis();
led5timer = millis();
led6timer = millis();
led7timer = millis();

Know… this is odd… I try to use the same principle like this;

defrost = digitalRead(DefrosSw);

if (defrost == HIGH)
{
   evap_timer = millis();
   
   if  (evapM < sizeof(evap_motors) && 
   millis() - evap_timer >= evap_delay )
   {
   evap_timer = millis(); 
   digitalWrite(evap_motors[evapM] , HIGH);
   evapM++;
   }
}

And of course… never executes… because the beggin time is always updated… is there a proper way to
handle this?
A boolean flag ?

So when the switch is toggle in the defrost mode… the start time of this fuction start at the current millis… and then… dont will be updated untill the defrost has been off and the timmer re-set.

Any tougths ?

-Alex.

Thanks !!!

AlexLPD:
Hi and thanks @arduinodlb… I think this line will never executes…

if (RelayCounter < sizeof(rele) && millis()-pastMillis >= 1000) {

because the timmer is in the if statment and never will be true…

Not true. It makes no difference.

millis() is a function. When you call it, it simply returns the current number of milliseconds elapsed since the Arduino was last re-started. It can be called essentially at any time, including in an if statement. There is nothing special about an if, other than it executes the next block of code or skips it, depending on the result of the if test. But, the if test can contain any valid code that ultimately results in a boolean test, including calls to existing functions.

So yes, this code will execute 1 second after the defrost button is pushed and held down. Actually, the “if” code will execute every single time you go through the loop, but the test will only pass 1 second after you hold down the defrost button.

AlexLPD: ``` defrost = digitalRead(DefrosSw);

if (defrost == HIGH) {   evap_timer = millis();  
  if  (millis() - evap_timer >= evap_delay )   {  
  } }

Yeah. That won't work.

Use my code. Your code won't work because You are effectively doing this:

    if (millis() - millis() > 1000) {
    }

and millis() - millis() is always going to be 0, or very, very close to 0.

Yes I see my circular reference but.....

I upload the code and won't work... Pherhaps I made a mistake.....

I will look in to it....

Thanks.

-Alex.

Well you are rigth as usual… I ended up with someting like this;

// soft delay para relevadores de tablero ejemplo.ino

//===================================LYBRARIES===================================//
#include "Arduino.h"          // call need for current arduino ide 
#include <Switch.h>
//#include <avr/wdt.h>

//===============================GLOBAL VARIABLES===============================//

const byte  evap_motors[4] = {4,5,6,7}; //relays pins  
byte const         DefrostSWpin = 2;         // defrost swithc pin 

const unsigned long evap_delay= 2000;      //delay between evap on fans 
unsigned long        evap_timer = 0;         //evap timmer to keep the time trak. 

byte                      evapM = 0; //counter for evap fans 

//===================================CONSTRUCTORS===================================//
Switch defrostSW = Switch (DefrostSWpin, INPUT, HIGH); 



//====================================VOID SETUP====================================//
void setup() 
{
Serial.begin(9600); 
Serial.println("Program starts"); 

for(byte d = 0; d<4; d++)
   {
    pinMode(evap_motors[d], OUTPUT);  
   }

evap_timer = millis();

   //wdt_enable(WDT0_4S); //set a watchdog to 4 seconds 
}


//====================================VOID LOOP=====================================//
void loop()
{
 //wdt_reset(); //reset the watchdog 
//if (sw == defrost)

   defrostSW.poll();

if (defrostSW.on())
   {
    defrostON(); 
    Serial.println("Encendido");
   }

if( !defrostSW.on() )
{
   Serial.println("En el else");
   evapM = 0; 
   evap_timer = millis();
   for(byte c = 0; c < 4; c++)
   {
     digitalWrite(evap_motors[c], LOW); 
   }
}

}


void defrostON()
{
   Serial.println("En el if");
   
   if  (evapM < sizeof(evap_motors) && 
   millis() - evap_timer >= evap_delay )
   {
   Serial.println("Cumple la funcion logica");   
   evap_timer = millis(); 
   digitalWrite(evap_motors[evapM] , HIGH);
   evapM++;
   
   }

}

It has a proper function and works well …

Know… I going to put on the original program to avoid delay on the relay start and stop…
Im using a tree way switch… feed by 5v and return to two digital pins…

Currently using Switch library… This is how it looks the switch reading…

//=============================HARDWARE PINS=============================//
const byte SW2 = 3;                  //pin para leer el switch principal
const byte SW1 = 2;                  //pin para leer el switch principal

//==============================CONSTRUCTORS==============================//
Switch EdoSW1 = Switch (SW1, INPUT, HIGH); 
Switch EdoSW2 = Switch (SW2, INPUT, HIGH); 

void loop() {
  
  EdoSW1.poll(); 
  EdoSW2.poll(); 
  EdoTerm.poll(); 

        
  if(EdoSW1.on() && !EdoSW2.on()){
        sw=1; 
     }
     else if ( !EdoSW1.on() && EdoSW2.on()){
        sw=2;
      } 
     else if (!EdoSW1.on() && !EdoSW2.on()){
        sw =0; 
        }

  switch(sw){
  case 0:
    Serial.println("SW apagado"); 
    //delay(100); //remover en la verson final 
    digitalWrite(focoRef, HIGH); 
    digitalWrite(focoDes, HIGH); 
    AllOff();
    break;

  case 1:
    Serial.println("SW en refrigeracion"); 
    //delay(100); //remover en la version final 
    digitalWrite(focoRef, HIGH); 
    digitalWrite(focoDes, LOW); 
    refrigeracion();
    break; 
    
  case 2:
    Serial.println("SW en deshielo");   
    //delay(100);// remover en la version final 
    digitalWrite(focoRef, LOW); 
    digitalWrite(focoDes, HIGH); 
    deshielo();  
    break;
  }

And Im wondering if there is a more “elegant” or clear way to do this?
The switch reading I mean…

Thanks in advance for the help.

-Alex.

The more elegant way is to name your switches. Any time you have variable names with numbers like SW1, SW2, SW3 then you either need an array SW[3] or you need to give them proper names like defrostLow, defrostHigh...

The same for your switch case statement. Name the different cases. For example...

#define state_off 0
#define state_deshielo 1
#define state_refrigeracion 2
byte state = state_off;
...
if(...) {
  state = state_refrigeracion;
} else ...
...
switch(state) {
  case state_refrigeracion:
    ...
  ...
}

This is starting to look like a "state machine". Search the forums here, there are some good posts about state machines. Look for the ones that tell you how to draw the state diagram. This problem is so complex that you need a picture - pseudocode isn't good enough.

Start with the 'off' state. What combination of switches will cause it to leave this state and enter the 'relay 1 on' state? Then the condition to leave that state is 'one second elapsed' and it will move to the 'relay 2 on' state. But what if the driver changes a switch while it's in 'relay 1 on' state? What state does it go to after that? You will end up with a lot of arrows (state transitions) in your diagram. Then you can program it as a very big switch() statement which looks at the current state, checks the possible reasons for changing state if necessary and then sets the appropriate outputs several thousand times per second.

Hi and thanks Morgan, for the reply... at first gaze the state machine sounds a litte bit impresive...

I look around and see this article; State machine

So, there are not as diffult as might sound,

Another thing... yesterday I saw a weird line of code, ans after seek on the line I came to see what is know as; Ternary operators.

This perhaps simplify the above code in to more short and easy to read?

if(EdoSW1.on() && !EdoSW2.on()){
        sw=1; 
     }
     else if ( !EdoSW1.on() && EdoSW2.on()){
        sw=2;
      } 
     else if (!EdoSW1.on() && !EdoSW2.on()){
        sw =0; 
        }

Or this? 

(  EdoSw1.on() && ! EdoSw2.on() ) ?  sw = 1 : sw = 0
(! EdoSw1.on() &&   EdoSw2.on() ) ?  sw = 2 : sw = 0
(! EdoSw1.on() && ! EdoSw2.on() ) ?  sw = 0 : sw = 0

Or this? 

    if      ( EdoSW1.on()  && !EdoSW2.on())       sw=1; 
  else if ( !EdoSW1.on() &&  EdoSW2.on())       sw=2;
  else if ( !EdoSW1.on() && !EdoSW2.on())       sw=0;

This will work?

About a state machine... The basics says we need a variable to hold the program on. I think this might be the above byte sw...

Is the switch-case the best way to do this?

For example, Im using a Switch to choose between 3 main functions; 1.- Defrost 2.- Refrigeration 3.- AllOff

At contrary of the example, the timmers of each Main functions, only work once the switch has been move to the function position. So I have to read the switch each time on the main loop to make sure any change will be register... Example;

If the switch is moved to the Defrost position, each of the fans will be on in a second apart manner. But it the switch is moved to refrigeration the program first need to chek for the temperature and after take a decision if turn on refrigeration... the fans must be on, in a one second delay manner, one second after the solenoid and two second after the compressor.

Thanks for the replys... I start to make the changes to the code to see what else can be improved.

-Alex.

Yes, ternary will work.

You have to be a little careful. Code should be easy to read, so sometimes it is better to have more obvious but more lines, rather than clever but fewer lines.

You could also do this for example (untested):

sw = EdoSw1.on() ?  (!EdoSw2.on() ?  1 : 0) : (EdoSw2.on() ?  2 : 0);

Well well a single line!

I try to test latter on...

The simplicity is a powerful message ...

-Alex

Here's another method:

sw = (EdoSw2.on() ? 2 : 0) | (EdoSw1.on() ? 1 : 0);

It wasn't clear what value you want for sw if EdoSw2.on() and EdoSw1.on() are both true. This method sets sw to 3 in that case.

Hi cristop thanks for the reply....

Y es un deed seems like feasible ... Question... Single pipe is the same as the double pipe operator or "or"?

Anybodoy knows what means ">" saw in another post... Never have seen it

Thanks. Alex

AlexLPD: Hi cristop thanks for the reply....

Y es un deed seems like feasible ... Question... Single pipe is the same as the double pipe operator or "or"?

No. Single pipe is a bitwise OR, not a logicial OR. They are different.

http://en.wikipedia.org/wiki/Bitwise_operation

Anybodoy knows what means ">" saw in another post... Never have seen it

Thanks. Alex

In the web world, it means the ">" (greater than) character