only executing 16 out of 47 "IF" statements

Using Arduino Programming Questions
1

What I seem to be experiencing is this:

the first 16 IF statements process the way I intend, but the 17th thru 47th don't seem to process.

My project: an UP/Down switch that sends different serial data depending on witch instance has been selected. specifically, changing patches (via MIDI) on 3 different devices. The Song Name and # are printed to an LCD for refrence. I added output to the serial monitor for troubleshooting. The LCD coding has been commented out(//) of this version because I notice no problem with the output to the LCD and I have the same issue with the output to the serial monitor. Song#'s 20 thru 45 have been omitted to save space(and your sanity).

example: switch from "song"#1(working for the weekend) to "song" #3(wild nights) should change my GR-55 to patch#26("lespaul"),change my POD to patch#97, and change my GR-30 to patch#28 in bank 10.

All the Midi switching works the way I expect it to, but only for "song"#'s 1-16. The problem is that switching up/down to/from "song"s 17-47 changes the "song" #, but does NOT send the correct MIDI signals. The signals from "song" # 16 are repeated.

As far as I can tell, the issue is with the "IF" statements, but I could be wrong(have been before, will be again).

the huge "SONGName" string array was the first way I found to print the Characters of the song name. These change with the "SONGNum" as I expect, but the variables for the MIDI data only change for "SONGNum" 1-16.

So what have I missed ?

I'm sure there are better organized ways for some of the program, and I am interested in opinions for that, but my focus is on sending the correct(or "what I expect") data out of the MIDI port.

serial monitor code

//#include <LiquidCrystal.h>

// initialize the library with the numbers of the interface pins
//LiquidCrystal lcd(12, 11, 7, 6, 5, 4);

//works with song #'s from 1-50
//16 charcter names
//program change format:(patch #, MIDI channel)
//control change format:(control #, value, MIDI channel)
//GR-55 MIDI channel=1
//PodXTLive MIDI channel=2
//GR-20 MIDI channel=3
//#include <MIDI.h>
//MIDI_CREATE_DEFAULT_INSTANCE();
int GRpatchNum=0; //GR-55 patch #
int PODpatchNum=0; //PODxt patch #
int GR20patchNum=0; //GR-20 patch #
int GR20bankNum=0; //GR-20 bank #
int SONGNum=0; //song #
int startNum;
int newNum;
char* SONGName[]={"_","Work Wkend","Cud Been Me ","Wild Nights","Everlong",
"Any Way U Want","Hey Jealosy","Sweetness","Summer 69","Think Wr Alone",
"Sex On Fire","Hotel Cali","Jenny, Jenny","Paradise City","Died In UR Arms",
"Somebdy Told Me","Evrybody Talks","Lit Up","Amrcn Girl","China Grove",
"Standing There","Laid","Buttercup","Ever st Rain","Green Day","500 Mi",
"Hurt So Good","BoysOSumer","Bonjovi","Brw Eye Girl","Evrtng Abt U",
"Rebel Yell","Ballroom Blitz","FF Right 2 Party","Sweet Caroline",
"Johnny B Goode","Cold Hard Bitch","JustWhatINeeded","Runaway DelSh",
"Hands 2 Yourself","You & YR Hand","Hey Ya","Still R&R","DancingInTheDark","Dance,Dance",
"Mr.Jones","Santaria","Want U 2 Want Me"};
 




void setup() {
  // put your setup code here, to run once:
pinMode(2, INPUT_PULLUP);   //patch down button
pinMode(3, INPUT_PULLUP);    //patch up button

//MIDI.begin(MIDI_CHANNEL_OMNI);  
  // set up the LCD's number of columns and rows:
  //lcd.begin(16, 2);
 // initialize serial communication:
  Serial.begin(9600);
}



void loop() {
  // put your main code here, to run repeatedly:
  startNum=(SONGNum);
  
if(digitalRead(3)== LOW&&SONGNum<50){    //when patch up pressed(set"< [max song #]")
  SONGNum++;   //song# + 1
  delay(400);
}
if(digitalRead(2)== LOW&&SONGNum>1){    //when patch down pressed
  SONGNum--;   //song# - 1
  delay(400);
}
if (SONGNum==1){
  //"working for the weekend";
  GRpatchNum=28;//1C strat
  PODpatchNum=81;//51
  GR20patchNum=1;
  GR20bankNum=0;
}
if (SONGNum==2){
  //"could have been me";
  GRpatchNum=28;//1C strat
  PODpatchNum=110;//6E
  GR20patchNum=8;//ACpiano
  GR20bankNum=0;
}
if (SONGNum==3){
  //"wild nights";
  GRpatchNum=26;//1A E335
  PODpatchNum=97;//61
  GR20patchNum=28;//Perc 29
  GR20bankNum=10;
}
if (SONGNum==4){
   //everlong
  GRpatchNum=28;//strat
  PODpatchNum=113;//modern rock
}
if (SONGNum==5){
   //any way want you it
  GRpatchNum=27;//Lespaul
  PODpatchNum=84;//80's rock lead
  GR20patchNum=5;//organ
  GR20bankNum=0;
}
if (SONGNum==6){
   //hey jealosy
  GRpatchNum=28;//strat
  PODpatchNum=90;//90's drive
}
if (SONGNum==7){
   //sweetness
  GRpatchNum=28;//strat
  PODpatchNum=114;//modern clean
  GR20patchNum=8;//ACpiano
  GR20bankNum=0;
}
if (SONGNum==8){
   //summer of 69
  GRpatchNum=28;//strat
  PODpatchNum=82;//80's chorus
  GR20patchNum=12;//
  GR20bankNum=0;
}
if (SONGNum==9){
   //i think were alone now
  GRpatchNum=28;//strat
  PODpatchNum=84;//80's rock lead
  GR20patchNum=2;//wave lead
  GR20bankNum=0;
}
if (SONGNum==10){
   //sex on fire
  GRpatchNum=28;//strat
  PODpatchNum=109;//sexonfire clean
}
if (SONGNum==11){
   //hotel california
  GRpatchNum=4;//12 str acst
  PODpatchNum=111;//eagles drive
}
if (SONGNum==12){
   //jenny jenny
  GRpatchNum=28;//strat
  PODpatchNum=82;//80's chorus
}
if (SONGNum==13){
   //paradise city
  GRpatchNum=27;//Lespaul
  PODpatchNum=86;//80's chr clean
}
if (SONGNum==14){
   //died in your arms
  GRpatchNum=12;//died in your arms
}
if (SONGNum==15){
   //somebody told me
  GRpatchNum=24;//smbdy told me
}
if (SONGNum==16){
   //everybody talks
  GRpatchNum=21;//cheap organ}
if (SONGNum==17){
   //lit up
  GRpatchNum=27;//LP
  PODpatchNum=100;//MSE rmy
}
if (SONGNum==18){
   //american girl
  GRpatchNum=19;//12str elect
}
if (SONGNum==19){
   //china grove
  GRpatchNum=6;//china grove
  PODpatchNum=99;// plexi LD
  GR20patchNum=8;//AC Piano
  GR20bankNum=0;
}
if (SONGNum==46){
  //santaria
  GRpatchNum=7;//rock organ
  PODpatchNum=88;//90s cln
}
if (SONGNum==47){
  //want u 2 want me
  GRpatchNum=27;//LP
  PODpatchNum=95;//70s LD
}
newNum=(SONGNum);

if (newNum!= startNum){
  Serial.print("GR#");
  Serial.println(GRpatchNum);
 // MIDI.sendProgramChange(GRpatchNum,1);
 Serial.print("POD#");
 Serial.println(PODpatchNum); 
  //MIDI.sendProgramChange(PODpatchNum,2);
  Serial.print("GR20BANK#");
  Serial.println(GR20bankNum); 
  //MIDI.sendControlChange(0,GR20bankNum,3);
  Serial.print("GR20#");
  Serial.println(GR20patchNum);
  Serial.print("Song: ");
  Serial.println(SONGName[SONGNum]);
  Serial.println(SONGNum);
  //MIDI.sendProgramChange(GR20patchNum,3);
  //MIDI.sendProgramChange(SONGNum,4); //turn on to display song# on midi diagnostic
  //lcd.clear();
}
  //lcd.setCursor(0, 0);//top line
  //lcd.print(SONGName[SONGNum]);
  //lcd.setCursor(0, 1);//bottm line
  //lcd.print(SONGNum);

}

GR_POD__GR20change_serialmonitor.ino (7.72 KB)

2

Count your left curly brackets and you right curly brackets and see if they are equal.

Paul

3

Look at where your closing bracket is for entry 16 (lost in the comment)

if (SONGNum==16){
   //everybody talks
  GRpatchNum=21;[color=red]//cheap organ[b]}[/b][/color]

You can easily find such bugs if you were indenting your code correctly - press ctrl-T in the IDE and you'll see the formatting will reveal the error

Using a switch/case construct would make your code easier to read or at least use else between your if to avoid executing all the tests once you have found the correct match

4

J-M-L:
Look at where your closing bracket is for entry 16 (lost in the comment)

if (SONGNum==16){

//everybody talks
  GRpatchNum=21;//cheap organ}




You can easily find such bugs if you were indenting your code correctly - press ctrl-T in the IDE and you'll see the formatting will reveal the error

Using a `switch/case` construct would make your code easier to read or at least use `else` between your `if` to avoid executing all the tests once you have found the correct match

Well I'm be damned. Thank you.never would have noticed that in that much code. And I must have looked at the 16th and 17th statements 20 times...

I was reading up on switch/case to try on the next revision of the program. I will definitely give it a try.

5

To be honest if memory is not an issue I'd take a big shortcut in your code and create something like this at the beginning (where you have your song names as well

int GRpatchNum = 0; //GR-55 patch #
int GRpatchNumTbl[] = {0, 28, 28, 26, 28, 27, 28, 28, ......}; // FILL IN WITH ALL ENTRIES (first entry void)

int PODpatchNum = 0; //PODxt patch #
int PODpatchNumTbl[] = {0, 81, 110, 97, 113, 84, 90, 114, ......};// FILL IN WITH ALL ENTRIES (first entry void)

int GR20patchNum = 0; //GR-20 patch #
int GR20patchNumTbl[] = {0, 1, 8, 28, 0, 5, 0, 8, ......};// FILL IN WITH ALL ENTRIES (first entry void)

int GR20bankNum = 0; //GR-20 bank #
int GR20bankNumTbl[] = {0, 0, 0, 10, 0, 0, 0, 0, ......};// FILL IN WITH ALL ENTRIES (first entry void)

(do they really need to be int, or a byte (between 0 and 255 would be enough?)

and then instead of having all the if tests I would just go read the values from the arrays

  if (SONGNum >= 1) && (SONGNum <= 47) {
    GRpatchNum = GRpatchNumTbl[SONGNum];
    PODpatchNum = PODpatchNumTbl[SONGNum];
    GR20patchNum = GR20patchNuTbl[SONGNum];
    GR20bankNum = GR20bankNumTbl[SONGNum];
    newNum = SONGNum;

    if (newNum != startNum) {
      Serial.print("GR#");
      Serial.println(GRpatchNum);
      .....      

    }
6

I've been trying to decide which method would make it easiest to add entries and change the order. I was actually thinking of trying to make a spread sheet in such a way that I could copy the text from the spreadsheet and paste it into the program as data... eventually. I'm really not crazy about the array of song name text, but I'm trying to only change 1 thing in the programat a time so as to make troubleshooting easier when there's a problem.

7

Tested the revised code. Works just the way I wanted. Consider my problem solved. Thanks!

8

Lpgoldtop:
Thank you.never would have noticed that in that much code.

That should be ringing warning bells for you.

If you used arrays you could probably get all that (for all the songs) done in 4 lines or so - something like

GRpatchNum = GRpatchArray[SONGnum];
PODpatchNum = PODpatchArray[SONGnum];
GR20patchNum = GR20patchArray[SONGnum];
GR20bankNum = GR20bankArray[SONGnum];

in which the variable SONGnum is used to index into the 4 arrays

And there may even be a neater way to do it.

I have based this on this snippet, picked at random

if (SONGNum==9){
   //i think were alone now
  GRpatchNum=28;//strat
  PODpatchNum=84;//80's rock lead
  GR20patchNum=2;//wave lead
  GR20bankNum=0;
}

...R

9

Robin - yes that's what was suggested in #4

10

J-M-L:
Robin - yes that's what was suggested in #4

Sorry. I missed that completely - and could have saved myself a lot of typing.

...R

11

:slight_smile:

12

Robin2:
If you used arrays you could probably get all that (for all the songs) done in 4 lines or so -

...R

I'm avoiding the arrays for now because it makes it difficult to change or re-order an entry.but,I will probably use arrays once I get all the data arranged in a table.then I can copy and paste a whole column in the right order.

13

I'm avoiding the arrays for now because it makes it difficult to change or re-order an entry

On the contrary, arrays make such changes simpler.

14

Maybe the use of an array of structs makes it easier to reshuffle in code. A struct creates a new type that combines a number of related fields. Think of a phonebook where one record contains e.g. a field for first and lastname, a phone number and an address.

struct SONGDATA
{
  char *songname;
  int GRpatchNum;
  int PODpatchNum;
  int GR20patchNum;
  int GR20bankNum;
};

SONGDATA songs[] =
{
  {"-", 0, 0, 0, 0},
  {"Work Wkend", 28, 81, 1, 9},
  {"Cud Been Me ", 28, 110, 8, 0},
  {"Wild Nights", 26, 97, 28, 10},
};

With this, you can just move rows around.

To demostrate how to use it

void setup()
{
  Serial.begin(250000);

  Serial.print("number of songs: "); Serial.println(sizeof(songs) / sizeof(songs[0]) - 1);
  Serial.println("++++++++");

  for (int songNum = 1; songNum < sizeof(songs) / sizeof(songs[0]); songNum++)
  {
    Serial.println(songs[songNum].songname);
    Serial.println(songs[songNum].GRpatchNum);
    Serial.println(songs[songNum].PODpatchNum);
    Serial.println(songs[songNum].GR20patchNum);
    Serial.println(songs[songNum].GR20bankNum);
    Serial.println("++++++++");

  }
}

void loop()
{
  // put your main code here, to run repeatedly:

}
15

sterretje:
Maybe the use of an array of structs makes it easier to reshuffle in code. A struct creates a new type that combines a number of related fields. Think of a phonebook where one record contains e.g. a field for first and lastname, a phone number and an address.

struct SONGDATA

{
 char *songname;
 int GRpatchNum;
 int PODpatchNum;
 int GR20patchNum;
 int GR20bankNum;
};

SONGDATA songs[] =
{
 {"-", 0, 0, 0, 0},
 {"Work Wkend", 28, 81, 1, 9},
 {"Cud Been Me ", 28, 110, 8, 0},
 {"Wild Nights", 26, 97, 28, 10},
};



With this, you can just move rows around.

To demostrate how to use it

void setup()
{
 Serial.begin(250000);

Serial.print("number of songs: "); Serial.println(sizeof(songs) / sizeof(songs[0]) - 1);
 Serial.println("++++++++");

for (int songNum = 1; songNum < sizeof(songs) / sizeof(songs[0]); songNum++)
 {
   Serial.println(songs[songNum].songname);
   Serial.println(songs[songNum].GRpatchNum);
   Serial.println(songs[songNum].PODpatchNum);
   Serial.println(songs[songNum].GR20patchNum);
   Serial.println(songs[songNum].GR20bankNum);
   Serial.println("++++++++");

}
}

void loop()
{
 // put your main code here, to run repeatedly:

}

That would suit me just fine. I didn't know about "struct". I'll have to learn more.thanks.