0

Context

This may be more of a C++ question than an arduino specific question, but it's dealing with arduino c++ so I thought I'd start here.

I'm building a class to abstract some simple melody playing that I want for a project. I got everything working in a procedural way, so I started moving everything out to a header and cpp file (still new-ish territory for me).

The general concept is that I want to call a public method on my class to allow my code to play a specific melody. I defined an enum for each melody available and in my playMelody() method I'm using a couple of pointers and a switch statement to get the specific frequencies and durations I need to play the specific melody. These frequency and duration values are stored in private arrays in the class.

Here's the header:

// MelodyPlayer.h

enum MELODY
{
    SHAVE_AND_A_HAIRCUT = 0,
    IDLE,
    READY,
    PROCESSING,
    COMPLETE,
};

class MelodyPlayer
{
    uint8_t speakerPin;

    int melody_shaveAndAHairCut[9];
    int durations_shaveAndAHairCut[9];
    int *activeMelody;
    int *activeDurations;

    void _playMelody();

public:
    MelodyPlayer(uint8_t speakerPin);
    void playMelody(MELODY name);
};

MelodyPlayer::MelodyPlayer(uint8_t pin)
{
    speakerPin = pin;

    int melody_shaveAndAHairCut[] = {
        NOTE_C4, NOTE_G3, NOTE_G3, NOTE_A3, NOTE_G3, 0, NOTE_B3, NOTE_C4, 0};

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

void MelodyPlayer::playMelody(MELODY name)
{
    switch (name)
    {
    case SHAVE_AND_A_HAIRCUT:
        Serial.println("Shave and a hair cut");
        activeMelody = melody_shaveAndAHairCut;
        activeDurations = durations_shaveAndAHairCut;
        break;
    }

    _playMelody();
}

And here's the implementation:

// MelodyPlayer.cpp

void MelodyPlayer::_playMelody()
{
    Serial.println("Playing Melody");
    // * pulled from the toneMelody arduino example
    for (int thisNote = 0; thisNote < 8; thisNote++)
    {

        int activeDuration = activeDurations[thisNote];
        int noteDuration = 1000 / activeDurations[thisNote];
        int noteFrequency = activeMelody[thisNote];

        Serial.println(thisNote);
        Serial.println(activeDuration);
        Serial.println(noteDuration);
        Serial.println(noteFrequency);

        tone(speakerPin, noteFrequency, noteDuration);

        int pauseBetweenNotes = noteDuration * 1.30;
        delay(pauseBetweenNotes);

        noTone(speakerPin);
    }
}

And I call it in my .ino file like this:

MelodyPlayer player = MelodyPlayer(SPEAKER);
player.playMelody(SHAVE_AND_A_HAIR_CUT);

The problem

I'll upload the sketch and everything fires. The problem I'm running into is that when I get to the private _playMelody() method the values I get for activeDuration, noteDuration and noteFrequency are all ultra wrong:

example output

I imagine I'm doing something wrong with the setting and getting of the values via the pointer variable I set in the switch statement, but I'm not sure where I'm going wrong.

You can see/pull down the actual code from the abstracting-to-class branch of my repo if you want to dig in.

I've done a similar dynamic-selection-via-pointer in a proof of concept before, but it was procedurally written, not classed. I can't seem to figure out where the difference is :|

Any ideas?

1
  • Your melody and duration arrays that contain the actual data are local to the constructor. Further to that, they aren't used, so the compiler will optimize them away.
    – Majenko
    Jun 14, 2019 at 14:13

1 Answer 1

3

As Majenko mentioned in the comment, you created new local arrays in the constructor, but never put anything in the member arrays of the classes. They have the same name, but are not the same variable. Responsible for creating the new variable is the keyword int. However, you cannot simply assign array values by just omitting the int. Therer are multiple possibilities:

  • You can fill the array for each index: durations_shaveAndAHairCut[0] = 1;

  • You can copy your local array to the member variable with memcpy: memcpy((void*)this->durations_shaveAndAHairCut, (void*)durations_shaveAndAHairCut, 9*sizeof(int));

  • You can declare the arrays static in the class declaration and initialize it outside of the constructor in MelodyPlayer.cpp:

    int MelodyPlayer::durations_shaveAndAHairCut[] = {
        4, 8, 8, 4, 4, 4, 4, 4, 4};
    

    I prefer the last option.

1
  • Awesome! Thanks for the help! I went with the last option. It definitely feels like the most simple and straight forward solution. Jun 14, 2019 at 17:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.