Sketch: am I getting it?

[Utahfireman]

Hello Arduinites,
I am fairly new to writing sketches for an Arduino but as I was working on a sketch, I think things started to make since but I wanted others opinions before I patted myself on the back. I am running a Duemilanove to drive a four relay module which will operate a linear actuator, led light, fog machine and play audio via a YX5300 player as the linear actuator gets 50% extended. In my mind, the light will turn on and then a burst of fog will happen, at that point the actuator will extend and while it is in its process, an audio file will play. Once extended, it will pause shortly and then retract after which the light will turn off. This will be triggered by a PIR. The process will continue as people walk by the sensor. My biggest unknown is to only play the mp3 file once each time the sequence is run.. do I take it out of the loop statement?

Here is my sketch: [code]

type or paste PIR to trigger again:
 Simple SerialMP3Player "zombie moan" example of YX5300 chip.

  Copy the "zombie.mp3" file to an empty SD card
  Connect the Serial MP3 Player to the Arduino board
    GND → GND
    VCC → 5V
    TX → pin 11
    RX → pin 10

  After compile and upload the code you must hear “zombie moan”.



  by Steve Larsen
 *******************************************************************************/
// digital pin 2 has a PIR attached to it.
int pir = 2;
// digital pin 5 has a fog trigger attached to it.
int fog = 5;
// digital pin 6 has led attached to it.
int led = 6;
// digital pin 7 will retract a linear actuator.
int retract = 7;
// digital pin 8 will extend a linear actuator.
int extend = 8;

#include "SerialMP3Player.h"

#define TX 11
#define RX 10

SerialMP3Player mp3(RX, TX);

void setup() {
  // make the pir an input:
  pinMode (pir, INPUT);
  // make the fourth relay an output to fog machine:
  pinMode (fog, OUTPUT);
  // make the third relay an output to led light:
  pinMode (led, OUTPUT);
  // make the second relay an output to retract actuator:
  pinMode (retract, OUTPUT);
  // make the first relay an output to extent actuator:
  pinMode (extend, OUTPUT);
  //initialize pin 7 as high:
  digitalWrite (retract, HIGH);
  //initialize pin 8 as high:
  digitalWrite (extend, HIGH);

  Serial.begin(9600);     // start serial interface
  mp3.begin(9600);        // start mp3-communication
  delay(500);             // wait for init

  mp3.sendCommand(CMD_SEL_DEV, 0, 2);   //select sd-card
  delay(500);             // wait for init
  mp3.play();     // Play "zombie.mp3". You must hear "zombie moan"
}

// the loop function runs over and over again forever
void loop() {
  // read the input pin;
  int pirState = digitalRead(pir);
  // print out the state of the pir;
  Serial.println(pirState);
  delay(1); //delay is between reads for stability

  digitalWrite (led, HIGH);
  delay (1000);
  digitalWrite (fog, HIGH);
  delay (2000);
  digitalWrite (fog, LOW);
  delay (500);
  digitalWrite (extend, HIGH);
  digitalWrite (retract, LOW);
  delay (4000);
  mp3.play();     // Play "zombie.mp3". You must hear "Moan"
  delay (5000);
  digitalWrite (extend, LOW);
  digitalWrite (retract, HIGH);


  if (digitalRead (2) == HIGH and digitalRead (3) == LOW) {
    //extend linear actuator:
    digitalWrite (extend, HIGH);
    digitalWrite (retract, LOW);
  }
  else if (digitalRead (2) == LOW and digitalRead (3) == HIGH) {
    //retract the linear actuator:
    digitalWrite (extend, LOW);
    digitalWrite (retract, HIGH);
  }
  else {
    //stops Actuator:
    digitalWrite (extend, HIGH);
    digitalWrite (retract, HIGH);
  }
  digitalWrite (led, LOW);
  // end program and wait for 

} code here

[jfjlaros]

Can you edit your post and use code tags to format the code?

[taschi]

You should test your code, instead of asking others to check it. Verifying code with actually running it is almost impossible.

[PerryBebbington]

What you have described is best dealt with using a thing called a finite state machine. There are tutorials and examples on FSMs, here are some of them:

Using millis for timing
Demonstration for several things at the same time

Timer counter state change toggle FSM - all at once

There is a recurring theme of people new to Arduino and/or microcontrollers not grasping the concept of putting what I call ‘building blocks’ of code together to achieve some goal. These short sketches are an attempt to offer some examples showing how small pieces can be used in concert to build up larger, more complex sketches. Thanks to @PerryBebbington and @ballscrewbob for support and input. You made it better. The examples will illustrate: • Doing ‘several things at once’ • pitfalls of …

As to taking things out of the loop() function, yes, take everything out. Ideally split your code into separate functions that each does a defined thing, then the only thing in your loop function should be calls to the individual functions. If you do that then the code is a lot easier to read and debug.

As to the words 'finite state machine', which might have you a little worried, consider a restaurant:

You wouldn’t expect this when you go into a restaurant:
A waiter meets you at the door, takes you to a table, gives you a menu then waits by your table while you decide what to order. The waiter takes your order, goes to the kitchen and waits there while the chef cooks your food. However, as the staff in this restaurant only ever deal with one customer at a time your waiter has to wait with other waiters while the chef cooks 5 other meals before starting on yours. When the food is eventually ready the waiter brings it to your table then waits by your table while you eat it. When you’ve finished eating the waiter takes your plates away and returns to ask if you want anything else. This continues until you leave. No one else gets served.
I’m not going to describe what really happens in a restaurant as you already know. A waiter uses exactly the same system as a state machine to serve people when they need serving and check to see who needs serving next between dealing with customers. You can build functions for the different tasks a waiter does such as:

void takeOrder();
void bringFoodToTable();

You call these from loop(); While in loop the waiter checks to see if any tables need attention, and if they do s/he goes to find out what they need. If not, then s/he keeps checking until someone needs something. Computer code should be written along the same principals.

A restaurant, indeed many ordinary things in life, operate as a finite state machine, it's just no one calles it that. Finite state machine in normal life is called 'multi-tasking', and you will be familiar with the debates over whether men or women are best at it. Well, regardless of the debate that's what you are aiming for, just on a different scale.

Good luck with your project.

[kolaha]

@Utahfireman

quad relay

show which one

[Koepel]

The Wokwi simulation does not simulate the MP3 module and can probably not deal with both the Serial Monitor and the MP3 module to the same serial output, but when I made it in Wokwi I noticed that I don't know what is connected to pin 3.
The structure of the code is missing. A Finite State Machine will solve it, but that might be too much over the top for now.

Can you describe in words (without thinking in code) what should happen. If you do that carefully, then the sketch is almost written.

[lastchancename]

You seem to be heading in the right direction for a self-stated beginner.

As @PerryBebbington suggested, a state machine (think of it as a sequencer) is where you want to go.

One thing you’ll find on that journey is the need to lose the delay() statements after setup().

Delay()s mean the processor has control of your event timing, not you ! Read up on millis() !

i’m sure that where this is going, you’ll get what you’re looking for pretty soon.

Hold your breath, and remember that everything inside loop() does exactly that…

[StefanL38]

You should re-edit your initial post using the method described here

Make experienced users help: two ways to quickly post code as a code-section

Hi newcomer, This thread wants to show you how you can speed-up getting helpful answers with small effort. If you manage at least to post this way. You have the sympathy of the experienced users. If you are willing to invest another 20 minutes these 20 minutes extra-time will save you a minimum of 200 minutes to finish your project. always post your full sketch. As a code-section from the very first to the very last line A code-section has a small height => keeps your posting "short" but…

[StefanL38]

You should use the IO-pin-constants everywhere in your code
instead of coding

digitalRead (2) == HIGH and digitalRead (3) == LOW) {

use the constants that you defined

digitalRead (pir) == HIGH and digitalRead (?????????) == LOW) {

You haven't explained what IO-pin 3 is for
How shall somebody else understand your code?

what does

digitalRead (pir) == HIGH

mean?

PIR is triggered or untrigered?

You should use self-explaining constants for that too

most relay-boards are LOW-active which means with input beeing LOW the contact is closed
From your code I assume it is opposite

  digitalWrite (led, HIGH);
  delay (1000);
  digitalWrite (fog, HIGH);
  delay (2000);
  digitalWrite (fog, LOW);
  delay (500);
  digitalWrite (extend, HIGH);
  digitalWrite (retract, LOW);

if you have defined constants lke this

const byte activated = HIGH;
const byte switchedOff= LOW;

The code becomes much easier to read and understand

  digitalWrite (led, activated);
  delay (1000);
  digitalWrite (fog, activated);
  delay (2000);
  digitalWrite (fog, switchedOff);
  delay (500);
  digitalWrite (extend, activated);
  digitalWrite (retract, switchedOff);

Your code does not yet beeing conditional on the PIR-state
you read in the PIR-state but there is no if-condition that makes executing the code below conditional to the PIR beeing triggered

best regards Stefan

[Utahfireman]

Thanks for your help

[Utahfireman]

Thanks, I am always looking to learn more. I hope that I edited my post correctly.

[Utahfireman]

thanks for the assistance, I just learned how int statements help and you mentioning the change with constants and that makes it much easier as well.

[StefanL38]

Yes this looks better now. OK back to your code:
playing the MP3-file only once per PIR-trigger
Imagine a a forgetful old man which needs to take a pill every morning.
This could be done with a pill box with a compartment for every day.

So in the morning the man looks at the pill box to see if the pill for today is in the compartment
If yes take it. If half an hour later he is asking himself "oops ! did I take the pill for today?" he simply walks to the books looking into it. Saturday is empty OK I have already taken the pill.
On sunday evening if all pills are taken out he fills it up again for the next week.

You can do something similar with code. Setting and / Re-Setting a flag-variable that indicates if an action is already done or not.

The variable-names should always be self-explaining. For your MP3-file-playing this could be something like
boolean MP3_File_not_YetPlayed;

In the code this would look like this

if (MP3_File_not_YetPlayed == true) { // REALLY self-explaining name

  MP3_File_not_YetPlayed = false; // as the file will be played set variable to false
  mp3.sendCommand(CMD_SEL_DEV, 0, 2); //select sd-card
  delay(500); // wait for init
  mp3.play(); // Play "zombie.mp3". You must hear "zombie moan"
}

or a bit shorter

if (MP3_File_not_YetPlayed) { // REALLY self-explaining name

  MP3_File_not_YetPlayed = false; // as the file will be played set variable to false
  mp3.sendCommand(CMD_SEL_DEV, 0, 2); //select sd-card
  delay(500); // wait for init
  mp3.play(); // Play "zombie.mp3". You must hear "zombie moan"
}

When the PIR gets triggered new than the flag-variable with name MP3_File_not_YetPlayed is set to false again

void loop() {
  // read the input pin;
  int pirState = digitalRead(pir);

  if (pirState == triggered) {
    MP3_File_not_YetPlayed = false; // not yet played so set to false
    // print out the state of the pir;
    Serial.println(pirState);
    delay(1); //delay is between reads for stability

    digitalWrite (led, HIGH);
    delay (1000);
    digitalWrite (fog, HIGH);
    delay (2000);
    digitalWrite (fog, LOW);

In this code the flag-variable is redundant because with the if-condition the playing of the MP3-file will happen only after PIR has been triggered and the looong delays will last longer as the PIR-pulse beeing high.

Using delay() in this case is sufficient but sooner or later you will come across situations where you need non-blocking code which is written very differently

best regards Stefan

[Utahfireman]

thanks, would there be an int statement to describe the "mp3_file_not_YetPlayed" or what do I need to declare it? Should there also be a "else if" statement?

[StefanL38]

@Utahfireman

would there be an int statement to describe the "mp3_file_not_YetPlayed" or what do I need to declare it? Should there also be a "else if" statement?

No.
As this variable is used with only two values you declare a variable of type boolean

boolean MP3_File_not_YetPlayed;

The variable-type boolean allows for very short if-conditions

if (MP3_File_not_YetPlayed) { // REALLY self-explaining name

No else if needed.
What are the reasons that you asked

@Utahfireman

Should there also be a "else if" statement?

best regards Stefan

[Utahfireman]

I wasn't sure if the true / false statement needed to return to particular state, I am probably over thinking it. I need to learn more about the boolean statement

[StefanL38]

Take a look into this tutorial:

Arduino Programming Course

It is easy to understand and has a good mixture between explaining important concepts and example-codes to get you going. So give it a try and report your opinion about this tutorial.

best regards Stefan

[Utahfireman]

thanks, will do