Trouble with functions and if statements

I asked this question before but no one was able to help. The problem is in the loop. This is a condensed version of the actual code, it will work just fine as it is with a 20x4 lcd and a button, though I don’t expect anyone to go to the hassle of testing it out themselves. Yes, I have tested this code out and it works exactly as it does with the rest of the code, so please don’t ask for the rest of the code. Again, the problem is in the loop. Thank you.

#include <Wire.h>
#include <LiquidCrystal_I2C.h>

LiquidCrystal_I2C lcd(0x3f, 2,1,0,4,5,6,7,3,POSITIVE);


const int eastB = 9;
const int westB = 8;
const int enterB = 7;
const int cancelB = 10;

int cursorPos[2] = {0,0};

bool hasPushed = false;
bool makeList = true;

String unitTypes[6] = {"Infantry", "Spearmen", "Archers", "Cavalry", "Peasants", "see army"};
int unitTypesSize = 6;
String questionOne[2] = {"Move or", "camp?"};

int lineTrackerX = 0;
int lineTrackerY = 0;
int itemPosCounter = 0;
bool confirmSelect = false;
bool selecting = false;

bool isLookingAtBigMap = false;
bool isLookingAtRecruitmentMenu = true;
bool isLookingAtArmy = false;

int numOfInfantry =0;
int numOfSpearmen=0;
int numOfArchers=0;
int numOfCavalry=0;
int numOfPeasants=0;

void setup()
{
 lcd.begin(20,4);
 
 pinMode(eastB, INPUT_PULLUP);
 pinMode(westB, INPUT_PULLUP);
 pinMode(enterB, INPUT_PULLUP);
 pinMode(cancelB, INPUT_PULLUP);

 lcd.home();
}




void loop() //-------------------------------------------------------------------
{
  if (isLookingAtBigMap == true) //isLookingAtBigMap is FALSE
    {
      MakeSelectionMenu(questionOne, 2, 0); //if this call is commented out, everything works fine
    }
   
  if (isLookingAtRecruitmentMenu == true)
    {
    MakeSelectionMenu(unitTypes, unitTypesSize, 1); //function being slightly messed up
    }
}





void MakeSelectionMenu(String stringArray[], int sizeOfArray, int separateLastItem)
{
  int itemPlacesX[sizeOfArray];
  int itemPlacesY[sizeOfArray];
 
  if (makeList == true)
  {
   for (int i = 0; i< sizeOfArray; i++)
   {
    if (i<sizeOfArray-separateLastItem)//to print the last item in the bottom corner as a "see army" button
    {
     if (stringArray[i].length() + 2 + lineTrackerX >20)
     {
      lineTrackerX = 0;
      lineTrackerY +=1;
     }
     lcd.setCursor(lineTrackerX,lineTrackerY);
     lcd.print(stringArray[i]);
     itemPlacesX[i] = lineTrackerX;
     itemPlacesY[i] = lineTrackerY;
     lineTrackerX += stringArray[i].length()+2;
     if (i<(sizeOfArray-separateLastItem)-1)
     {
      lcd.print(", ");
     }
    }
   else if (i<sizeOfArray && separateLastItem >0)//print the see army button
    {
      itemPlacesX[i] = 12;
      itemPlacesY[i] = 3;
      lcd.setCursor(12,3);
      lcd.print(stringArray[i]);
    }
   }
   makeList = false;
   lcd.blink();
  }

 
  //-------------------------------------------------------------------------------------------
 
 
  if (digitalRead(eastB) == LOW && hasPushed == false && confirmSelect == false)
  {
    //-------------------------------------------------------------
    if (itemPosCounter<sizeOfArray-1)
    {
      hasPushed = true;
      itemPosCounter+=1;
      cursorPos[0] = itemPlacesX[itemPosCounter];
      cursorPos[1] = itemPlacesY[itemPosCounter];
      delay(175);
      hasPushed = false;
    }
  }
  //---------------------------------------------------------------------
   if (digitalRead(westB) == LOW && hasPushed == false && confirmSelect == false)
  {
    if (itemPosCounter>0)
    {
      hasPushed = true;
      itemPosCounter-=1;
      cursorPos[0] = itemPlacesX[itemPosCounter];
      cursorPos[1] = itemPlacesY[itemPosCounter];
      delay(175);   
      hasPushed = false;   
    }
  }
//----------------------------------------------------------
  if (digitalRead(enterB) == LOW && hasPushed == false && confirmSelect == false
     && itemPosCounter < sizeOfArray-1)
  {
    hasPushed = true;
    confirmSelect = true;
    lcd.clear();
    lcd.noBlink();
    lcd.noCursor();
   
    lcd.setCursor(0,0);
    lcd.print("Are you sure you ");
    lcd.setCursor(0,1);
    lcd.print("want to recruit ");
    lcd.setCursor(0,2);
    lcd.print(stringArray[itemPosCounter] + "?");
   
    delay(175);   
    hasPushed = false;
  }
  //--------------------------------------------------------------
  if (digitalRead(enterB) == LOW && hasPushed == false && confirmSelect == false
     && itemPosCounter == sizeOfArray-1) //if it's the last element in array
  {
    hasPushed = true;
    confirmSelect = true;
    lcd.clear();
    lcd.noBlink();
    lcd.noCursor();
   
    isLookingAtArmy = true;//specifically for looking at army
   
    delay(175);   
    hasPushed = false;
  }
 //----------------------------------------------------------------
  if (digitalRead(cancelB) == LOW && hasPushed == false && confirmSelect == true)
    {
      hasPushed = true;
      lcd.clear();
      lineTrackerX = 0;
      lineTrackerY = 0;
      makeList = true;
      delay(175);   
      hasPushed = false;
      confirmSelect = false;
    }
//-----------------------------------------------
    if (digitalRead(enterB) == LOW && hasPushed == false && confirmSelect == true)
    {
      hasPushed = true;
     
      if (stringArray[itemPosCounter] == "Infantry")
      {
        numOfInfantry +=1;
      }
      if (stringArray[itemPosCounter] == "Spearmen")
      {
        numOfSpearmen +=1;
      }
      if (stringArray[itemPosCounter] == "Archers")
      {
        numOfArchers +=1;
      }
      if (stringArray[itemPosCounter] == "Cavalry")
      {
        numOfCavalry +=1;
      }
      if (stringArray[itemPosCounter] == "Peasants")
      {
        numOfPeasants +=1;
      }
      lcd.clear();
      lineTrackerX = 0;
      lineTrackerY = 0;
      makeList = true;
      delay(175);   
      hasPushed = false;
      confirmSelect = false;
    }
    //----------------------------------------------
  lcd.setCursor(cursorPos[0],cursorPos[1]);
}

To put it simply, a function call that shouldn’t even be reached is somehow messing up another call of the same function but with different inputs. If either one of the calls are commented out, the other works perfectly.

To be even clearer, almost everything about the function works fine, except for what must be either the cursorPos or one of the lineTracker variables getting messed up, the selection cursor jumps all over the place when scrolling through the list, it should only move to the positions of the first letter of every element in the array. Thank you for reading this even if you are unable to help me.

tristan369:
I asked this question before but no one was able to help.

Please post a link to the earlier Thread so we don't repeat stuff. Better still, click Report to Moderator and ask to have the Threads merged so we have all the info about the project in one place

My wild guess is that your use of the String class is causing memory corruption. It is not a good idea to use the String (capital S) class on an Arduino as it can cause memory corruption in the small memory on an Arduino. This can happen after the program has been running perfectly for some time. Just use cstrings - char arrays terminated with '\0' (NULL).

When using Cstrings you must use strcmp() to compare values rather than ==

...R

Nothing will be repeated here, I guarantee you. All they did was nag me for not posting the entire code, and when I finally did, a couple people nit picked completely irrelevant code (code practices in functions that were not even being called). So I tried to condense the code to only relevant information, but it was too late, my post was old news by then and there hasn't been a reply to it since. I hope the same thing doesn't happen here where you all nag me to link to the post and then when I finally do none of you help.

And I will certainly try that. I noticed such String corruption in other code too but didn't know that the String was the problem. If I tried to list an array of Strings, if it was more than 20 elements, (or there were more than one array), the last few lines are missing letters, sometimes having random characters. I assumed this was just because they took up a lot of memory on the Arduino, as it displayed 80% usage or above when this happened and would say "unstable" or something. Thank you.

Please, I think it is fair to ask for a link to the other thread. It will save a lot of time.

@tristan369 we only allow people to have one account on the Arduino forum.

tristan369:
it was too late, my post was old news

In my experience, it doesn't work that way here. Every time your post gets a reply, it is bumped back to the top of the board.

{
  if (isLookingAtBigMap == true) //isLookingAtBigMap is FALSE
    {
      MakeSelectionMenu(questionOne, 2, 0); //if this call is commented out, everything works fine
    }
   
  if (isLookingAtRecruitmentMenu == true)
    {
    MakeSelectionMenu(unitTypes, unitTypesSize, 1); //function being slightly messed up
    }
}

I've not read or tried to understand your entire code but I am wondering what happens if both isLookingAtBigMap and isLookingAtRecruitmentMenu are true, does that cause a problem? Should they be exclusive so if one is true the other is not?

I agree about Strings; guaranteed problems.

PerryBebbington:
I've not read or tried to understand your entire code but I am wondering what happens if both isLookingAtBigMap and isLookingAtRecruitmentMenu are true, does that cause a problem? Should they be exclusive so if one is true the other is not?

I agree about Strings; guaranteed problems.

I tried this out just now and what happens is it displays the questionOne array, but the cursor positions are all over place, even changing as i go back and forth between two positions. I have no idea how to explain that, only the itemPos should change when I scroll through it. Similar results as the problem I have now.

pert:
@tristan369 we only allow people to have one account on the Arduino forum.
In my experience, it doesn't work that way here. Every time your post gets a reply, it is bumped back to the top of the board.

I didn't want to sound ungrateful or resentful but ya they basically just stopped "helping" me after I gave them what they wanted. The full code fits in two replies.

In my experience, condensing my sketch down the the absolute minimum amount of code required to reproduce the problem often results in the source of the problem becoming clear to me. In the event that doesn't happen, it certainly helps the people you are asking for assistance, so the time you spent on that certainly wasn't wasted.

Robin2:
My wild guess is that your use of the String class is causing memory corruption. It is not a good idea to use the String (capital S) class on an Arduino as it can cause memory corruption in the small memory on an Arduino. This can happen after the program has been running perfectly for some time. Just use cstrings - char arrays terminated with '\0' (NULL).

When using Cstrings you must use strcmp() to compare values rather than ==

Can you explain how to do this exactly. There doesn't seem to be a "cstring" value type, and I need to make an array of words, not an array of letters that makes a single word.

I tried this out just now and what happens is it displays the questionOne array, but the cursor positions are all over place

OK, don't know then. I hope someone else can help you find the solution.

Good luck :slight_smile:

pert:
In my experience, condensing my sketch down the the absolute minimum amount of code required to reproduce the problem often results in the source of the problem becoming clear to me. In the event that doesn't happen, it certainly helps the people you are asking for assistance, so the time you spent on that certainly wasn't wasted.

Which is exactly why I condensed it so it STILL WORKS on the Arduino. You clearly didn't read my problem thoroughly. When I comment out that first call of the function under an if statement that shouldn't even be reached, IT WORKS PERFECTLY. Yes, I am talking about the condensed code I gave to you. From now on, don't think of it as "condensed code," it IS the code at this point, and it doesn't work. I shouldn't even have told you it was condensed code, that was problem with the last post everyone just ignored my problem and got sidetracked making completely irrelevant points. Thank you.

Tristan,
I really think you need to deal with the Strings before looking any further. They might or might not be the cause of your problems, but there will always be the suspicion that they are until you demonstrate the problem still exists with char arrays (c-strings).

PerryBebbington,
How is this done?I need to make an array of words, how would this be done with c-strings? Thank you.

Which Arduino are you using? Can you lay your hands on one with more memory as a test?

tristan369:
I didn't want to sound ungrateful or resentful but ya they basically just stopped "helping" me after I gave them what they wanted.

That's exactly why I want to see the previous Thread. There must have been some reason why people stopped helping you.

tristan369:
Nothing will be repeated here, I guarantee you

How can you guarantee that? People gave you advice, or asked you questions in the previous Thread and they may easily give the same advice or ask the same questions here because they cannot see that it was done before.

...R

wildbill:
Which Arduino are you using? Can you lay your hands on one with more memory as a test?

No, I'm using my Uno, I've only got Nanos besides that.

Robin2:
That’s exactly why I want to see the previous Thread. There must have been some reason why people stopped helping you.
How can you guarantee that? People gave you advice, or asked you questions in the previous Thread and they may easily give the same advice or ask the same questions here because they cannot see that it was done before.

How I created two different accounts I have no idea, I think one was made from a long time ago and I forgot about it and now I’m signed into it and it won’t let me sign into another account. But I found the link. It’s not going to help you… I shouldn’t even have mentioned that I made another post… Are you going to try and help me now?

tristan369:
I need to make an array of words, how would this be done with c-strings? Thank you.

May not be 100% applicable but, here’s a demo from a past thread. The strings are stored in flash to save RAM.

const char*  const dictionary[3][8] PROGMEM = {
  {"Account", "Date and Time", "Idle", "Language", "Main Menu", "Prescription", "Settings", "Sound"},
  {"Cuenta", "Fecha y Hora", "Espera", "Idioma", "Menu Principal", "Receta", "Configuracion", "Sonido"},
  {"Compte", "Date et heure", "Idle", "Langue", "Main Menu", "prescription", "Parametres", "Son"}
};


void setup() {

  Serial.begin(115200);
  Serial.println("Setup");

  int numRows = sizeof(dictionary) / sizeof(dictionary[0]);
  Serial.print("Rows: ");
  Serial.println(numRows);

  int numCols = sizeof(dictionary[0]) / sizeof(dictionary[0][0]);
  Serial.print("Cols: ");
  Serial.println(numCols);

  for (int i = 0; i < numCols; i++) {
    char * message;
    Serial.print("Entry: ");
//    Serial.println(dictionary[0][i]);
   char *x = (char *)pgm_read_word(&(dictionary[1][i]));
    Serial.println (x );
  }
}

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

In the next-to-last line change the first dimension index of dictionary to select the language printed.

An all RAM version would be similar, just without the references/accesses to flash memory.

tristan369:
How is this done?I need to make an array of words, how would this be done with c-strings? Thank you.

Instead of:

String unitTypes[6] = {"Infantry", "Spearmen", "Archers", "Cavalry", "Peasants", "see army"};

Use:

char unitTypes[6][9] = {"Infantry\0", "Spearmen\0", "Archers\0", "Cavalry\0", "Peasants\0", "see army\0"};

Note the \0 at the end of each bit of text, this is to ensure there is a null terminator (0x00) inserted at the end of each one.
I don't know if your code will work with the above exactly as it is or if it will need to be modified.
Note I put [6][9] to define the size of the array, [9] is 8 letters (make sure I have not miscounted) plus the null terminator.