Refactored #75

[johnwargo]

Updated #75 based on feedback from @ladyada. I couldn't figure out how to make a PR on the existing PR (although I didn't spend a lot of time on it) so this is a new PR that refactors the original PR with requested changes and more.

Note: Please close the other PR when you merge this one.

Changes

• Moved the definitions for currentTrack and playingMusic back to their original locations in the header file. I can't think of a good reason to have moved them (sorry).
• Moved definition of _loopPlayback from Adafruit_VS1053.h to Adafruit_VS1053.cpp as its an internal variable for the library and didn't need to be exposed through the header file (AFAIK).
• Deleted enablePlaybackLooping and disablePlaybackLooping and replaced them with a single function playbackLoop(boolean loopState) - pass true to enable playback looping, false to disable it.
• Added Boolean function playbackLooped() that returns the current _loopPlayback variable. This way a developer can set it and read it; a requirement for any complete API.

Limitations

With playback looping enabled if a sketch plays a sound file using playFullFile, the sketch blocks, no other code executes because playFullFile blocks the thread and with playback looping enabled it never stops playing. I thought about putting in a check to skip looping on this condition, but i thought that this was something somebody may actually want to do. I can't see why, but developers have done stranger things. @ladyada please let me know if you want this limitation removed.

Test Code

I tested the changes using the following code:

// Specifically for use with the Adafruit Feather, the pins are pre-set here!

// #include <Adafruit_VS1053.h>
#include "Adafruit_VS1053.h"

// include SPI, MP3 and SD libraries
#include <SPI.h>
#include <SD.h>

// These are the pins used
#define VS1053_RESET -1  // VS1053 reset pin (not used!)

// Feather ESP8266
#if defined(ESP8266)
#define VS1053_CS 16   // VS1053 chip select pin (output)
#define VS1053_DCS 15  // VS1053 Data/command select pin (output)
#define CARDCS 2       // Card chip select pin
#define VS1053_DREQ 0  // VS1053 Data request, ideally an Interrupt pin

// Feather ESP32
#elif defined(ESP32) && !defined(ARDUINO_ADAFRUIT_FEATHER_ESP32S2)
#define VS1053_CS 32    // VS1053 chip select pin (output)
#define VS1053_DCS 33   // VS1053 Data/command select pin (output)
#define CARDCS 14       // Card chip select pin
#define VS1053_DREQ 15  // VS1053 Data request, ideally an Interrupt pin

// Feather Teensy3
#elif defined(TEENSYDUINO)
#define VS1053_CS 3    // VS1053 chip select pin (output)
#define VS1053_DCS 10  // VS1053 Data/command select pin (output)
#define CARDCS 8       // Card chip select pin
#define VS1053_DREQ 4  // VS1053 Data request, ideally an Interrupt pin

// WICED feather
#elif defined(ARDUINO_STM32_FEATHER)
#define VS1053_CS PC7     // VS1053 chip select pin (output)
#define VS1053_DCS PB4    // VS1053 Data/command select pin (output)
#define CARDCS PC5        // Card chip select pin
#define VS1053_DREQ PA15  // VS1053 Data request, ideally an Interrupt pin

#elif defined(ARDUINO_NRF52832_FEATHER)
#define VS1053_CS 30    // VS1053 chip select pin (output)
#define VS1053_DCS 11   // VS1053 Data/command select pin (output)
#define CARDCS 27       // Card chip select pin
#define VS1053_DREQ 31  // VS1053 Data request, ideally an Interrupt pin

// Feather RP2040
#elif defined(ARDUINO_ADAFRUIT_FEATHER_RP2040)
#define VS1053_CS 8    // VS1053 chip select pin (output)
#define VS1053_DCS 10  // VS1053 Data/command select pin (output)
#define CARDCS 7       // Card chip select pin
#define VS1053_DREQ 9  // VS1053 Data request, ideally an Interrupt pin

// Feather M4, M0, 328, ESP32S2, nRF52840 or 32u4
#else
#define VS1053_CS 6    // VS1053 chip select pin (output)
#define VS1053_DCS 10  // VS1053 Data/command select pin (output)
#define CARDCS 5       // Card chip select pin
// DREQ should be an Int pin *if possible* (not possible on 32u4)
#define VS1053_DREQ 9  // VS1053 Data request, ideally an Interrupt pin

#endif

Adafruit_VS1053_FilePlayer musicPlayer =
  Adafruit_VS1053_FilePlayer(VS1053_RESET, VS1053_CS, VS1053_DCS, VS1053_DREQ, CARDCS);

// sample file is three seconds long
const char* sampleFile = "/sample.mp3";

void setup() {
  Serial.begin(115200);
  delay(5000);
  // if you're using Bluefruit or LoRa/RFM Feather, disable the radio module
  //pinMode(8, INPUT_PULLUP);

  Serial.println("\n\nAdafruit VS1053 Feather Test");

  if (!musicPlayer.begin()) {  // initialise the music player
    Serial.println("Couldn't find VS1053, do you have the right pins defined?");
    while (1)
      ;
  }
  Serial.println("VS1053 found");

  musicPlayer.sineTest(0x44, 500);  // Make a tone to indicate VS1053 is working
  if (!SD.begin(CARDCS)) {
    Serial.println("SD failed, or not present");
    while (1)
      ;  // don't do anything more
  }
  Serial.println("SD OK!");

  // list files
  printDirectory(SD.open("/"), 0);

  // Set volume for left, right channels. lower numbers == louder volume!
  musicPlayer.setVolume(10, 10);

#if defined(__AVR_ATmega32U4__)
  // Timer interrupts are not suggested, better to use DREQ interrupt!
  // but we don't have them on the 32u4 feather...
  musicPlayer.useInterrupt(VS1053_FILEPLAYER_TIMER0_INT);  // timer int
#else
  // If DREQ is on an interrupt pin we can do background
  // audio playing
  musicPlayer.useInterrupt(VS1053_FILEPLAYER_PIN_INT);  // DREQ int
#endif

  // Play a file in the background, REQUIRES interrupts!

  // sound should play completely through, wait five seconds, then do the next file
  Serial.println("\nPlaying full sample file (non-looped)");
  musicPlayer.playbackLoop(false);
  checkLoopState(false);
  // sample file is three seconds long
  musicPlayer.playFullFile(sampleFile);
  Serial.println("Player stopped");
  // timer starts AFTER the file stops playing
  doWait(5);

  // Sound file should start playing, then a five second timer kicks off
  Serial.println("\nStarting sample file (non-looped)");
  musicPlayer.playbackLoop(false);
  checkLoopState(false);
  musicPlayer.startPlayingFile(sampleFile);
  // Timer starts as soon as the file starts playing
  doWait(6);

  // Sound file should start playing, then a five second timer kicks off
  Serial.println("\nStarting sample file (looped)");
  musicPlayer.playbackLoop(true);
  checkLoopState(true);
  musicPlayer.startPlayingFile(sampleFile);
  // sound should play a few times, then stop
  doWait(10);
  // after the five seconds, disable loopback then the playback should stop the next
  // time the library goes back for more sound file data
  Serial.println("Disabling looped playback");
  musicPlayer.playbackLoop(false);
  // sound should stop now or shortly thereafter
  doWait(5);

  // test doing it again
  // Sound file should start playing, then a five second timer kicks off
  Serial.println("\nStarting sample file (looped)");
  musicPlayer.playbackLoop(true);
  checkLoopState(true);
  musicPlayer.startPlayingFile(sampleFile);
  // sound should play twice, then stop
  doWait(5);
  // after the five seconds, disable loopback then the playback should stop the next
  // time the library goes back for more sound file data
  Serial.println("Disabling looped playback");
  musicPlayer.playbackLoop(false);
  // sound should stop now or shortly thereafter
  doWait(5);

  Serial.println("\nPlaying sample file (looped)");
  musicPlayer.playbackLoop(true);
  checkLoopState(true);
  // play this file repeatedly
  musicPlayer.playFullFile("/sample.mp3");

  // the following code Never executes
  Serial.println("Waiting 10 seconds");
  delay(10000);
  Serial.println("Cancelling playback loop");
  // then stop
  musicPlayer.playbackLoop(false);
  checkLoopState(false);
}

void loop() {}

/// File listing helper
void printDirectory(File dir, int numTabs) {
  while (true) {

    File entry = dir.openNextFile();
    if (!entry) {
      // no more files
      //Serial.println("**nomorefiles**");
      break;
    }
    for (uint8_t i = 0; i < numTabs; i++) {
      Serial.print('\t');
    }
    Serial.print(entry.name());
    if (entry.isDirectory()) {
      Serial.println("/");
      printDirectory(entry, numTabs + 1);
    } else {
      // files have sizes, directories do not
      Serial.print("\t\t");
      Serial.println(entry.size(), DEC);
    }
    entry.close();
  }
}

void doWait(int delaySeconds) {
  Serial.print("Waiting ");
  Serial.print(String(delaySeconds));
  Serial.println(" seconds");
  delay(delaySeconds * 1000);
}

void checkLoopState(boolean expectedState) {
  boolean currentState = musicPlayer.playbackLooped();
  String currentStateStr = currentState ? "True" : "False";
  String expectedStr = expectedState ? "True" : "False";

  Serial.print("Checking playback loop state (expected: ");
  Serial.print(expectedStr);
  Serial.println(")");
  Serial.print("Loop state: ");
  Serial.println(currentStateStr);
  if (currentState != expectedState) {
    Serial.println("State Validation FaILED");
  }
}

[ladyada]

can you perform a rebase since we just merged the other PR? that way it will pass CI :)

[johnwargo]

Sorry @ladyada I don't really know how to fix this.

[johnwargo]

[ladyada]

do you see this?

[johnwargo]

it looks like I made it worse

[ladyada]

yeah you'll need to do a git pull on your local repo, and see what got messed up, probably just a comment broke

[johnwargo]

nothing is ever easy (Wizard's First Rule)

[johnwargo]

@ladyada I don't get it, _loopplayback is an internal variable, why does it need to be documented? I'm not a C expert, but it doesn't make sense to move it to the header file just to document it.

Member _loopPlayback (variable) of file Adafruit_VS1053.cpp is not documented.

[johnwargo]

aaah, nevermind, I see how to document it in the file. Sigh.

[ladyada]

we use doxygen which requires any non-private functions/vars to be documented

[johnwargo]

OK @ladyada , all set.

[ladyada]

nice work!!