Added code does not work

Hey folks, I am currently working out how to playback soundfiles with the DYPLayer library.
I want to modify the following working code in a way that it plays randomly on of 5 "firing soundfiles":

this is the working code:

/*////////////////////////////////////////////////////////
*
* BOOT function & poti brightness function improvement on linear brightness scaling
*
*
////////////////////////////////////////////////////////*/



//Button
#include "Button.h"

//Function Execution Timer
#include "NextExcecutionTime.h"

//DM13A classes
#include "DM13A_LedSegment.h"
#include "DM13A.h"

//Speaker
#include <Arduino.h>
#include <SoftwareSerial.h>
#include "DYPlayerArduino.h"

//FastLED
#include <FastLED.h>

// exluded static declarations
#include "declarations.h"


//Buttons
Button SW_WandTip(10);
Button SW_StartUp1(8);
Button SW_StartUp2(7);
Button SW_Intensify(4);
Button SW_BarGraph(3);
Button SW_Vent(2);


//Speaker 0
constexpr int rxPin0 = A4;
constexpr int txPin0 = A5;

constexpr int rxPin1 = A3;
constexpr int txPin1 = A2;

SoftwareSerial mySerial0 = SoftwareSerial(rxPin0, txPin0);  //pack
DY::Player playerSPack(&mySerial0);

SoftwareSerial mySerial1 = SoftwareSerial(rxPin1, txPin1);  // wand
DY::Player playerSWand(&mySerial1);

//###################################

/*###############
Boot and loop
################*/
//Afterlife Proton Pack
constexpr char StartUpAndIdle_ProtonPack_AfterLife[] = "/01010.mp3";
constexpr char StartUpAndIdle_ProtonPack_AfterLifeAlt[] = "/01015.mp3";
constexpr char Idle_ProtonPack_AfterLife[] = "/01020.mp3";
constexpr char Idle_ProtonPack_AfterLifeAlt[] = "/01025.mp3";


//Ghostbusters 1+2
//Proton Pack
constexpr char Idle_ProtonPack_Ghostbusters[] = "/01700.mp3";
constexpr char Idle_ProtonPack_GhostbustersAlt[] = "/01710.mp3";
constexpr char StartUpAndIdle_ProtonPack_Ghostbusters[] = "/01720.mp3";
constexpr char StartUpAndIdle_ProtonPack_GhostbustersAlt[] = "/01730.mp3";

constexpr char Idle_Wand_Ghostbusters[] = "/02700.mp3";
constexpr char Idle_Wand_GhostbustersAlt[] = "/02710.mp3";
constexpr char StartUpAndIdle_Wandk_Ghostbusters[] = "/02720.mp3";
constexpr char StartUpAndIdle_Wand_GhostbustersAlt[] = "/02730.mp3";


/*###############
Firing start + loop
################*/
constexpr char FiringAndLoop_ProtonPack_Ghostbusters[] = "/01100.mp3";
constexpr char FiringAndLoop_ProtonPack_GhostbustersAlt[] = "/01120.mp3";
constexpr char FiringAndLoop_ProtonPack_GhostbustersHighPower[] = "/01110.mp3";

constexpr char FiringAndLoop_ProtonPack_AfterLife[] = "/01130.mp3";
constexpr char FiringAndLoop_ProtonPack_FrozenEmpire[] = "/01140.mp3";


/*###############
Firing End + loop
################*/
constexpr char FiringEndAndIdle_ProtonPack_AfterLife1[] = "/01200.mp3";
constexpr char FiringEndAndIdle_ProtonPack_AfterLife1V2[] = "/01210.mp3";

constexpr char FiringEndAndIdle_ProtonPack_AfterLife2[] = "/01230.mp3";
constexpr char FiringEndAndIdle_ProtonPack_AfterLife2V2[] = "/01240.mp3";

constexpr char FiringEndAndIdle_ProtonPack_AfterLife3[] = "/01260.mp3";
constexpr char FiringEndAndIdle_ProtonPack_AfterLife3V2[] = "/01270.mp3";

constexpr char FiringEndAndIdle_ProtonPack_Ghostbusters1[] = "/01220.mp3";
constexpr char FiringEndAndIdle_ProtonPack_Ghostbusters2[] = "/01250.mp3";
constexpr char FiringEndAndIdle_ProtonPack_Ghostbusters3[] = "/01280.mp3";


/*###############
misc
################*/
constexpr char Recovery_ProtonPack[] = "/01900.mp3";

constexpr char Recovery_Wand[] = "/02900.mp3";
constexpr char BarGraphBeep_Wand[] = "/02900.mp3";


/*###############
Shutdown
################*/
constexpr char Shutdown_ProtonPack_Ghostbusters[] = "/01800.mp3";
constexpr char Shutdown_ProtonPack_AfterLife[] = "/01810.mp3";
constexpr char Shutdown_ProtonPack_AfterLifeAlt[] = "/01820.mp3";

constexpr char Shutdown_Wand_Ghostbusters[] = "/02800.mp3";
//###################################



void setup() {
  randomSeed(analogRead(A0));  // change if necessary

  // Buttons
  SW_WandTip.begin();
  SW_StartUp1.begin();
  SW_StartUp2.begin();
  SW_Intensify.begin();
  SW_BarGraph.begin();
  SW_Vent.begin();

  playerSPack.begin();
  playerSPack.setVolume(15);

  playerSWand.begin();
  playerSWand.setVolume(25);

  playerSWand.setCycleMode(DY::PlayMode::RepeatOne);
  playerSWand.stop();
  //Print stuff
  Serial.begin(9600);
}


constexpr char testSound_recovery_pack[] = "/01900.mp3";
constexpr char testSound_recovery_wand[] = "/02910.mp3";
constexpr char testSound_beep_wand[] = "/02900.mp3";
constexpr char testSound_shutdown[] = "/01810.mp3";


struct AudioSequence {
  const char *loopSound;
  const char *interludeSound;
  bool canInterlude;
};


AudioSequence FiringV1{
  testSound_recovery_pack,
  testSound_shutdown,
  true
};

AudioSequence FiringEnd{
  Idle_ProtonPack_AfterLife,
  FiringEndAndIdle_ProtonPack_AfterLife1,
  true
};


bool hasBooted = false;  // helper variable

enum class PlaySound {
  play_booting,
  play_firing,
  play_firingEnd,
  play_shuttingDown,
  onHold
};

PlaySound determinePlaySound();  // otherwise the function does not compile

PlaySound determinePlaySound() {  //give Button Array??
  if (SW_Vent.read() && SW_Vent.toggled()) {
    Serial.println("Booting");
    hasBooted = true;
    return PlaySound::play_booting;
  }

  if (SW_Intensify.released() && hasBooted) {
    Serial.println("Firing");
    return PlaySound::play_firing;
  }

  if (SW_Intensify.pressed() && hasBooted) {
    Serial.println("Firing ended");
    return PlaySound::play_firingEnd;
  }

  if (SW_Vent.pressed() && hasBooted) {
    playerSWand.setCycleMode(DY::PlayMode::OneOff);
    Serial.println("Shuting down");
    hasBooted = false;
    return PlaySound::play_shuttingDown;
  }

  //default case
  return PlaySound::onHold;
  Serial.println("On Hold");
}



PlaySound currentState;

void PlayCurrentSound() {
  switch (currentState) {

    case PlaySound::play_booting:
      playerSWand.setCycleMode(DY::PlayMode::RepeatOne);
      playerSWand.playSpecifiedDevicePath(DY::Device::Sd, StartUpAndIdle_ProtonPack_AfterLife);
      break;

    case PlaySound::play_firing:
   playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_Ghostbusters);
    /* 
     int randomFireStart = random(5);
     Serial.println(randomFireStart);
      if (randomFireStart == 0) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_Ghostbusters);
      else if (randomFireStart == 1) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_GhostbustersAlt);
      else if (randomFireStart == 2) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_GhostbustersHighPower);  //maybe put to other
      else if (randomFireStart == 3) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_AfterLife);
      else if (randomFireStart == 4) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_FrozenEmpire);
      */
      break;

    case PlaySound::play_firingEnd:
      playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringEnd.loopSound);
      playerSWand.interludeSpecifiedDevicePath(DY::Device::Sd, FiringEnd.interludeSound);
      break;

    case PlaySound::play_shuttingDown:
      playerSWand.playSpecifiedDevicePath(DY::Device::Sd, Shutdown_ProtonPack_AfterLifeAlt);
      break;

    default:
      Serial.println("ERROR");

      break;
  }
}



void loop() {

  currentState = determinePlaySound();
  PlayCurrentSound();

  //######################################

  /*
Serial.print(SW_BarGraph.read());
Serial.print(SW_BarGraph.pressed());
Serial.print(SW_BarGraph.released());
Serial.print(SW_BarGraph.has_changed());
Serial.println(SW_BarGraph.toggled());

//Button unpressed: 00000
//Button pressed:   10100 -> initial, then 10000
//Button released:  01000 -> initial, then 00000

//Switch off: 00000
//Button pressed:   10100 -> initial, then 10000
//Button released:  01000 -> initial, then 00000
*/
}

when i run this code i get constantly printed "ERROR" until a state change has occurred, all soundfiles play when they meet the conditions.
when i modify the function with a random statement the code only plays the firing sounds randomly but never meets the other conditions. it also does not print "ERROR" after starting the arduino.

I don't get why adding the randomization breaks the code.

void PlayCurrentSound() {
  switch (currentState) {

    case PlaySound::play_booting:
      playerSWand.setCycleMode(DY::PlayMode::RepeatOne);
      playerSWand.playSpecifiedDevicePath(DY::Device::Sd, StartUpAndIdle_ProtonPack_AfterLife);
      break;

    case PlaySound::play_firing:
   // playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_Ghostbusters);
     
     int randomFireStart = random(5);
     Serial.println(randomFireStart);
      if (randomFireStart == 0) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_Ghostbusters);
      else if (randomFireStart == 1) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_GhostbustersAlt);
      else if (randomFireStart == 2) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_GhostbustersHighPower);  //maybe put to other
      else if (randomFireStart == 3) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_AfterLife);
      else if (randomFireStart == 4) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_FrozenEmpire);
      
      break;

    case PlaySound::play_firingEnd:
      playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringEnd.loopSound);
      playerSWand.interludeSpecifiedDevicePath(DY::Device::Sd, FiringEnd.interludeSound);
      break;

    case PlaySound::play_shuttingDown:
      playerSWand.playSpecifiedDevicePath(DY::Device::Sd, Shutdown_ProtonPack_AfterLifeAlt);
      break;

    default:
      Serial.println("ERROR");

      break;
  }
}

please reassure us that you got the program to work properly before you began to change it!

This is the default case in the switch using currentState. None of the PlaySound states agree.

Yes. I do not see case code for on hold.

Tiny window, doesn't mean it isn't there. Print in the default case to see the value of the switch variable that isn't hadnled.

a7

You also have "local" libraries that I can not use... unless you post them.

#include "Button.h"
#include "NextExcecutionTime.h"
#include "DM13A_LedSegment.h"
#include "DM13A.h"

Me neither. If you are correct insaying the only difference between a working sketch and a failing sketch is all in those few different lines in PlayCurrentSound() ,there is no obvious reason.

What does the compiler report when you verify with respect to how much memory is being used? Copy those few lines at the end of its report and paste them here. Use the <CODE/> tool you see in the message composition window tool bar. And do as it says

type or paste code here

And check, double check then check again that there are no other differences no matter how small or that you think cannot be the source of your troubles.

A mystery. As @xfpd notes, your sketch is not organized in a way conducive to live testing of the logic, and looking where you've asked us to focus shows nothing out of the ordinary.

a7

yes it works probably without the modification.

i found the "mistake" i moved

 int randomFireStart = random(5);

out of the function and only put

randomFireStart = random(5);

into the function so that it looks like this:

int randomFireStart;
PlaySound currentState;

void PlayCurrentSound() {
  switch (currentState) {

    case PlaySound::play_booting:
      playerSWand.setCycleMode(DY::PlayMode::RepeatOne);
      playerSWand.playSpecifiedDevicePath(DY::Device::Sd, StartUpAndIdle_ProtonPack_AfterLife);
      break;

    case PlaySound::play_firing:
      randomFireStart = random(5);

      Serial.println(randomFireStart);
      if (randomFireStart == 0) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_Ghostbusters);
      else if (randomFireStart == 1) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_GhostbustersAlt);
      else if (randomFireStart == 2) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_GhostbustersHighPower);  //maybe put to other
      else if (randomFireStart == 3) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_AfterLife);
      else if (randomFireStart == 4) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_FrozenEmpire);
      break;

    case PlaySound::play_firingEnd:
      playInterluded(FiringEnd);
      break;

    case PlaySound::play_shuttingDown:
      playerSWand.playSpecifiedDevicePath(DY::Device::Sd, Shutdown_ProtonPack_AfterLifeAlt);
      break;

    default:
      Serial.println("ERROR");

      break;
  }
}

i do not understand why the deceleration of a variable in a function messes up the function but it seems that it is the case when using the random function.

[last typing first!]

Enjoy my ranting and raving below…

but there is a very good reason I missed every time I looked at this matter, and it is a bitch of an mistake to find.

When you declare local variables in a switch/case statement, odd things get happening unless the entirety of the code for a case where that is done is { embraced }.

I can't say everyone gets bitten by this, only that I have been, and I am ashamed to admit more than once…

Go to the IDE now and dial up all verbosity and warnings in the preferences. I believe this is something the compiler could have pointed out by way of a warning, if allowed. I have never understood why the IDE ships with being quiet about this and several other mistakes it is too easy to make as the default setting.


TBC, the below was composed before a critical mass of brain cells began cooperating enough to at least see WTF was the issue:

Unlearn that.

There is nothing wrong with what you were doing, it has no possibility of being what you"fixed".

I have searched through all your code and your explanation is not correct.

If you would like to, post two sketches with absolutely no difference other than the manner in which you decelerate declare that variable.

One you say works, one you say does not, along with a reminder of just how the non-working one fails to work.

Since we can't run this for ourselves, be clear about the flaw(s),

a7

Thanks @alto777 !
I have never entered this trap.
But that is just sheer luck...
Good to know this.

Groet, Koen.

@e1m1 I agree with @alto777 that there seems nothing wrong with your declaration...

You don't need to move the declaration up to global scope (i.e. out of the function) just out of the case statement

void PlayCurrentSound() {
  int randomFireStart = random(5);
  switch (currentState) {
    ...
  }

of, if you don't want to call the random() function every time you call PlayCurrentSound(), you can just have the declaration outside the switch statement, but have the assignment inside the specific case statement

void PlayCurrentSound() {
  int randomFireStart;
  switch (currentState) {
    ...
    case PlaySound::play_firing:
      randomFireStart = random(5);
      Serial.println(randomFireStart);
      ...
  }

Could you try spelling this like:

   case PlaySound::play_firing: {  // Note added brace!     
     int randomFireStart = random(5);
     Serial.println(randomFireStart);
     if (randomFireStart == 0) playerSWand.playSpecifiedDevicePath(DY::Device::Sd, FiringAndLoop_ProtonPack_Ghostbusters);
     // the rest of the cases ...
     break;
   }

I believe that in C++ you need to surround the "int randomFireStart" here in a new scope if it appears in a switch statement. The compilers I've used before will emit a warning or an error about not being able to declare a variable there normally. I am not sure why yours didn't.

yes that fixed the issue.thanks to all of you.

Your points have been addressed, and part of that was that you missed

HTH

a7