timed action - trying to add timeout/break

Hi,

I am trying to add a timeout to a statement that is an endless loop, within a timed action loop.

the setup: I wanted an old rotary telephone to ring at a set interval, say every half a hour (i set the interval at a shorter time whilst testing) and if picked up to play back a message. if the phone was picked up in the meantime i wanted a dead tone to play through the receiver.

After the phone has been ringing for 15 seconds and not been picked up i want the loop to return to the main void loop and not carry on to the other statements within the timed action loop.

i adapted blink without delay code in one of the examples then added the break statement to end the loop at this point it seems to kind of work but has some glitches.

    if (currentMillis - previousMillis2 > interval2) {
      previousMillis2 = currentMillis;  
      
      if (ringState == LOW)
      ringState = HIGH;
      else 
      ringState = LOW;
      rings ++;
    }
      
      if ((ringState == LOW) && (rings >1)) {
        Serial.print("PHONE NOT PICKED UP STOP RINGING ");
        rings = 0;
        break;
    }

after 15 seconds the phone stops ringing but,

  1. looking at the Serial.print it still runs through the next two steps in the ring loop - though very fast - less than a second, before returning to the main loop. this causes a slight glitch in the sound coming from the speaker- is it possible to get straight back to the main loop from here?
  2. if the handset is picked up whilst ringing, the message plays ok, but when the ring loop comes around the second time the ringing track is not activated and the Serial.print shows that it runs through all the stages of the ring loop before returning to the void loop again. the next time the ring loop occurs it works fine - the ringer plays as expected

this is the code within the ring loop,

void ring (){
  
 

  if (hookState==HIGH) {
    ringAction.setInterval(30*1000); //check again in 30 sec because the phone was already lifted
  } 
  else { // start ringing
    Serial.println("2 hookval LOW. start ringing: tracktwo HIGH, relay LOW - START VOID RING LOOP ");
    digitalWrite(relayswitch, LOW); //relay switch is LOW - main speaker
    while (digitalRead(switchPin)==LOW) {
     unsigned long currentMillis = millis();
 
  if(currentMillis - previousMillis > interval) {
    // save the last time you blinked the LED 
    previousMillis = currentMillis;   

    // if the LED is off turn it on and vice-versa:
    if ((ledState == LOW) && (blinks <1))
      ledState = HIGH;
    else
      ledState = LOW;
      blinks ++;

    // set the LED with the ledState of the variable:
    digitalWrite(tracktwo, ledState);
  }
  
      //TODO add a timeout to this endless loop, calls until pickup
      
    if (currentMillis - previousMillis2 > interval2) {
      previousMillis2 = currentMillis;  
      
      if (ringState == LOW)
      ringState = HIGH;
      else 
      ringState = LOW;
      rings ++;
    }
      
      if ((ringState == LOW) && (rings >1)) {
        Serial.print("PHONE NOT PICKED UP STOP RINGING ");
        rings = 0;
        break;
        

 // ringAction.setInterval(INTERVAL); //schedule next call
    
    }
    
    }
    
    //phone is ringing and is picked up
    Serial.println("3 hookval - trackthree HIGH, relayswitch HIGH - PHONE PICKED UP PLAYING MESSAGE ");
    digitalWrite(relayswitch, HIGH);               //relay switch is high - plays through earpiece
    digitalWrite(trackthree, HIGH); // play track 3 answer message
    delay(200);
    digitalWrite(trackthree, LOW);
    while (digitalRead(switchPin)==HIGH) {/*wait*/
    }
    Serial.print("4 hookval- trackstop HIGH, relayswitch LOW - PHONE PUT DOWN STOP MESSAGE PLAYING ");
    digitalWrite(trackstop, HIGH);
    delay(200);
    digitalWrite(trackstop, LOW);
    digitalWrite(relayswitch, LOW);               //relay switch is LOW - main speaker
    Serial.println("END OF VOID RING LOOP ");
    blinks=0;
    ringAction.setInterval(INTERVAL); //schedule next call
  
  }
}

just in case anyone is interested here is the entire code

#include <TimedAction.h>

//this initializes a TimedAction class to make a phone ring every 30mins.
const unsigned long INTERVAL = (60000*1); // 1 minute * x
TimedAction ringAction = TimedAction(INTERVAL, ring);

//pin / state variables
int trackone = 13;
const int tracktwo = 12;
int trackthree = 11;
int trackstop = 10;
int relayswitch = 9;
int switchPin = 2;  // switch attached on pin 2
int hookState; // reads the state of switchPin and stores it
int oldHookState;
int softswitch;
int hookval1;
int hookval2;
int rings = 0;

void(* resetFunc) (void) = 0;

// Variables will change:
int ledState = LOW;             // ledState used to set the LED
int ringState = LOW;
long previousMillis = 0;        // will store last time LED was updated
long previousMillis2 = 0;

// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long interval = 500;           // interval at which to blink (milliseconds)
long interval2 = (1000*15);    // time ringing track plays before retuning to main loop


int blinks = 0;

void setup(){
  pinMode(switchPin,INPUT);

  pinMode(trackone,OUTPUT);
  pinMode(tracktwo,OUTPUT);
  pinMode(trackthree,OUTPUT);
  pinMode(trackstop,OUTPUT);
  pinMode(relayswitch,OUTPUT);
  
  hookval1=hookval2= digitalRead(switchPin);
  softswitch = 0;
  oldHookState = hookState = digitalRead(switchPin); //reading input on pin 2
  Serial.begin(9600);
}

void loop(){

  hookState = digitalRead(switchPin);
  hookval1 = hookState;
  delay(20);
  hookval2 = hookState;
  

  ringAction.check();


  if ((hookState==HIGH) && (softswitch == 0) && (hookval1 ==hookval2)){ //phone is picked up without ringing - play dial tone track
    Serial.println("loop phone up");
    digitalWrite(relayswitch, HIGH); //relay switch is high - plays through earpiece
    digitalWrite(trackone, HIGH);
    delay(400);
    digitalWrite(trackone, LOW);
    softswitch = 1;
  }

    //IF THIS SHOULD TRIGGER EVERY LOOP WHEN PHONE IS DOWN IT SHOULD BE SIMPLY 'else'
      if((hookState == LOW) && (softswitch ==1) && (hookval1 ==hookval2)) { // when phone is put down play track with nothing on
      Serial.println("loop phone down");

      digitalWrite(trackstop, HIGH);
      delay(300);
      digitalWrite(trackstop,LOW);
      delay(100);
      digitalWrite(relayswitch,LOW);                  // relayswitch is LOW
      oldHookState = hookState;
      softswitch = 0;
      
      //SHOULD THERE BE A CONDITION FOR THIS? WHAT DOES IT DO?
      // digitalWrite (trackstop,LOW);
      //digitalWrite (trackone,LOW);
      //digitalWrite(relayswitch,LOW);
    
  }

}

void ring (){
  
 

  if (hookState==HIGH) {
    ringAction.setInterval(30*1000); //check again in 30 sec because the phone was already lifted
  } 
  else { // start ringing
    Serial.println("2 hookval LOW. start ringing: tracktwo HIGH, relay LOW - START VOID RING LOOP ");
    digitalWrite(relayswitch, LOW); //relay switch is LOW - main speaker
    while (digitalRead(switchPin)==LOW) {
     unsigned long currentMillis = millis();
 
  if(currentMillis - previousMillis > interval) {
    // save the last time you blinked the LED 
    previousMillis = currentMillis;   

    // if the LED is off turn it on and vice-versa:
    if ((ledState == LOW) && (blinks <1))
      ledState = HIGH;
    else
      ledState = LOW;
      blinks ++;

    // set the LED with the ledState of the variable:
    digitalWrite(tracktwo, ledState);
  }
  
      //TODO add a timeout to this endless loop, calls until pickup
      
    if (currentMillis - previousMillis2 > interval2) {
      previousMillis2 = currentMillis;  
      
      if (ringState == LOW)
      ringState = HIGH;
      else 
      ringState = LOW;
      rings ++;
    }
      
      if ((ringState == LOW) && (rings >1)) {
        Serial.print("PHONE NOT PICKED UP STOP RINGING ");
        rings = 0;
        break;
    }
    
    }
    
    //phone is ringing and is picked up
    Serial.println("3 hookval - trackthree HIGH, relayswitch HIGH - PHONE PICKED UP PLAYING MESSAGE ");
    digitalWrite(relayswitch, HIGH);               //relay switch is high - plays through earpiece
    digitalWrite(trackthree, HIGH); // play track 3 answer message
    delay(200);
    digitalWrite(trackthree, LOW);
    while (digitalRead(switchPin)==HIGH) {/*wait*/
    }
    Serial.print("4 hookval- trackstop HIGH, relayswitch LOW - PHONE PUT DOWN STOP MESSAGE PLAYING ");
    digitalWrite(trackstop, HIGH);
    delay(200);
    digitalWrite(trackstop, LOW);
    digitalWrite(relayswitch, LOW);               //relay switch is LOW - main speaker
    Serial.println("END OF VOID RING LOOP ");
    blinks=0;
    ringAction.setInterval(INTERVAL); //schedule next call
  
  }
}

What if you change:

while (digitalRead(switchPin)==LOW) {
     unsigned long currentMillis = millis();

to:

unsigned long currentMillis = millis();
previousMillis = millis();
while (digitalRead(switchPin)==LOW) {

then move this:

    while (digitalRead(switchPin)==HIGH) {/*wait*/
    }
    Serial.print("4 hookval- trackstop HIGH, relayswitch LOW - PHONE PUT DOWN STOP MESSAGE PLAYING ");
    digitalWrite(trackstop, HIGH);
    delay(200);
    digitalWrite(trackstop, LOW);
    digitalWrite(relayswitch, LOW);               //relay switch is LOW - main speaker
    Serial.println("END OF VOID RING LOOP ");
    blinks=0;
    ringAction.setInterval(INTERVAL); //schedule next call

below the while (digitalRead(switchPin)==LOW) loop.

NOT TESTED AND PROBABLY NOT A COMPLETE SOLUTION, YES I WANTED TO WRITE IN FULL CAPS :sunglasses:

cheers, I'll let you know how it goes.

Alphabeta; i tried making both those adjustments to the code and they didnt work out so well.

with just the first change

What if you change: Code: while (digitalRead(switchPin)==LOW) { unsigned long currentMillis = millis();

to: Code: unsigned long currentMillis = millis(); previousMillis = millis(); while (digitalRead(switchPin)==LOW) {

the code stopped before the phone started ringing.

with both changes make the code remained in a continuous loop in the ring loop also without ringing.

still stuck!

#include <TimedAction.h>

//this initializes a TimedAction class to make a phone ring every 30mins.
const unsigned long INTERVAL = (60000*1); // 1 minute * x
TimedAction ringAction = TimedAction(INTERVAL, ring);

//pin / state variables
int trackone = 13;
const int tracktwo = 12;
int trackthree = 11;
int trackstop = 10;
int relayswitch = 9;
int switchPin = 2;  // switch attached on pin 2
int hookState; // reads the state of switchPin and stores it
int oldHookState;
int softswitch;
int hookval1;
int hookval2;
int rings = 0;

void(* resetFunc) (void) = 0;

// Variables will change:
int ledState = LOW;             // ledState used to set the LED
int ringState = LOW;
long previousMillis = 0;        // will store last time LED was updated
long previousMillis2 = 0;

// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long interval = 500;           // interval at which to blink (milliseconds)
long interval2 = (1000*15);    // time ringing track plays before retuning to main loop


int blinks = 0;

void setup(){
  pinMode(switchPin,INPUT);

  pinMode(trackone,OUTPUT);
  pinMode(tracktwo,OUTPUT);
  pinMode(trackthree,OUTPUT);
  pinMode(trackstop,OUTPUT);
  pinMode(relayswitch,OUTPUT);
  
  hookval1=hookval2= digitalRead(switchPin);
  softswitch = 0;
  oldHookState = hookState = digitalRead(switchPin); //reading input on pin 2
  Serial.begin(9600);
}

void loop(){

  hookState = digitalRead(switchPin);
  hookval1 = hookState;
  delay(20);
  hookval2 = hookState;
  

  ringAction.check();


  if ((hookState==HIGH) && (softswitch == 0) && (hookval1 ==hookval2)){ //phone is picked up without ringing - play dial tone track
    Serial.println("loop phone up");
    digitalWrite(relayswitch, HIGH); //relay switch is high - plays through earpiece
    digitalWrite(trackone, HIGH);
    delay(400);
    digitalWrite(trackone, LOW);
    softswitch = 1;
  }

    //IF THIS SHOULD TRIGGER EVERY LOOP WHEN PHONE IS DOWN IT SHOULD BE SIMPLY 'else'
      if((hookState == LOW) && (softswitch ==1) && (hookval1 ==hookval2)) { // when phone is put down play track with nothing on
      Serial.println("loop phone down");

      digitalWrite(trackstop, HIGH);
      delay(300);
      digitalWrite(trackstop,LOW);
      delay(100);
      digitalWrite(relayswitch,LOW);      // relayswitch is LOW
      oldHookState = hookState;
      softswitch = 0;
      
      //SHOULD THERE BE A CONDITION FOR THIS? WHAT DOES IT DO?
      // digitalWrite (trackstop,LOW);
      //digitalWrite (trackone,LOW);
      //digitalWrite(relayswitch,LOW);
    
  }

}

void ring (){
  
  if (hookState==HIGH) {
    ringAction.setInterval(30*1000); //check again in 30 sec because the phone was already lifted
  }
  else { // start ringing
    Serial.println("2 hookval LOW. start ringing: tracktwo HIGH, relay LOW - START VOID RING LOOP ");
    digitalWrite(relayswitch, LOW); //relay switch is LOW - main speaker
  
    unsigned long currentMillis = millis();
    previousMillis = millis();
      while (digitalRead(switchPin)==LOW) {
      //this will loop until timeout
      //TODO RING HERE
      
      if(currentMillis - previousMillis > interval) {
        break;
      }
    }
    
    //phone is ringing and is picked up
    Serial.println("3 hookval - trackthree HIGH, relayswitch HIGH - PHONE PICKED UP PLAYING MESSAGE ");
    digitalWrite(relayswitch, HIGH);       //relay switch is high - plays through earpiece
    digitalWrite(trackthree, HIGH); // play track 3 answer message
    delay(200);
    digitalWrite(trackthree, LOW);
    while (digitalRead(switchPin)==HIGH) {/*wait*/
    }
    Serial.print("4 hookval- trackstop HIGH, relayswitch LOW - PHONE PUT DOWN STOP MESSAGE PLAYING ");
    digitalWrite(trackstop, HIGH);
    delay(200);
    digitalWrite(trackstop, LOW);
    digitalWrite(relayswitch, LOW);       //relay switch is LOW - main speaker
    Serial.println("END OF VOID RING LOOP ");
    blinks=0;
    ringAction.setInterval(INTERVAL); //schedule next call
  
  }
}

You need to implement something in at the TODO comment

Sidenote: The program is getting a bit hard to read and you keep a lot of states that I do not think you need.

This program should have a pretty basic flow.

  • Read state of phone

  • If it is time to ring the phone (determined by timed action)

  • a ring function will be called, this will NOT return until the ring state is complete

  • if the phone is already off the hook, set interval to 30 sec and return

  • else enter a timed loop that lasts for x ms and ring the phone

  • if phone is picked up then play the message and when the phone is put back down reset interval and return

  • if phone is never picked up simply return

  • if the state of the phone was lifted then play the busy message for as long as it is

thanks for the suggestions. i tried the changes, but this time the phone doesn’t ring it just skips through the ring loop continuously, not returning to the void loop. this is the code after the adjustments:

#include <TimedAction.h>

//this initializes a TimedAction class to make a phone ring every 30mins.
const unsigned long INTERVAL = (60000*1); // 1 minute * x
TimedAction ringAction = TimedAction(INTERVAL, ring);

//pin / state variables
int trackone = 13;
const int tracktwo = 12;
int trackthree = 11;
int trackstop = 10;
int relayswitch = 9;
int switchPin = 2;  // switch attached on pin 2
int hookState; // reads the state of switchPin and stores it
int oldHookState;
int softswitch;
int hookval1;
int hookval2;
int rings = 0;

void(* resetFunc) (void) = 0;

// Variables will change:
int ledState = LOW;             // ledState used to set the LED
int ringState = LOW;
long previousMillis = 0;        // will store last time LED was updated
long previousMillis2 = 0;

// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long interval = 500;           // interval at which to blink (milliseconds)
long interval2 = (1000*15);    // time ringing track plays before retuning to main loop


int blinks = 0;

void setup(){
  pinMode(switchPin,INPUT);

  pinMode(trackone,OUTPUT);
  pinMode(tracktwo,OUTPUT);
  pinMode(trackthree,OUTPUT);
  pinMode(trackstop,OUTPUT);
  pinMode(relayswitch,OUTPUT);

  hookval1=hookval2= digitalRead(switchPin);
  softswitch = 0;
  oldHookState = hookState = digitalRead(switchPin); //reading input on pin 2
  Serial.begin(9600);
}

void loop(){

  hookState = digitalRead(switchPin);
  hookval1 = hookState;
  delay(20);
  hookval2 = hookState;


  ringAction.check();


  if ((hookState==HIGH) && (softswitch == 0) && (hookval1 ==hookval2)){ //phone is picked up without ringing - play dial tone track
    Serial.println("loop phone up");
    digitalWrite(relayswitch, HIGH); //relay switch is high - plays through earpiece
    digitalWrite(trackone, HIGH);
    delay(400);
    digitalWrite(trackone, LOW);
    softswitch = 1;
  }

  //IF THIS SHOULD TRIGGER EVERY LOOP WHEN PHONE IS DOWN IT SHOULD BE SIMPLY 'else'
  if((hookState == LOW) && (softswitch ==1) && (hookval1 ==hookval2)) { // when phone is put down play track with nothing on
    Serial.println("loop phone down");

    digitalWrite(trackstop, HIGH);
    delay(300);
    digitalWrite(trackstop,LOW);
    delay(100);
    digitalWrite(relayswitch,LOW);                  // relayswitch is LOW
    oldHookState = hookState;
    softswitch = 0;

    //SHOULD THERE BE A CONDITION FOR THIS? WHAT DOES IT DO?
    // digitalWrite (trackstop,LOW);
    //digitalWrite (trackone,LOW);
    //digitalWrite(relayswitch,LOW);

  }

}

void ring (){



  if (hookState==HIGH) {
    ringAction.setInterval(30*1000); //check again in 30 sec because the phone was already lifted
  } 
  else { // start ringing
    Serial.println("relay LOW - START VOID RING LOOP ");
    digitalWrite(relayswitch, LOW); //relay switch is LOW - main speaker

    unsigned long currentMillis = millis();
    previousMillis = millis();
    previousMillis2 = millis();

    while (digitalRead(switchPin)==LOW) {

      if(currentMillis - previousMillis > interval)
        // save the last time you blinked the LED

      // if the LED is off turn it on and vice-versa
      if ((ledState == LOW) && (blinks <1))
        ledState = HIGH;
      else
        ledState = LOW;
      blinks ++;
      // set the LED with the ledState of the variable:
      digitalWrite(tracktwo, ledState);
      Serial.println ("START RINGING");


      //TODO add a timeout to this endless loop, calls until pickup

      if (currentMillis - previousMillis2 > interval2) {
        break;

        /* 
         if (ringState == LOW)
         ringState = HIGH;
         else 
         ringState = LOW;
         rings ++;
         }
         
         if ((ringState == LOW) && (rings >1)) {
         Serial.print("PHONE NOT PICKED UP STOP RINGING ");
         rings = 0;
         
         */

       

      }


      //phone is ringing and is picked up
      Serial.println("3 hookval - trackthree HIGH, relayswitch HIGH - PHONE PICKED UP PLAYING MESSAGE ");
      digitalWrite(relayswitch, HIGH);               //relay switch is high - plays through earpiece
      digitalWrite(trackthree, HIGH); // play track 3 answer message
      delay(200);
      digitalWrite(trackthree, LOW);
      while (digitalRead(switchPin)==HIGH) {/*wait*/
      }
      Serial.print("4 hookval- trackstop HIGH, relayswitch LOW - PHONE PUT DOWN STOP MESSAGE PLAYING ");
      digitalWrite(trackstop, HIGH);
      delay(200);
      digitalWrite(trackstop, LOW);
      digitalWrite(relayswitch, LOW);               //relay switch is LOW - main speaker
      Serial.println("END OF VOID RING LOOP ");
      blinks=0;
      ringAction.setInterval(INTERVAL); //schedule next call

    }
  }
}

Any ideas anyone? i'm still stuck with this. the first bit of code i posted seemed to come the closest to doing what i wanted - it only had a couple of problems with it.

My idea is that you start over. Implement the easy parts first, such as the 'busy signal' when phone is not ringing but the handle is lifted. Then go from that, and add the ringing and the message playback.

I bet you'll end up with about half the number of lines and a much more clean code. Forget all the code that handles multiple calls and states between each iteration. You need to handle each iteration like the previously outlined matter.

PSEUDOCODE:

loop
  if handle is lifted play busy tone
  check timed action

ring
  if handle is lifted set interval to 30000 and return
  else loop for 30 seconds and sound the ringtone
    if handle is lifted play message
  reset interval

ok, thanks. I'll let you know how it goes...

Good luck! :) Remember to keep it simple 8-)

does anyone have a suggestion of how to write this statement.

else loop for 30 seconds and sound the ringtone

i have tried    while (currentMillis - previousMillis2 < interval2) { // ring for x time but i doesn’t work.

How about:

previousMillis = millis();
while (millis() - previousMillis < interval) { // ring for x time

fixed the problem!

just had to move set 'rings = 0' before the while loop.

 else { // start ringing
    Serial.println("2 hookval LOW. start ringing: tracktwo HIGH, relay LOW - START VOID RING LOOP ");
    digitalWrite(relayswitch, LOW); //relay switch is LOW - main speaker
            rings = 0;
    while (digitalRead(switchPin)==LOW) {
     unsigned long currentMillis = millis();

thanks for all the help so far :)