Loop freezes after using custom library function.

Hello again,

So, my project is basically adding onto the Arduino "Pitchs.h" library. My problem is that the Arduino will freeze after its done playing the sound. I have a piezo hooked up to pin 8 and a button and an LCD for testing the loop. Here are my codes:

//Sketch

#include <AdvancedSound.h>
AdvancedSound aSound(8);
#include <LiquidCrystal.h> //Import LiquidCrystal library for LCD functionality.
LiquidCrystal lcd(13, 7, 4, 2, 1, 0); //Define LCD pin layout.
int introMelody[] = { //Melody to be played on startup.
  NOTE_G5, NOTE_E5, NOTE_C5
};
int introTempo[] = { //Tempo for intro melody.
  8, 4, 4
};
void setup(){ //Begin setup.
  lcd.begin(16, 2); //Define LCD size as 16x2.
  lcd.print("Hello, world!"); //Print initial message. 
}

void loop(){ //Program loop.
  int yButton = analogRead(A0);//read button
  lcd.setCursor(0, 1);
  lcd.print(yButton);
  aSound.aPlay(introMelody, sizeof(introMelody), introTempo);//Play sound.
}
/*
  AdvancedSound.h - Library for advanced piezo sounds.
  Created by Christian S. Batchelor, December 14, 2014.
  Released into the public domain.
*/
#ifndef AdvancedSound_h
#define AdvancedSound_h
#include "Arduino.h"
class AdvancedSound{
  public:
    #define NOTE_B0  31
    #define NOTE_C1  33
    #define NOTE_CS1 35
    #define NOTE_D1  37
    #define NOTE_DS1 39
    #define NOTE_E1  41
    #define NOTE_F1  44
    #define NOTE_FS1 46
    #define NOTE_G1  49
    #define NOTE_GS1 52
    #define NOTE_A1  55
    #define NOTE_AS1 58
    #define NOTE_B1  62
    #define NOTE_C2  65
    #define NOTE_CS2 69
    #define NOTE_D2  73
    #define NOTE_DS2 78
    #define NOTE_E2  82
    #define NOTE_F2  87
    #define NOTE_FS2 93
    #define NOTE_G2  98
    #define NOTE_GS2 104
    #define NOTE_A2  110
    #define NOTE_AS2 117
    #define NOTE_B2  123
    #define NOTE_C3  131
    #define NOTE_CS3 139
    #define NOTE_D3  147
    #define NOTE_DS3 156
    #define NOTE_E3  165
    #define NOTE_F3  175
    #define NOTE_FS3 185
    #define NOTE_G3  196
    #define NOTE_GS3 208
    #define NOTE_A3  220
    #define NOTE_AS3 233
    #define NOTE_B3  247
    #define NOTE_C4  262
    #define NOTE_CS4 277
    #define NOTE_D4  294
    #define NOTE_DS4 311
    #define NOTE_E4  330
    #define NOTE_F4  349
    #define NOTE_FS4 370
    #define NOTE_G4  392
    #define NOTE_GS4 415
    #define NOTE_A4  440
    #define NOTE_AS4 466
    #define NOTE_B4  494
    #define NOTE_C5  523
    #define NOTE_CS5 554
    #define NOTE_D5  587
    #define NOTE_DS5 622
    #define NOTE_E5  659
    #define NOTE_F5  698
    #define NOTE_FS5 740
    #define NOTE_G5  784
    #define NOTE_GS5 831
    #define NOTE_A5  880
    #define NOTE_AS5 932
    #define NOTE_B5  988
    #define NOTE_C6  1047
    #define NOTE_CS6 1109
    #define NOTE_D6  1175
    #define NOTE_DS6 1245
    #define NOTE_E6  1319
    #define NOTE_F6  1397
    #define NOTE_FS6 1480
    #define NOTE_G6  1568
    #define NOTE_GS6 1661
    #define NOTE_A6  1760
    #define NOTE_AS6 1865
    #define NOTE_B6  1976
    #define NOTE_C7  2093
    #define NOTE_CS7 2217
    #define NOTE_D7  2349
    #define NOTE_DS7 2489
    #define NOTE_E7  2637
    #define NOTE_F7  2794
    #define NOTE_FS7 2960
    #define NOTE_G7  3136
    #define NOTE_GS7 3322
    #define NOTE_A7  3520
    #define NOTE_AS7 3729
    #define NOTE_B7  3951
    #define NOTE_C8  4186
    #define NOTE_CS8 4435
    #define NOTE_D8  4699
    #define NOTE_DS8 4978
    AdvancedSound(int piezoPin);
    void aPlay(int song[], int songSize, int tempo[]);
    void aBuzz(int targetPin, long frequency, long length);
  private:
    int _piezoPin;
};
#endif
#include "AdvancedSound.h"
AdvancedSound::AdvancedSound(int piezoPin){
  _piezoPin = piezoPin;
  tone(_piezoPin, 1, 1);
}
void AdvancedSound::aPlay(int song[], int songSize, int tempo[]){
  for (int thisNote = 0; thisNote < songSize; thisNote++) {
      int noteDuration = 1000/tempo[thisNote];
      aBuzz(_piezoPin, song[thisNote], noteDuration);
      int pauseBetweenNotes = noteDuration * 1.30;
      delay(pauseBetweenNotes);
      aBuzz(_piezoPin, 0, noteDuration);
  }
}
void AdvancedSound::aBuzz(int targetPin, long frequency, long length){
  long delayValue = 1000000/frequency/2;
  long numCycles = frequency * length/ 1000;
  for (long i=0; i < numCycles; i++){
    digitalWrite(targetPin,HIGH);
    delayMicroseconds(delayValue);
    digitalWrite(targetPin,LOW);
    delayMicroseconds(delayValue);
  }
}

for a commented form of the .cpp file look HERE

AdvancedSound::AdvancedSound(int piezoPin){
  _piezoPin = piezoPin;
  tone(_piezoPin, 1, 1);
}

Diddling with the hardware before the hardware is ready to be diddled with is a bad idea.

  long delayValue = 1000000/frequency/2;

You call the function that contains this code with a value of 0 for the frequency. Dividing by 0 is stupid.

What are the arguments for the LiquidCrystal constructor? Connecting the LCD to the hardware serial pins is not a good idea, if that is what you are doing. Reserve them for debugging.

PaulS:

AdvancedSound::AdvancedSound(int piezoPin){

_piezoPin = piezoPin;
  tone(_piezoPin, 1, 1);
}



Diddling with the hardware before the hardware is ready to be diddled with is a bad idea.



long delayValue = 1000000/frequency/2;



You call the function that contains this code with a value of 0 for the frequency. Dividing by 0 is stupid.

What are the arguments for the LiquidCrystal constructor? Connecting the LCD to the hardware serial pins is not a good idea, if that is what you are doing. Reserve them for debugging.

Thank you for replying, I have to use the tone function or no sound would play at all. If you could give me an alternative way to get this to work I would appreciate it.

As for dividing by 0 that part of the code was designed to stop the sound after the note was played.

And I needed to put the LCD pins on 1 and 0 because I will need the PWM pins for later in my project. I was told that using those pins would not be a problem if it was not connected to USB.

The code worked fine when it was just in the sketch, but now that i moved it to the library it seems to freeze the loop.

int introMelody[] = {NOTE_G5, NOTE_E5, NOTE_C5 };
int introTempo[] = {8, 4, 4 };

  aSound.aPlay(introMelody, sizeof(introMelody), introTempo);//Play sound.

The 'sizeof' operator return the size IN BYTES. Since your arrays are 6 bytes long you are telling .aPlay() to play six notes. I'm guessing one or more of the three notes you are playing beyond the end of your arrays has a very long duration and no playable tone.

To get the NUMBER OF ELEMENTS in an array:

sizeof introMelody / sizeof introMelody[0]

That gives you 6 bytes divided by 2 bytes per integer: 3 notes.

Note that 'sizeof' is an operator, not a function. It doesn't need parens around the argument.

johnwasser:

int introMelody[] = {NOTE_G5, NOTE_E5, NOTE_C5 };

int introTempo[] = {8, 4, 4 };

aSound.aPlay(introMelody, sizeof(introMelody), introTempo);//Play sound.




The 'sizeof' operator return the size IN BYTES. Since your arrays are 6 bytes long you are telling .aPlay() to play six notes. I'm guessing one or more of the three notes you are playing beyond the end of your arrays has a very long duration and no playable tone.

To get the NUMBER OF ELEMENTS in an array:



sizeof introMelody / sizeof introMelody[0]




That gives you 6 bytes divided by 2 bytes per integer: 3 notes.

Note that 'sizeof' is an operator, not a function. It doesn't need parens around the argument.

Thank you very much, the loop has stopped freezing. However another problem has now appeared, instead of reading and displaying the the buttons value it starts printing gibberish on the LCD... How do I fix this? It seems odd because my library doesn't have any code that should mess with the LCD.

BatcheloShop:
Thank you very much, the loop has stopped freezing. However another problem has now appeared, instead of reading and displaying the the buttons value it starts printing gibberish on the LCD... How do I fix this? It seems odd because my library doesn't have any code that should mess with the LCD.

What gibberish? Remember that your code doesn't erase the LCD so if the analog value goes from 1023 to 57 the display will show 5723 when you write 57 over the first part of 1023.

Here is what my LCD does if aPlay() is called:

Here is what it does if the sketch never calls aPlay():

image1.JPG

image2.JPG

image3.JPG

I think my library and the LiquidCrystal are somehow interfering with each other, beacuse if I use "lcd.begin(16, 2);" AFTER I use the "aPlay();" function it will display correctly.

How do you think I should fix this?

Sorry if the images don't show up...

Next time attach the images to your post rather than using an off-site server.

[quote author=Coding Badly link=msg=2003253 date=1418701776]
Next time attach the images to your post rather than using an off-site server.[/quote]

Sorry, wasnt aware I could do that.

Anyway, I was able to fix the problem by doing this in the .cpp file.

AdvancedSound::AdvancedSound(int piezoPin){
  _piezoPin = piezoPin;
  pinMode(_piezoPin, OUTPUT);
}

I wasn't aware I had to set a pin as an output or input if it was in a library file. But apparently I do! (the tutorial I used didn't instruct me to do so).

Anyway, thank you everyone who helped!

Anyway, I was able to fix the problem by doing this in the .cpp file.

You're still doing it wrong. See reply #1, and think about why Serial.begin() is necessary.

PaulS:
You're still doing it wrong. See reply #1, and think about why Serial.begin() is necessary.

I assume you mean I should use a debugger to find whats causing the screen issue?

I assume you mean I should use a debugger to find whats causing the screen issue?

No. I mean that you need to stop trying to diddle with the hardware (calling pinMode) before the hardware is ready. Diddling with the hardware in a constructor is a waste of time and effort.

If you need to diddle with the hardware, you need a begin() method - like the HardwareSerial class has.

PaulS:
No. I mean that you need to stop trying to diddle with the hardware (calling pinMode) before the hardware is ready. Diddling with the hardware in a constructor is a waste of time and effort.

If you need to diddle with the hardware, you need a begin() method - like the HardwareSerial class has.

Okay, I made a begin() function to put pinMode() and and tone(). The only problem is that the screen starts printing gibberish if I use lcd.begin() before I use aSound.begin().

The simple fix is to use aSound.begin() before lcd.begin(), but I don't understand why it matters which one I use first, any ideas?

The simple fix is to use aSound.begin() before lcd.begin(), but I don't understand why it matters which one I use first, any ideas?

Without seeing the changes you've made to your code? No.