0

I am a college student in an intro to engineering class looking to create a final project. My goal is to create a small "present" that opens its lid via a servo and plays jingle bells with LEDs flashing (as shown in the youtube video linked below), before closing the lid once again until a further button press.
https://www.youtube.com/watch?v=V6TcAhXCL6w

The code that this video uses is:

/*
  "Jingle Bells" Melody

 Plays "Jingle Bells"

 circuit:
 * 8-ohm speaker on digital pin 7

#define NOTE_C4  262
#define NOTE_D4  294
#define NOTE_E4  330
#define NOTE_FS4 370
#define NOTE_G4  392
#define NOTE_A4  440
#define NOTE_B4  494
#define NOTE_C5  523
#define NOTE_D5  587
#define NOTE_E5  659

// notes in the melody:
int melody[] = {
  NOTE_D4, NOTE_B4, NOTE_A4, NOTE_G4, NOTE_D4, NOTE_D4, NOTE_D4,
  NOTE_D4, NOTE_B4, NOTE_A4, NOTE_G4, NOTE_E4,
  NOTE_E4, NOTE_C5, NOTE_B4, NOTE_A4, NOTE_FS4,
  NOTE_D5, NOTE_D5, NOTE_C5, NOTE_A4, NOTE_B4, NOTE_D4,
  NOTE_D4, NOTE_B4, NOTE_A4, NOTE_G4, NOTE_D4, NOTE_D4, NOTE_D4,
  NOTE_D4, NOTE_B4, NOTE_A4, NOTE_G4, NOTE_E4, NOTE_E4,
  NOTE_E4, NOTE_C5, NOTE_B4, NOTE_A4, NOTE_D5, NOTE_D5, NOTE_D5, NOTE_D5,
  NOTE_E5, NOTE_D5, NOTE_C5, NOTE_A4, NOTE_G4,
  NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4,
  NOTE_B4, NOTE_D5, NOTE_G4, NOTE_A4, NOTE_B4,
  NOTE_C5, NOTE_C5, NOTE_C5, NOTE_C5, NOTE_C5, NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4,
  NOTE_B4, NOTE_A4, NOTE_A4, NOTE_B4, NOTE_A4, NOTE_D5,
  NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4,
  NOTE_B4, NOTE_D5, NOTE_G4, NOTE_A4, NOTE_B4,
  NOTE_C5, NOTE_C5, NOTE_C5, NOTE_C5, NOTE_C5, NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4,
  NOTE_D5, NOTE_D5, NOTE_C5, NOTE_A4, NOTE_G4 };

// note durations: 4 = quarter note, 8 = eighth note, etc.:
int noteDurations[] = {

  8, 8, 8, 8, 3, 16, 16,
  8, 8, 8, 8, 2,
  8, 8, 8, 8, 2, 
  8, 8, 8, 8, 3, 8,
  8, 8, 8, 8, 3, 16, 16,
  8, 8, 8, 8, 3, 8,
  8, 8, 8, 8, 8, 8, 8, 8,
  8, 8, 8, 8, 2,
  8, 8, 4, 8, 8, 4,
  8, 8, 6, 16, 2,
  8, 8, 6, 16, 8, 8, 8, 16, 16,
  8, 8, 8, 8, 4, 4,
  8, 8, 4, 8, 8, 4, 8, 8, 6, 16, 2,
  8, 8, 6, 16, 8, 8, 8, 16, 16,
  8, 8, 8, 8, 2 };

void setup() {
    pinMode(3,OUTPUT);
    pinMode(6,OUTPUT);
    pinMode(9,OUTPUT);
    pinMode(11,OUTPUT);
    digitalWrite(3, LOW);
    digitalWrite(6, LOW);
    digitalWrite(9, LOW);
    digitalWrite(11, LOW);
  // iterate over the notes of the melody:
  for (int thisNote = 0; thisNote < 100; thisNote++) {

    // to calculate the note duration, take one second 
    // divided by the note type.
    //e.g. quarter note = 1000 / 4, eighth note = 1000/8, etc.
    int noteDuration = 1800/noteDurations[thisNote];
    // I found that by lengthening the noteDuration,
    // I was able to "slow" the song down.
    tone(7, melody[thisNote],noteDuration);
    LEDon();

    // to distinguish the notes, set a minimum time between them.
    // the note's duration + 30% seems to work well:
    int pauseBetweenNotes = noteDuration * 1.30;
    delay(pauseBetweenNotes);
    // stop the tone playing:
    noTone(7);
    LEDoff();
  }
}

void loop() {
  // no need to repeat the melody.
}

int currentLEDpin=3;
void LEDon()
{
    digitalWrite(currentLEDpin, HIGH);
}

void LEDoff()
{
    digitalWrite(currentLEDpin, LOW);
    if (currentLEDpin==3) currentLEDpin=6;
    else if (currentLEDpin==6) currentLEDpin=9;
    else if (currentLEDpin==9) currentLEDpin=11;
    else currentLEDpin=3;
}

My general outline for the project looks like this:

  1. Set LEDs, Piezo, and Servo to OUTPUT
  2. Set pushbutton to INPUT
  3. Start all LEDs off/LOW
  4. Start a loop that has if statement for button push
  5. Open the lid (servo motor about 100 degrees)
  6. Start the song/LED sequence using code borrowed from the youtube video
  7. After finishing the song, close the lid (servo motor back 100 degrees)

I am very much a beginner to coding Arduino, so I was wondering if my uploaded sketch seems correct...

//Notes for Jingle Bells
#define NOTE_C4  262
#define NOTE_D4  294
#define NOTE_E4  330
#define NOTE_FS4 370
#define NOTE_G4  392
#define NOTE_A4  440
#define NOTE_B4  494
#define NOTE_C5  523
#define NOTE_D5  587
#define NOTE_E5  659

//Notes in melody Command
int melody[] = {
  NOTE_D4, NOTE_B4, NOTE_A4, NOTE_G4, NOTE_D4, NOTE_D4, NOTE_D4,
  NOTE_D4, NOTE_B4, NOTE_A4, NOTE_G4, NOTE_E4,
  NOTE_E4, NOTE_C5, NOTE_B4, NOTE_A4, NOTE_FS4,
  NOTE_D5, NOTE_D5, NOTE_C5, NOTE_A4, NOTE_B4, NOTE_D4,
  NOTE_D4, NOTE_B4, NOTE_A4, NOTE_G4, NOTE_D4, NOTE_D4, NOTE_D4,
  NOTE_D4, NOTE_B4, NOTE_A4, NOTE_G4, NOTE_E4, NOTE_E4,
  NOTE_E4, NOTE_C5, NOTE_B4, NOTE_A4, NOTE_D5, NOTE_D5, NOTE_D5, NOTE_D5,
  NOTE_E5, NOTE_D5, NOTE_C5, NOTE_A4, NOTE_G4,
  NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4,
  NOTE_B4, NOTE_D5, NOTE_G4, NOTE_A4, NOTE_B4,
  NOTE_C5, NOTE_C5, NOTE_C5, NOTE_C5, NOTE_C5, NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4,
  NOTE_B4, NOTE_A4, NOTE_A4, NOTE_B4, NOTE_A4, NOTE_D5,
  NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4,
  NOTE_B4, NOTE_D5, NOTE_G4, NOTE_A4, NOTE_B4,
  NOTE_C5, NOTE_C5, NOTE_C5, NOTE_C5, NOTE_C5, NOTE_B4, NOTE_B4, NOTE_B4, NOTE_B4,
  NOTE_D5, NOTE_D5, NOTE_C5, NOTE_A4, NOTE_G4 };

// Note Durations (4 = quarter note, 8 = eighth note, etc.)
int noteDurations[] = {
  8, 8, 8, 8, 3, 16, 16,
  8, 8, 8, 8, 2,
  8, 8, 8, 8, 2, 
  8, 8, 8, 8, 3, 8,
  8, 8, 8, 8, 3, 16, 16,
  8, 8, 8, 8, 3, 8,
  8, 8, 8, 8, 8, 8, 8, 8,
  8, 8, 8, 8, 2,
  8, 8, 4, 8, 8, 4,
  8, 8, 6, 16, 2,
  8, 8, 6, 16, 8, 8, 8, 16, 16,
  8, 8, 8, 8, 4, 4,
  8, 8, 4, 8, 8, 4, 8, 8, 6, 16, 2,
  8, 8, 6, 16, 8, 8, 8, 16, 16,
  8, 8, 8, 8, 2 };


#include <Servo.h>      //Include built-in Servo sketch
Servo servo;            //Servo sketch reference for setup
const int servoPin=5;   //Set servoPin to 5
int angle = 0;          //Set initial servo angle to 0 degrees

const int buttonPin=2;   //Set buttonPin to 2
int buttonState=0;       //variable for reading pin status

int currentLEDpin=3;    //Set LED pin variable for LED flashes
void LEDon()            //Define LEDon for turning on LED to music
{
    digitalWrite(currentLEDpin, HIGH);
}

void LEDoff()           //Define LEDoff for turning off LED when music off
{
    digitalWrite(currentLEDpin, LOW);
    if (currentLEDpin==3) currentLEDpin=6;
    else if (currentLEDpin==6) currentLEDpin=9;
    else if (currentLEDpin==9) currentLEDpin=11;
    else currentLEDpin=3;
}


void setup() {
    pinMode(3,OUTPUT);           //Set LED Pins to OUTPUTs
    pinMode(6,OUTPUT);
    pinMode(9,OUTPUT);
    pinMode(11,OUTPUT);
    digitalWrite(3, LOW);        //Start all LED lights off
    digitalWrite(6, LOW);
    digitalWrite(9, LOW);
    digitalWrite(11, LOW);

    pinMode(buttonPin,INPUT);   //Set button to INPUT

    servo.attach(servoPin);     //Servo setup

}

void loop() {
 buttonState=digitalRead(buttonPin);     //read input value 

  if (buttonState == HIGH) {              //check if pushbutton is pressed

  for(angle = 0; angle <110; angle++)    //rotate servo 110 degrees, opening present box lid
    {                                  
    servo.write(angle);               
    delay(15);                   
    } 

  for (int thisNote = 0; thisNote < 100; thisNote++) {
    int noteDuration = 1800/noteDurations[thisNote];    //define note duration
    tone(7, melody[thisNote],noteDuration);             //Play melody of Jingle Bells
    LEDon();                                            //Flash LEDs to tune
    int pauseBetweenNotes = noteDuration * 1.30;        //Pause between notes to annunciate each note
    delay(pauseBetweenNotes);
    noTone(7);                                          //Stop playing music when melody over
    LEDoff();                                           //Stop flashing LEDs when melody over
}

  for(angle = 110; angle > 0; angle--)                  //Close present box lid
    {                                
    servo.write(angle);           
    delay(15);       
    } 
   }

}

So when you say to erase the LEDon and LEDoff calls, is that in this section of the code?:

 for (int thisNote = 0; thisNote < 100; thisNote++) {
    int noteDuration = 1800/noteDurations[thisNote];    //define note duration
    tone(7, melody[thisNote],noteDuration);             //Play melody of Jingle Bells
    LEDon();                                            //Flash LEDs to tune
    int pauseBetweenNotes = noteDuration * 1.30;        //Pause between notes to annunciate each note
    delay(pauseBetweenNotes);
    noTone(7);                                          //Stop playing music when melody over
    LEDoff();                                           //Stop flashing LEDs when melody over
}
  • I misread your question. You want the leds, so keep the calls and copy the functions from Jingle Bells sketch to your own. – user31481 Nov 16 '17 at 16:26
  • 1
    There doesn't seem to be a specific question here. This is not a code review site so my uploaded sketch seem correct is not a valid question - you must ask about a specific problem, not for general improvement suggestions. – Chris Stratton Nov 16 '17 at 18:25
  • Half of the questions posted here are code-review ... – user31481 Nov 16 '17 at 18:46
1

LEDon and LEDoff are defined in the Jingle Bell sketch. They just lit on/of the leds.

You don't need them for your project, just erase the calls. Otherwise, copy the LEDon/off functions:

int currentLEDpin=3;
void LEDon()
{
    digitalWrite(currentLEDpin, HIGH);
}

void LEDoff()
{
    digitalWrite(currentLEDpin, LOW);
    if (currentLEDpin==3) currentLEDpin=6;
    else if (currentLEDpin==6) currentLEDpin=9;
    else if (currentLEDpin==9) currentLEDpin=11;
    else currentLEDpin=3;
}

You made a mistake:

if(buttonState = HIGH) {               //check if pushbutton is pressed

Here you assign HIGH to buttonState.

The "=" is assignment operator. What you were thinking is "==", the equality-comparison operator (a common mistake). Replace it with

if(buttonState == HIGH) {               //check if pushbutton is pressed

Also, you need to debounce you button. Read about it in this excelent tutorial by Nick Gammon: Switches tutorial. Well, read his whole blog if you have the time.

Otherwise, you are doing well.

  • Thank you so much for your help! If you could clarify on where I should copy the LEDon and LEDoff to that would be great! Also, where in the sketch should I erase the calls? – user39870 Nov 16 '17 at 16:23
  • Copy the functions, keep the calls. I misreaded your question. – user31481 Nov 16 '17 at 16:27