Proper way to run functions in loop

Question

I am having a problem getting my functions to run properly from the loop.

I am building a model railroad crossing gate and need to run the 4 processes once the sensor signals the train entering the crossing zone. For right now I need to learn how to "function" in C++ running in loop, as I'm a C# ASP.Net guy.

My thought process was in loop to

• Check entry sensor, this is all the loop should ever do UNTIL its tripped
• Once tripped, run the 4 processes functions (close the gates, turn on the sound, turn on the flashers, power down the road that crosses the tracks)
• After the entry sensor is tripped and the processes run (the sound and flashers should operate continuously until the exit sensor is cleared) the exit sensor must be monitored, and once its tripped, its monitored until it goes static again indicating the train has cleared. Then the 4 process are run to shut down the flashers, shut the sound, power up the road and raise the gates.

Right now I'm just trying to get the sound functioning properly. Right now, as the code stands, it plays for a split second then stops. I need that file playing until I call the function to stop it.

Here is the code so far. I've removed the includes and defines for brevity.

// These are the pins used for the music maker shield
#define SHIELD_RESET  -1      // VS1053 reset pin (unused!)
#define SHIELD_CS     7      // VS1053 chip select pin (output)
#define SHIELD_DCS    6      // VS1053 Data/command select pin (output)

// These are common pins between breakout and shield
#define CARDCS 4     // Card chip select pin
// DREQ should be an Int pin, see http://arduino.cc/en/Reference/attachInterrupt
#define DREQ 3       // VS1053 Data request, ideally an Interrupt pin

// create shield-example object
Adafruit_VS1053_FilePlayer musicPlayer = Adafruit_VS1053_FilePlayer(SHIELD_RESET, SHIELD_CS, SHIELD_DCS, DREQ, CARDCS);

//global variables required
boolean crossingSignalInOperation = false;
boolean signalEntrySensor = false;
boolean signalExitSensor = false;
boolean debugMode = false;

boolean checkEntrySensor() {
    //check entry sensor, if tripped return true otherwise false
    /*if () return true;
    return false;*/
    return true;  //hardcoded for testing
}

void setup() {
    // put your setup code here, to run once:
    Serial.begin(9600);
    debugMode = true;   //set this to false once code is fully operational
    //initialise music maker shield
        if (!musicPlayer.begin()) { 
            if (debugMode)
            {
                Serial.println(F("Couldn't find VS1053, do you have the right pins defined?"));
            }

        while (1);
    }
    // 1st parameter is left, second parameter is right             
    musicPlayer.setVolume(20, 20);
    if (debugMode)
    {
        Serial.println(F("VS1053 found"));
    }

    // initialise the SD card
    SD.begin(CARDCS);    

    //ensure our global booleans all start false
    crossingSignalInOperation = false;  

}

void loop() {
    // put your main code here, to run repeatedly

    signalEntrySensor = checkEntrySensor();
    if (signalEntrySensor == true) {  //only want to catch the FIRST time it senses
        if (crossingSignalInOperation == false)
        {
            //crossing signal is inactive, activate it
            // turn the sound on and return the music player object
            Adafruit_VS1053_FilePlayer mp = onSignalSound(musicPlayer,"trainxng.mp3");  

            while (mp.playingMusic) {
                // file is now playing in the 'background' so now's a good time
                // to do something else 
                if (!crossingSignalInOperation)
                {                   
                    crossingSignalInOperation = true;
                    //run the other three processes
                    //offRoadPower();
                    //lowerSignalArms();
                    //startSignalFlashers();

                }               
            }                   
        }
    }
 }

Here are the functions;

Adafruit_VS1053_FilePlayer onSignalSound(Adafruit_VS1053_FilePlayer mp, const char* fileName) {
    // Play the train crossing sound file
    if (debugMode)
    {
        Serial.println(F("Playing track - "));
        Serial.print(fileName);
    }
    //all sound files are on the micro sd card on the music player shield
    if (!mp.startPlayingFile(fileName)) {
        Serial.println(F("Could not open file "));
        Serial.print(fileName);
        while (1);
    }
    return mp;
}
boolean offSignalSound(Adafruit_VS1053_FilePlayer mp) {
    if (mp.playingMusic)
    {
        mp.stopPlaying();
        return true;
    }
    return true;    
}

Any assistance would be appreciated.

Update:

New version using the state machine methodology suggested by JRobert and Edgar Any comments on structure?

// include SPI, MP3 and SD libraries
#include <SPI.h>
#include <Adafruit_VS1053.h>
#include <SD.h>
// include AccelStepper libraries
#include <MultiStepper.h>
#include <AccelStepper.h>
using namespace std;
/*-----( Declare Enums & Constants )-----*/
// Arduino Digital I/O pin numbers for MEGA
//note: digital pins 30-45 
//MEGA is a MUST for this project
enum {
    Relay1 = 30, Relay2 = 31, Relay3 = 32, Relay4 = 33, Relay5 = 34, Relay6 = 35,
    Relay7 = 36, Relay8 = 37, Relay9 = 38, Relay10 = 39, Relay11 = 40,
    Relay12 = 41, Relay13 = 42, Relay14 = 43, Relay15 = 44, Relay16 = 45
};

enum { RELAY_OFF = HIGH, RELAY_ON = LOW };

enum {
    CROSSINGZONE_CLEAR, // no train in crossing area, also initialized state
    CROSSINGZONE_OCCUPIED, // train detected by the entry sensor
    CROSSINGZONE_EXITING  // train detected by the exit sensor, when sensor clears state = CLEAR
};

 enum {
     EXITSENSOR_CLEAR, // no train has passed the crossing gate exit sensor
     EXITSENSOR_ENTERED, // train detected by the exit sensor
     EXITSENSOR_WAITINGTOCLEAR // train passed by the exit sensor, when sensor clears state = CLEAR

 };

byte state = CROSSINGZONE_CLEAR;
 //*-----( Declare Variables )-----*/
// hardware is an optocoupler 16 channel 12V relay
const byte relays[] = { Relay1,Relay2,Relay3,Relay4,
Relay5,Relay6,Relay7,Relay8,
Relay9,Relay10,Relay11,Relay12,
Relay13,Relay14,Relay15,Relay16 };
enum { maxRelayCount = sizeof relays / sizeof relays[0] };

byte entrySensorPin = 26;
byte exitSensorPin = 28;
const char* soundFile = "trainxng.mp3";
boolean debugMode = true;
boolean exitSensorTripped = false;

// These are the pins used for the music maker shield
#define SHIELD_RESET  -1      // VS1053 reset pin (unused!)
#define SHIELD_CS     7      // VS1053 chip select pin (output)
#define SHIELD_DCS    6      // VS1053 Data/command select pin (output)
// These are common pins between breakout and shield
#define CARDCS 4     // Card chip select pin
// DREQ should be an Int pin, see  http://arduino.cc/en/Reference/attachInterrupt
#define DREQ 3       // VS1053 Data request, ideally an Interrupt pin

AccelStepper gateMotor1Left(AccelStepper::DRIVER, 4, 3);  // 1=Stepper driver option, 4=Step pulse pin, 3=Direction
AccelStepper gateMotor1Right(AccelStepper::DRIVER, 6, 5);  // 1=Stepper driver option, 6=Step pulse pin, 5=Direction
AccelStepper gateMotor2Left(AccelStepper::DRIVER, 8, 7);  // 1=Stepper driver option, 8=Step pulse pin, 7=Direction
AccelStepper gateMotor2Right(AccelStepper::DRIVER, 10, 9);  // 1=Stepper driver option, 10=Step pulse pin, 9=Direction
Adafruit_VS1053_FilePlayer musicPlayer =
    Adafruit_VS1053_FilePlayer(SHIELD_RESET, SHIELD_CS, SHIELD_DCS, DREQ, CARDCS);


class CrossingGate {
  public:
    // hmm voids or booleans? 
    boolean GateFlashersOn(byte relayNumber) {
        digitalWrite(relayNumber, RELAY_ON);
        //need a check here to ensure its set LOW which is inactive for these relays
        return true;
    }
    boolean GateFlashersOff(byte relayNumber) {
        digitalWrite(relayNumber, RELAY_OFF);
        //need a check here to ensure its set HIGH which is inactive for these relays
        return true;
    }
    boolean GateArmFlashersOn(byte relayNumber) {
        digitalWrite(relayNumber, RELAY_ON);
        //need a check here to ensure its set LOW which is inactive for these relays
        return true;
     }
     boolean GateArmFlashersOff(byte relayNumber) {
         digitalWrite(relayNumber, RELAY_OFF);
        //need a check here to ensure its set HIGH which is inactive for these relays
        return true;
    }
    boolean GatesUp(byte relayNumber) {
        digitalWrite(relayNumber, RELAY_ON);
        //need a check here to ensure its set LOW which is inactive for these relays
        return true;
    }
    boolean GatesDown(byte relayNumber) {
        digitalWrite(relayNumber, RELAY_OFF);
        //need a check here to ensure its set HIGH which is inactive for these relays
        return true;
    }

};

 class CrossingGateRoad {
   public:
       void PowerOn(byte relayNumber) {
           digitalWrite(relayNumber, RELAY_ON);

       }
       void PowerOff(byte relayNumber) {
           digitalWrite(relayNumber, RELAY_OFF);

       }
};
//  Sensor methods
int CheckEntrySensor(byte esp) {
    if (digitalRead(esp) == HIGH)
    {
         state = CROSSINGZONE_CLEAR;
         return CROSSINGZONE_CLEAR;
    }
    else {
        state = CROSSINGZONE_OCCUPIED;
        return CROSSINGZONE_OCCUPIED;
    }
     return -1;
}
int CheckExitSensor(byte xsp) {
    if (digitalRead(xsp) == HIGH && state == CROSSINGZONE_OCCUPIED)
    {
        state = CROSSINGZONE_OCCUPIED;
        return CROSSINGZONE_OCCUPIED;
    }
    else if(digitalRead(xsp) == LOW && state == CROSSINGZONE_OCCUPIED)
    {
        state = CROSSINGZONE_CLEAR;
        return CROSSINGZONE_OCCUPIED;
    }
    return -1;
}

boolean CrossingBellsOn(Adafruit_VS1053_FilePlayer & mp, const char* soundFile) {
    if (!mp.startPlayingFile(soundFile)) {
        Serial.println("Could not open file trainxng.mp3 using the passed in object");
        while (1);
    }
    return true;
}
boolean CrossingBellsOff(Adafruit_VS1053_FilePlayer & mp) {
    if (mp.playingMusic) {
        mp.stopPlaying();
         return true;
    }
    if (mp.stopped())
    {
        return true;
    }
    return false;
}

CrossingGate xGate1Left;
CrossingGate xGate1Right;
CrossingGate xGate2Left;
CrossingGate xGate2Right;

CrossingGateRoad xGateRoad;

void setup() {
    Serial.begin(9600);
    // Set max speed. Steps per second
    gateMotor1Left.setMaxSpeed(500); 
    gateMotor2Left.setMaxSpeed(500); 
    gateMotor1Right.setMaxSpeed(500); 
    gateMotor2Right.setMaxSpeed(500); 
    // set motor acceleration
    gateMotor1Left.setAcceleration(35);  
    gateMotor2Left.setAcceleration(35);  
    gateMotor1Right.setAcceleration(35); 
    gateMotor2Right.setAcceleration(35);  
    // set motor speeds
    gateMotor1Left.setSpeed(20);
    gateMotor2Left.setSpeed(20);
    gateMotor1Right.setSpeed(20);
    gateMotor2Right.setSpeed(20);

    Serial.println("Adafruit VS1053 Library Test");
    // initialise the music player
    if (!musicPlayer.begin()) { // initialise the music player
         Serial.println(F("Couldn't find VS1053, do you have the right pins defined?"));
        while (1);
    }
    Serial.println(F("VS1053 found"));
    //musicPlayer.sineTest(0x44, 500);    // Make a tone to indicate VS1053 is working
    // check the card reader
    if (!SD.begin(CARDCS)) {
        Serial.println(F("SD failed, or not present"));
        while (1);  
    }
    Serial.println("SD OK!");
    // Set volume for left, right channels. lower numbers == louder volume!
    musicPlayer.setVolume(20, 20);

    // This option uses a pin interrupt. But DREQ must be on an
    //  interrupt pin. For Uno/Duemilanove/Diecimilla that's Digital #2 or #3 
    // See http://arduino.cc/en/Reference/attachInterrupt for other pins
    // * This is the preferred method
    if (!musicPlayer.useInterrupt(VS1053_FILEPLAYER_PIN_INT))
        Serial.println(F("DREQ pin is not an interrupt pin"));

}
void loop() {

    switch (state) {
        case CROSSINGZONE_CLEAR:
          // Check entry sensor. If tripped, turn on the sound.
         if (CheckEntrySensor(entrySensorPin) == 1) {
            // first go around i'll try to tun all these here otherwise
            // I will break out to more states
            //these happen instantly
            xGateRoad.PowerOff(relays[0]);
            xGate1Left.GateArmFlashersOn(relays[1]);
            xGate1Right.GateArmFlashersOn(relays[2]);
            xGate2Left.GateArmFlashersOn(relays[3]);
            xGate2Right.GateArmFlashersOn(relays[4]);
            xGate1Left.GateFlashersOn(relays[5]);
            xGate1Right.GateFlashersOn(relays[6]);
            xGate2Left.GateFlashersOn(relays[7]);
            xGate2Right.GateFlashersOn(relays[8]);
            //these take some time
            xGate1Left.GatesDown(relays[9]);
            xGate1Right.GatesDown(relays[10]);
            xGate2Left.GatesDown(relays[11]);
            xGate2Right.GatesDown(relays[12]);
            //this is the last process, if it completes change state
            if (CrossingBellsOn(musicPlayer, soundFile)) {
                state = CROSSINGZONE_OCCUPIED;
            }           
        }
        else {
            state = CROSSINGZONE_CLEAR;
        }
        break;
    case CROSSINGZONE_OCCUPIED:     
        // Once zone is occupied, monitor the exit sensor
        if (CheckExitSensor(exitSensorPin) == 0) { // Exit sensor not tripped yet
            state = CROSSINGZONE_OCCUPIED;
        }
        else {
            exitSensorTripped = true;
        }
        if (exitSensorTripped)
        {
             // need to code here to wait until Exit Sensor has 1500 milliseconds where its clear
            // to indicate that the last car of train has passed it
            // if above set exitSensorTripped back to false and state to EXITING
            state = CROSSINGZONE_EXITING;
        }
        break;
    case CROSSINGZONE_EXITING:
        // Check if the train has cleared, then shut the sound.
        if (CheckExitSensor(exitSensorPin) == 1) {
            // first go around i'll try to tun all these here otherwise
            // I will break out to more states
            //these happen instantly
            xGateRoad.PowerOn(relays[0]);
            xGate1Left.GateArmFlashersOff(relays[1]);
            xGate1Right.GateArmFlashersOff(relays[2]);
            xGate2Left.GateArmFlashersOff(relays[3]);
            xGate2Right.GateArmFlashersOff(relays[4]);
            xGate1Left.GateArmFlashersOff(relays[5]);
            xGate1Right.GateArmFlashersOff(relays[6]);
            xGate2Left.GateArmFlashersOff(relays[7]);
            xGate2Right.GateArmFlashersOff(relays[8]);
            //these take some time
            xGate1Left.GatesUp(relays[9]);
            xGate1Right.GatesUp(relays[10]);
            xGate2Left.GatesUp(relays[11]);
            xGate2Right.GatesUp(relays[12]);
            //this is the last process, if it completes change state
            if (CrossingBellsOff(musicPlayer)) {
                state = CROSSINGZONE_CLEAR;
            }
        }
        break;
    }
}

Answer 1

Elaborating on JRobert's answer, which I think is definitively the way to go. Here is a typical way of writing a state machine with only three states:

1. No train in the area. You are monitoring the entry sensor.
2. You have detected the train on the entry sensor. You started playing the sound. You are monitoring the exit sensor.
3. You have detected the train on the exit sensor. You are still monitoring the same sensor in order to know when the train clears the area.

Notice that the actions you perform (starting and stopping the sound for instance) are not associated with a particular state but with a transition between states: you start the sound on the 1 → 2 transition and stop it on 3 → 1.

Here is the code for loop():

void loop() {
    static enum {
        NO_TRAIN,        // no train in the area
        TRAIN_ENTERED,   // train seen by the entry sensor
        TRAIN_EXITING    // train seen by the exit sensor
    } state = NO_TRAIN;  // initial state

    switch (state) {
        case NO_TRAIN:
            // Check entry sensor. If tripped, turn on the sound.
            if (checkEntrySensor()) {
                musicPlayer.startPlayingFile("trainxng.mp3");
                flashers.turnOn();
                gates.close();
                road.powerDown();
                state = TRAIN_ENTERED;
            }
            break;
        case TRAIN_ENTERED:
            // The exit sensor must be monitored.
            if (checkExitSensor()) {
                state = TRAIN_EXITING;
            }
            break;
        case TRAIN_EXITING:
            // Check if the train has cleared, then shut the sound.
            if (!checkExitSensor()) {
                musicPlayer.stopPlaying();
                flashers.turnOff();
                gates.open();
                road.powerUp();
                state = NO_TRAIN;
            }
            break;
    }
}

This may be a bit oversimplified: if an action need some time, you may have to create an extra state for it. For example, in order to avoid blocking the program while the barrier goes down, you can split the transition NO_TRAIN → TRAIN_ENTERED in two:

NO_TRAIN → LOWERING_BARRIER → TRAIN_ENTERED

and the action “lower the barrier” is itself split into “start lowering the barrier” (on the first transition) and “stop lowering the barrier” (on the second transition).

Answer 2

This is a classic state-machine problem. That isn't the only way to do it but it's just made for this kind of problem, one in which only certain external events are interesting at a given time; and when such an event occurs, the controller must tweak the outside world in some way, and then start paying attention to a different set of interesting events.

Does playing a sound file totally occupy the Arduino until the sound is done? And if so, can your process tolerate the controller being unresponsive for the duration of the sound track? Because otherwise you'll need something more capable than an Arduino, or you'll need to be able to program the Arduino to feed the sound in small pieces to a buffered sound card, so it (the Arduino) can continue monitoring the interesting set of possible events while the sound is playing. That probably means some clever programming with the SimpleTimer library or similar, or using interrupt service routines to inform some part of your code when an event occurs. Probably the ideal cases would be to have a "sound machine" that can play 'n' different tracks, and would do so autonomously when told to start playing track 'j'. Begins to sound like another MCU just handling sound, doesn't it?

Update:

Look up state machine diagrams - basically circles representing states and arrows representing transitions to a new state in response to an events. There are several ways to put the state machine's control of the external equipment; my choice is that each state "does something" then "waits for something to happen", where something to happen may include a clock signal indicating the event(s) it was waiting for didn't happen (i.e. a timeout). Look at some state diagrams to get an idea how they are used, then start diagramming your own project.

I draw the "happy path" straight down the page - that path that represents the normal or expected operation - with less usual or error paths drawn to either side. At that point you should be pretty confident in your algorithm.

There are several ways to code state machines - that'll be a further discussion.

Answer 3

I think your problem here is that you are using multiple instances of Adafruit_VS1053_FilePlayer.

Namely:

Adafruit_VS1053_FilePlayer musicPlayer = Adafruit_VS1053_FilePlayer(SHIELD_RESET, SHIELD_CS, SHIELD_DCS, DREQ, CARDCS);

That's two already.

Then you pass a copy here:

Adafruit_VS1053_FilePlayer onSignalSound(Adafruit_VS1053_FilePlayer mp, const char* fileName) {

And return another copy:

return mp;

All this creating/destroying copies is probably stopping the playback of the current copy.

I've reworked your code to avoid all that:

#include <Adafruit_VS1053.h>

// These are the pins used for the music maker shield
#define SHIELD_RESET  -1      // VS1053 reset pin (unused!)
#define SHIELD_CS     7      // VS1053 chip select pin (output)
#define SHIELD_DCS    6      // VS1053 Data/command select pin (output)

// These are common pins between breakout and shield
#define CARDCS 4     // Card chip select pin
// DREQ should be an Int pin, see http://arduino.cc/en/Reference/attachInterrupt
#define DREQ 3       // VS1053 Data request, ideally an Interrupt pin

// create shield-example object
Adafruit_VS1053_FilePlayer musicPlayer (SHIELD_RESET, SHIELD_CS, SHIELD_DCS, DREQ, CARDCS);

//global variables required
boolean crossingSignalInOperation = false;
boolean signalEntrySensor = false;
boolean signalExitSensor = false;
boolean debugMode = false;

boolean checkEntrySensor() {
    //check entry sensor, if tripped return true otherwise false
    /*if () return true;
    return false;*/
    return true;  //hardcoded for testing
}

void setup() {
    // put your setup code here, to run once:
    Serial.begin(9600);
    debugMode = true;   //set this to false once code is fully operational
    //initialise music maker shield
        if (!musicPlayer.begin()) { 
            if (debugMode)
            {
                Serial.println(F("Couldn't find VS1053, do you have the right pins defined?"));
            }

        while (1);
    }
    // 1st parameter is left, second parameter is right             
    musicPlayer.setVolume(20, 20);
    if (debugMode)
    {
        Serial.println(F("VS1053 found"));
    }

    // initialise the SD card
    SD.begin(CARDCS);    

    //ensure our global booleans all start false
    crossingSignalInOperation = false;  

}

void loop() {
    // put your main code here, to run repeatedly

    signalEntrySensor = checkEntrySensor();
    if (signalEntrySensor == true) {  //only want to catch the FIRST time it senses
        if (crossingSignalInOperation == false)
        {
            //crossing signal is inactive, activate it
            // turn the sound on and return the music player object
            onSignalSound(musicPlayer,"trainxng.mp3");  

            while (musicPlayer.playingMusic) {
                // file is now playing in the 'background' so now's a good time
                // to do something else 
                if (!crossingSignalInOperation)
                {                   
                    crossingSignalInOperation = true;
                    //run the other three processes
                    //offRoadPower();
                    //lowerSignalArms();
                    //startSignalFlashers();

                }               
            }                   
        }
    }
 }

void onSignalSound(Adafruit_VS1053_FilePlayer & mp, const char* fileName) {
    // Play the train crossing sound file
    if (debugMode)
    {
        Serial.println(F("Playing track - "));
        Serial.print(fileName);
    }
    //all sound files are on the micro sd card on the music player shield
    if (!mp.startPlayingFile(fileName)) {
        Serial.println(F("Could not open file "));
        Serial.print(fileName);
        while (1);
    }
}
boolean offSignalSound(Adafruit_VS1053_FilePlayer & mp) {
    if (mp.playingMusic)
    {
        mp.stopPlaying();
        return true;
    }
    return true;    
}

Untested, because I don't have the hardware, but that should work better.

The Adafruit_VS1053_FilePlayer object is passed by reference to the function, which avoids another copy being made. We are using the original one at global scope, so we don't keep creating and discarding copies. In fact, since we are using the global one you could avoid passing it to the function at all.