Problem with timer... it makes mistakes all the time. code wrong?

Hi again,
I am experiencing some problems with my code. I am using dtmf to turn on a led for some time. But most of the times the times doesn’t work fine. After I press “XX#” on my phone the timer resets whenever it wants sometimes, and resets several times for no reason.
Can you see a coding problem or its a timer/memory problem?
the time for the led is in seconds…

I think there is something wrong with the declaration of " unsigned long watercurrentMillis = millis();".

const int waterPin =  13;      // the number of the LED pin
int waterState = LOW;             // waterState used to set the LED
long waterpreviousMillis = 0;        // will store last time LED was updated
long waterinterval = 0;

#define IRQ_PIN  2   //SID / IRQ Pin of MT8870 (info for new data)
#define D0_PIN   3   // dato bit 0
#define D1_PIN   4   // dato bit 1
#define D2_PIN   5   // dato bit 2
#define D3_PIN   6   // dato bit 3

int timerON = 0;
int m_i =0;
int index = -1; // Index into array; where to store the character
int mypinakas[5];
int dtmf_digit;
byte irq_state;
int m_x=0;
int m_y =0;
int dtmfdelay;
byte dato;           // dtmf received as byte number
 char key;            // byte converted to char
 int jmi=0;
 
 void setup()
{
 
   pinMode(waterPin, OUTPUT); 
  
Serial.begin (4800);
  Serial.println("Starting test...");
 
 // Chech the pins of the DTMF Decoder
 pinMode(IRQ_PIN, INPUT);
 pinMode(D0_PIN, INPUT);
 pinMode(D1_PIN, INPUT);
 pinMode(D2_PIN, INPUT);
 pinMode(D3_PIN, INPUT);
   
}

void loop()
{
  
  Serial.print("timerON: ");
  Serial.print(timerON);
    Serial.print("key: ");
  Serial.print(key);
  Serial.print("===== ");

  dtmf_read();
digitalWrite(waterPin, waterState);

 unsigned  long watercurrentMillis = millis();
 if (jmi==1) { waterpreviousMillis = watercurrentMillis;  }
  if (timerON==1) {
   waterState = HIGH;
 
 jmi =0;
 

 
  if(watercurrentMillis - waterpreviousMillis > waterinterval) {
    // save the last time you blinked the LED 
   waterpreviousMillis = watercurrentMillis;
   timerON = 0;
   
     waterState = LOW;
    }
  
 } // if timer on
 
  Serial.println(watercurrentMillis - waterpreviousMillis);
  
  
}

/*-------------------------------------------------------------*/

void dtmf_read() 
{
  
  
  
  
 
 
   
  
 //Check the state of the pin for new DTMF Tone
 irq_state = digitalRead(IRQ_PIN);
 
 if ( irq_state == 1 )  //if there is new tone
 
    {    
    
  char  key_previous = key;  // save the key to previous so we can compare it
  Serial.print("Key : ");
  char key = read_codigo(); // Read the DTMF byte
  Serial.println(key); 
    delay(100);       
 
  if((key!='#')||(key!='*')) { m_i++; index++; } 

  if ((key_previous==key)&&(key!='#')) { index=index-1; m_i=m_i-1; } 

  if ((key_previous==key)&&(key=='#')) {
     delay(1000); 
     memset(mypinakas, 0, sizeof(mypinakas)); 
     index=-1;
     m_i=0;  
   }  // What to do after duplication of "#"

  if(key=='*') {
   Serial.println("DTMF Reset! Waiting for new input..."); memset(mypinakas, 0, sizeof(mypinakas));  index=-1; m_i=0;}
 
  if((key=='#')&&(key_previous!=key)) { 
   

m_x=mypinakas[0];
  m_y=mypinakas[1];
       if (m_i==2) { dtmfdelay = m_x; 
         }
       else  { dtmfdelay = m_x*10+m_y;  m_i=0; 
       
         }  
  Serial.print("Thermosifwnas ON for: ");
  Serial.print(dtmfdelay);
  Serial.println(" minutes.");
  Serial.print(mypinakas[0]); Serial.print(mypinakas[1]);Serial.print(mypinakas[2]);Serial.println(mypinakas[3]);
  memset(mypinakas, 0, sizeof(mypinakas)); 
  index=-1;
  m_i=0;
 waterinterval = dtmfdelay*1000; 
 jmi=1;
   
  timerON = 1;
    
  }
      
      
  if(index < 4) // One less than the size of the array
  { mypinakas[index] = dtmf_digit; } // Store it
	
	     
      
     
    } //end of IRQ_state
        
 } // End of Loop

/*********************************/
byte read_codigo()
{
 


 byte D0,D1,D2,D3;    // Variable Donde Se Lee el Estado
 
 //Check the pin state of MT8870 (1 or 0 / On or OFF)
 D0 = digitalRead(D0_PIN);
 D1 = digitalRead(D1_PIN);
 D2 = digitalRead(D2_PIN);
 D3 = digitalRead(D3_PIN);
 
 //Set a byte for dtmf tone
 // by merging the pins to a "dato" byte
 bitWrite(dato,0,D0);
 bitWrite(dato,1,D1);
 bitWrite(dato,2,D2);
 bitWrite(dato,3,D3);

// Convert the dato to char so we can manipulate it
 if ( dato == 1  ) key = '1';
 if ( dato == 2  ) key = '2';
 if ( dato == 3  ) key = '3';
 if ( dato == 4  ) key = '4';
 if ( dato == 5  ) key = '5';
 if ( dato == 6  ) key = '6';
 if ( dato == 7  ) key = '7';
 if ( dato == 8  ) key = '8';
 if ( dato == 9  ) key = '9';
 
 if ( dato == 10 ) key = '0';
 if ( dato == 11 ) key = '*';
 if ( dato == 12 ) key = '#';
 if ( dato == 13 ) key = 'A';
 if ( dato == 14 ) key = 'B';
 if ( dato == 15 ) key = 'C';
 if ( dato == 0  ) key = 'D';
 
dtmf_digit =atoi (&key);

return key;

}

My guess is that the problem is with your handling of "index", but you could usefully run the auto-format (ctrl-T) tool on your code and repost it. Looking at it as it is hurts my eyes. What do your debug prints tell you?

char key;

dtmf_digit =atoi (&key); Oops.

I had the same code before but without the timer code. All worked fine. Now by debugging there are random errors with the " Serial.println(watercurrentMillis - waterpreviousMillis);" it resets at any time it wants. Sometimes the led turnes on for 30 seconds and then is turned off. But some other times the led turns on and never goes to LOW state. Some times it also turnes on and is turned offf before the time given. Each time i can see that the "Serial.println(watercurrentMillis - waterpreviousMillis);" is either 0 or -XXXXX. I've had the same timer working fine other times.

All worked fine.

Perhaps "fine" means something different where you are.

Code:

char key;

Code:

dtmf_digit =atoi (&key);

I have entered "&key" because the compiler showed errors and never compiled with "dtmf_digit =atoi (&key);" How can I replace it? I just need key to be converted to integer number.

because the compiler showed errors and never compiled with “dtmf_digit =atoi (&key);”

?

You need to understand what atoi operates on:
http://www.cplusplus.com/reference/clibrary/cstdlib/atoi/

(uncompiled, untested)

byte read_codigo()
{
  byte dato = 0;
  //Set a byte for dtmf tone
  // by merging the pins to a "dato" byte
  bitWrite(dato, 0, digitalRead(D0_PIN));
  bitWrite(dato, 1, digitalRead(D1_PIN));
  bitWrite(dato, 2, digitalRead(D2_PIN));
  bitWrite(dato, 3, digitalRead(D3_PIN));

  byte key = "D1234567890*#ABC" [dato];

  if (key >= '0' && key <= '9') {
    dtmf_digit = key - '0';
  } else {
    dtmf_digit = -1;
  }
  return key;
}

the problem is still there.

All i can see is that waterinterval works fine until the number 32. If the number is 34 and above something changes. When i give the number 12, the waterinterval is changed to 12000, for 13 -> 13000, 14-> 14000. But when i give the number 34 it stores to waterinterval the number -31536, 35 -> -30536 & for number 68 the waterinterval is 2464. Why is this happening? Buffer memory problem? The problem starts from number 34

You need to post your code. We're good, but we're not psychic.

AWOL:
You need to post your code. We’re good, but we’re not psychic.

Hahahaha! you’re right!

/*

*/
const int waterPin =  13;      // the number of the LED pin
int waterState = LOW;             // waterState used to set the LED
long waterpreviousMillis = 0;        // will store last time LED was updated
long waterinterval = 0;

#define IRQ_PIN  2   //SID / IRQ Pin of MT8870 (info for new data)
#define D0_PIN   3   // dato bit 0
#define D1_PIN   4   // dato bit 1
#define D2_PIN   5   // dato bit 2
#define D3_PIN   6   // dato bit 3

int timerON = 0;
int m_i =0;
int index = -1; // Index into array; where to store the character
int mypinakas[5];
int dtmf_digit;
byte irq_state;
int m_x=0;
int m_y =0;
int dtmfdelay;

byte dato=0;           // dtmf received as byte number
  byte key = "D1234567890*#ABC" [dato];
   
 int jmi=0;
 
 void setup()
{
 
   pinMode(waterPin, OUTPUT); 
  
Serial.begin (4800);
  Serial.println("Starting test...");
 
 // Chech the pins of the DTMF Decoder
 pinMode(IRQ_PIN, INPUT);
 pinMode(D0_PIN, INPUT);
 pinMode(D1_PIN, INPUT);
 pinMode(D2_PIN, INPUT);
 pinMode(D3_PIN, INPUT);
   
}

void loop()
{
  
  Serial.print("timerON: ");
  Serial.print(timerON);
    Serial.print("key: ");
  Serial.print(key);
  Serial.print("===== ");

  dtmf_read();
digitalWrite(waterPin, waterState);

 unsigned  long watercurrentMillis = millis();
 if (jmi==1) { waterpreviousMillis = watercurrentMillis;  }
  if (timerON==1) {
   waterState = HIGH;
 
 jmi =0;
 

 
  if(watercurrentMillis - waterpreviousMillis > waterinterval) {
    // save the last time you blinked the LED 
   waterpreviousMillis = watercurrentMillis;
   timerON = 0;
   
     waterState = LOW;
    }
  
 } // if timer on
 
  Serial.print(watercurrentMillis - waterpreviousMillis);
  Serial.print(" ");
  Serial.println(waterinterval);
  
  
}

/*-------------------------------------------------------------*/

void dtmf_read() 
{
  
  
  
  
 
  
 //Check the state of the pin for new DTMF Tone
 irq_state = digitalRead(IRQ_PIN);
 
 if ( irq_state == 1 )  //if there is new tone
 
    {    
    
  char  key_previous = key;  // save the key to previous so we can compare it
  Serial.print("Key : ");
  char key = read_codigo(); // Read the DTMF byte
  Serial.println(key); 
    delay(100);       
 
  if((key!='#')||(key!='*')) { m_i++; index++; } 

  if ((key_previous==key)&&(key!='#')) { index=index-1; m_i=m_i-1; } 

  if ((key_previous==key)&&(key=='#')) {
     delay(1000); 
     memset(mypinakas, 0, sizeof(mypinakas)); 
     index=-1;
     m_i=0;  
   }  // What to do after duplication of "#"

  if(key=='*') {
   Serial.println("DTMF Reset! Waiting for new input..."); memset(mypinakas, 0, sizeof(mypinakas));  index=-1; m_i=0;}
 
  if((key=='#')&&(key_previous!=key)) { 
   

m_x=mypinakas[0];
  m_y=mypinakas[1];
       if (m_i==2) { dtmfdelay = m_x; 
         }
       else  { dtmfdelay = m_x*10+m_y;  m_i=0; 
       
       
         }  
  Serial.print("Thermosifwnas ON for: ");
  Serial.print(dtmfdelay);
  Serial.println(" minutes.");
  Serial.print(mypinakas[0]); Serial.print(mypinakas[1]);Serial.print(mypinakas[2]);Serial.println(mypinakas[3]);
  memset(mypinakas, 0, sizeof(mypinakas)); 
  index=-1;
  m_i=0;
  waterinterval =dtmfdelay*1000;
 jmi=1;
   
  timerON = 1;
    
  }
      
      
  if(index < 4) // One less than the size of the array
  { mypinakas[index] = dtmf_digit; } // Store it
	
	     
      
     
    } //end of IRQ_state
        
 } // End of Loop

/*********************************/
byte read_codigo()
{
 


 byte D0,D1,D2,D3;    // Variable Donde Se Lee el Estado
 
 //Check the pin state of MT8870 (1 or 0 / On or OFF)
 D0 = digitalRead(D0_PIN);
 D1 = digitalRead(D1_PIN);
 D2 = digitalRead(D2_PIN);
 D3 = digitalRead(D3_PIN);
 
 //Set a byte for dtmf tone
 // by merging the pins to a "dato" byte
 bitWrite(dato,0,D0);
 bitWrite(dato,1,D1);
 bitWrite(dato,2,D2);
 bitWrite(dato,3,D3);

// Convert the dato to char so we can manipulate it
 if ( dato == 1  ) key = '1';
 if ( dato == 2  ) key = '2';
 if ( dato == 3  ) key = '3';
 if ( dato == 4  ) key = '4';
 if ( dato == 5  ) key = '5';
 if ( dato == 6  ) key = '6';
 if ( dato == 7  ) key = '7';
 if ( dato == 8  ) key = '8';
 if ( dato == 9  ) key = '9';
 
 if ( dato == 10 ) key = '0';
 if ( dato == 11 ) key = '*';
 if ( dato == 12 ) key = '#';
 if ( dato == 13 ) key = 'A';
 if ( dato == 14 ) key = 'B';
 if ( dato == 15 ) key = 'C';
 if ( dato == 0  ) key = 'D';
 
  if (key >= '0' && key <= '9') {
    dtmf_digit = key - '0';
  } else {
    dtmf_digit = 0;
  }

return key;

}

And what does 33 * 1000 equal? (Hint: Think carefully)

Can I also suggest you familiarise yourself with auto format tool in the IDE?

Auto Format This formats your code nicely: i.e. indents it so that opening and closing curly braces line up, and that the statements instead curly braces are indented more.

Its the first time I notice it and its great!

About number 33 I should have declared it as long!

Integers are your primary datatype for number storage, and store a 2 byte value. This yields a range of -32,768 to 32,767 (minimum value of -2^15 and a maximum value of (2^15) - 1).

I cannot find out why the 34 is getting -31586

Because they're both 16 bit ints.

Its the first time I notice it and its great!

I mentioned it in reply #1

Thank you! By making these variables "long" all worked fine. I suppose I will look more into it next time...

long m_x=0; long m_y =0; long dtmfdelay;