Loopy Problem - Timed Action

Hi there,

i have a problem with the timed action loop of this code. it only seems to be ready to execute each if statement within the (void ring) loop each time the Timed Action loop is run, in this case every minute. Is there a way of changing the code structure in the loop so the whole series of if statements will be worked through in one go and are responsive to the switch input triggering them? for example it reaches and carries out this bit of codeelse if ((hookval== LOW) && (hookval == hookval2) && (softswitch == 0)) { // check if phone is on the hook, if it is start ringing

then pauses for a minute before checking the switch again and carring out the next if statement here:  if ((hookval == HIGH) && (hookval == hookval2) && (softswitch == 3)) { //phone is ringing and is picked up.

it does the same thing again when it reaches the next if statement. Here is the whole piece of code, any help appreciated :slight_smile:

#include <TimedAction.h>

//this initializes a TimedAction class to make a phone ring every 30mins.
TimedAction ringAction = TimedAction(60000, ring);

//pin / state variables
int trackone = 13;
int tracktwo = 12;
int trackthree = 11;
int trackstop = 10;
int relayswitch = 9;
int switchPin = 2;  // switch attached on pin 2
int softswitch;
int hookState; // reads the state of switchPin and stores it
int hookval; //reads the switchPin state in loop
int hookval2;

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

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

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

void loop(){

  hookval = digitalRead(switchPin);
  delay(50);
  hookval2 = digitalRead(switchPin);


  ringAction.check();


  if ((hookval == HIGH) && (hookval == hookval2) && (softswitch == 0)) { //phone is picked up without ringing - play dial tone track
    digitalWrite(relayswitch, HIGH);               //relay switch is high - plays through earpiece
    digitalWrite(trackone, HIGH);
    delay(400);
    digitalWrite(trackone, LOW);
    delay(200);
    softswitch = 1;  //stops code running in continuous loop
    Serial.print("loop softswitch - picked up no ring ");
  Serial.println(softswitch);
  }

  if ((hookval == LOW) && (hookval == hookval2) && (softswitch ==1)) { // when phone is put down play track with nothing on
    digitalWrite(relayswitch,LOW);                  // relayswitch is LOW
    digitalWrite(trackstop, HIGH);
    delay(400);
    digitalWrite(trackstop,LOW);
    delay(200);
    softswitch = 0;  //phone is put down and is not ringing - this is the beginning of the loop
    Serial.print("loop softswitch - put down no ring ");
  Serial.println(softswitch);
  }

  if ((hookval == LOW) && (hookval == hookval2) && (softswitch == 0)){
    digitalWrite (trackstop,LOW);
    digitalWrite (trackone,LOW);
    digitalWrite(relayswitch,LOW);                  // relayswitch is LOW - main speaker
  } 

}

void ring (){ 

  hookval = digitalRead(switchPin);
  delay(20);
  hookval2 = digitalRead(switchPin);

if ((hookval == HIGH) &&(hookval == hookval2) && (softswitch == 1)) { 
  Serial.print("1 hookval HIGH - START VOID RING LOOP ");
  Serial.println(hookval);
    Serial.print("1 softswitch - phone is picked up wait for 30 secs ");
  Serial.println(softswitch);
    delay(30000); //if phone is picked up already wait 30 secs then check again.
  
}
else if ((hookval== LOW) && (hookval == hookval2) && (softswitch == 0)) { // check if phone is on the hook, if it is start ringing
        Serial.print("2 hookval LOW - START VOID RING LOOP ");
      Serial.println(hookval);
    Serial.print("2.1 softswitch status ringing begin ");
  Serial.println(softswitch);
    digitalWrite(relayswitch, LOW);               //relay switch is LOW - main speaker
    digitalWrite(tracktwo, HIGH); // trigger track two - ringtone
    delay (1000);
    digitalWrite(tracktwo, LOW);
    softswitch = 3;   // set softswitch to 3 - phone in ringing phase
}
 /*   hookval = digitalRead(switchPin);
  delay(20);
  hookval2 = digitalRead(switchPin);
  */
  
  if ((hookval == HIGH) && (hookval == hookval2) && (softswitch == 3)) { //phone is ringing and is picked up
    Serial.print("3 hookval PHONE PICKED UP PLAYING MESSAGE ");
    Serial.println(hookval);
    digitalWrite(relayswitch, HIGH);               //relay switch is high - plays through earpiece
    digitalWrite(trackthree, HIGH); // play track 3 answer message
    delay(1000);
    digitalWrite(trackthree, LOW);
    softswitch = 4; // phone has rung, been picked up and message is playing
    Serial.print("3 softswitch ");
  Serial.println(softswitch);
  }
  if ((hookval == LOW) && (hookval == hookval2) && (softswitch == 4)) { // phone put down after it has rung, been picked up and message is playing
    Serial.print("4 hookval PHONE PUT DOWN STOP MESSAGE PLAYING ");
    Serial.println(hookval);
    digitalWrite(trackstop, HIGH);
    delay(400);
    digitalWrite(trackstop, LOW);
    digitalWrite(relayswitch, LOW);               //relay switch is LOW - main speaker
    softswitch = 0; // ready to start loop at the beggining again.
    Serial.print("4 softswitch ");
  Serial.println(softswitch);
   Serial.println("END OF VOID RING LOOP ");
  
  }

}

I don't understand the question, or what you are doing. Please explain the purpose of the whole program. What is the TimedAction function supposed to do for you?

Here's what i trying to do:

I wanted an old rotary telephone to ring at a set interval, say 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.

hardware setup - A micro switch under the handset sends HIGH/LOW signal to pin 2 on the arduino indicating when the handset has been picked up/put down. the arduino then triggers one of four tracks on an MP3 triggerV2 by sending a short HIGH 5V signal out on pins 13-10. A SPDT relay controlled on pin9, toggles the output of the mp3 player between an internal speaker in the phone when the handset is down (for ringing) and a speaker in the handset when the phone is picked up.

In my limited understanding of coding i thought that putting the code to do with the phone being picked up - turning on the relay activating the speaker in the handset and playing back the first mp3 track of a dial tone - in the void loop. Then using the timed action to activate another set of code at a timed interval ie every half a hour when the phone rings that leads to a different set of if statements.
The problem seems to be my assumption that the timed action loop would run all the way through before returning to the void loop. It caries out some of the if statements - the phone starts ringing, then waits till the next interval the timed action loop is called to start the next if statement (phone is picked up) and so on... Was i wrong in thinking timed action is suitable and should i be looking for a different approach to the code?

/*
PSEUDOCODE comments
implement each step and you should be well on your way
*/

#include <TimedAction.h>

//this initializes a TimedAction class to make a phone ring every 30mins.
TimedAction ringAction = TimedAction(60000, ring);


void setup(){
}

void loop(){
  ringAction.check();
  //if the phone is picked up, play the beep for as long at is it picked up
}


void ring() {
   //start the ringer
   //loop for as long as you want to ring
     //if the phone is picked up
       //play the message
   //stio the ringer
}

You could start with a logic like that. It might be what you've done. I do not have time to read now, unfortunantly.

Thanks AlphaBeta, but i think that is pretty much where i have got to already. just not sure why it is not running through the code in one sequence.

remember that ring() only gets called ONCE per interval, and that loop is halted while executing it.

ok. but i'm still not sure how to make the ring loop run in one go. if i cant is there an alternative i should consider? is it possible to recall (after the first interval of half an hour) the timed action with an interrupt? i would need to do this twice to complete the loop.

I would suggest that you try to implement the pseudocode I gave :slight_smile: It should work.

You basically have three states:
1] no tone while phone down
2] busy tone while phone up
3] message while phone up while inside a while loop in ring

I suggest you code it like that as well. :slight_smile:

It could be that 30 second delay in ring() that is causing a problem. If the first if text is false, it should cause the body to be skipped, and the next (else) if to be tested.

Since you have several tests in ring(), but haven't shared any results, it's really tough to help you any more.

Say ring() gets called, and the handset is down. What happens?

Say ring gets called, and the handset is not down. What happens?

The more details, minus the frustration that is is apparent you are feeling, the better chance that we can help you. As AlphaBeta says, what you want to do should work. The issue is most likely a flaw in the logic in ring(), but we need some hints to narrow down where to look.

Alphabeta, thanks again, I'll try rewriting the code to that structure and let you know what happens. i think the idea of containing the code in a while loop sounds good.

The only thing is the logic i was working to had a couple more stages
main loop:

  1. handset down - no tone
  2. handset picked up without ringing - dial tone

ring loop to happen every 30 mins:
3. check if handset is picked up or not. if it is wait 30 seconds then check again. if it isn't start ringing.
4 when handset is picked up play message.
5 when handset is put down stop playing message and return to main loop

PaulS. the results i have got from the code so far:
one statement is run each time the loop is called. it then waits the 60 second interval for the loop to be called again before moving on to the next statement. for example the last time i ran the code;
handset is up. the first if statement returns true and there is a delay it should be 30 seconds then check to see whether the handset is up or down.
instead after 30 seconds we return to the main loop. then if the handset is down after another 30 seconds the if statement turning on the ringing come back true.

at this point if the handset is picked up it should play the message. if the handset stays picked up, after a minute the next statement comes back true. the message plays. if the handset is put down at any point it is another minute before the next statement comes back true.

whilst each if statement has a minute wait between each time the ring loop is called it does not allow any functions of the main loop to happen during this time, giving the appearance of it being stuck whilst waiting for the recall.

here is the serial window log of the code being run

loop softswitch - picked up no ring 1
1 hookval HIGH - START VOID RING LOOP 1
1 softswitch - phone is picked up wait for 30 secs 1
loop softswitch - put down no ring 0
2 hookval LOW - START VOID RING LOOP 0
2.1 softswitch status ringing begin 0
3 hookval PHONE PICKED UP PLAYING MESSAGE 1
3 softswitch 4
4 hookval PHONE PUT DOWN STOP MESSAGE PLAYING 0
4 softswitch 0
END OF VOID RING LOOP 
2 hookval LOW - START VOID RING LOOP 0
2.1 softswitch status ringing begin 0

Looks like i've managed to write some really bad code... Thanks for your patience... I'll try and revise it and post something better.

I just hacked a bit on your code:

#include <TimedAction.h>

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

//pin / state variables
int trackone = 13;
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;


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

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

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

void loop(){
  
  hookState = digitalRead(switchPin);

  ringAction.check();


  if (hookState==HIGH) { //phone is picked up without ringing - play dial tone track
    Serial.print("loop phone up");
    digitalWrite(relayswitch, HIGH); //relay switch is high - plays through earpiece
    digitalWrite(trackone, HIGH);
    delay(400);
    digitalWrite(trackone, LOW);
  } 
  //IF THIS SHOULD TRIGGER EVERY LOOP WHEN PHONE IS DOWN IT SHOULD BE SIMPLY 'else'
  else if (hookState!=oldHookState) { // when phone is put down play track with nothing on
    Serial.print("loop phone down");
    digitalWrite(relayswitch,LOW);                  // relayswitch is LOW
    digitalWrite(trackstop, HIGH);
    delay(400);
    digitalWrite(trackstop,LOW);
    delay(200);
    oldHookState = hookState;
    //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.print("2 hookval LOW - START VOID RING LOOP ");
    digitalWrite(relayswitch, LOW); //relay switch is LOW - main speaker
    while (digitalRead(switchPin)==LOW) {
      digitalWrite(tracktwo, HIGH); // trigger track two - ringtone
      delay (1000);
      digitalWrite(tracktwo, LOW);
      //TODO add a timeout to this endless loop, calls until pickup
    }
    //phone is ringing and is picked up
    Serial.print("3 hookval PHONE PICKED UP PLAYING MESSAGE ");
    digitalWrite(relayswitch, HIGH);               //relay switch is high - plays through earpiece
    digitalWrite(trackthree, HIGH); // play track 3 answer message
    delay(1000);
    digitalWrite(trackthree, LOW);
    while (digitalRead(switchPin)==HIGH) {/*wait*/}
    Serial.print("4 hookval PHONE PUT DOWN STOP MESSAGE PLAYING ");
    digitalWrite(trackstop, HIGH);
    delay(400);
    digitalWrite(trackstop, LOW);
    digitalWrite(relayswitch, LOW);               //relay switch is LOW - main speaker
    Serial.println("END OF VOID RING LOOP ");
    ringAction.setInterval(INTERVAL); //schedule next call 
  }
}

it is NOT TESTED but it does compile
:slight_smile:

What does it do and what is wrong with it?

Thanks for the hacked code. just suffered total computer failure, which has set things back a few hours... managed to borrow another one from a friend, so as soon as i've loaded fresh arduino software i'll report back on the code...

It works! with a bit of modification to the main loop. thanks AlphaBeta.

one improvement i still want to make (which you added a to do note to) is add more control to the phone ringing. at the moment it triggers the mp3 player once by turning the pin HIGH. this means that the phone will only ring till the end of the mp3 track(im not sure what the maximum track length is off the top of my head) but the code will still be in the same place- waiting for the handset to be picked up. I tried adding a delay(say 5 seconds,) when the pin turned LOW that could correspond to the length of the mp3 track but this made the response to the phone being picked up being up to 5 seconds as well.
what would a adding a timeout as you talked about do to this? and is there a way of re -triggering the ringing mp3 track at a given interval without compromising the responsiveness of the phone being picked up?

Also i see what you meant now with adding while statements, so much still to learn!

here is the hacked code with the changes i made to the loop

#include <TimedAction.h>

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

//pin / state variables
int trackone = 13;
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;

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(relayswitch,LOW);                  // relayswitch is LOW
      digitalWrite(trackstop, HIGH);
      delay(400);
      digitalWrite(trackstop,LOW);
      delay(200);
      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) {
      digitalWrite(tracktwo, HIGH); // trigger track two - ringtone
      delay (1000);
      digitalWrite(tracktwo, LOW);
      //TODO add a timeout to this endless loop, calls until pickup
    }
    //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(1000);
    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(400);
    digitalWrite(trackstop, LOW);
    digitalWrite(relayswitch, LOW);               //relay switch is LOW - main speaker
    Serial.println("END OF VOID RING LOOP ");
    ringAction.setInterval(INTERVAL); //schedule next call
  }
}