Help with memory corruption problem

First off I am fairly new at this but have worked through a number of minor projects. This is my first somewhat larger project.
First a bit of background on the project. I am creating a tank monitoring solution to monitor the fluid level in up to 4 holding tanks. The sensors used to monitor the tanks return a variable voltage between 0 and 5 volts. I have set up 4 analog inputs to read the levels. I use 2 buttons to execute the required functions. I call them “mode” and “set”.
As part of the system initialization, I determine the number of connected tanks and then in several nested loops pick a name from a preconfigured array (tankDescript) and assign it to a tank. At the same time I write the chosen name to EEPROM to save it through power cycles or resets.
This all works perfectly as long as I only have 4 names in the tankDescript array. As soon as I add more than 4 names to the array, things get hairy in that I seem to get corruption and system freezes as it runs through the loops.
I am using an array technique I found here by nickgammon https://forum.arduino.cc/index.php?topic=331337.0 post #4.
I have attached my code below. Sorry if it a bit long but is stripped to the minimum I could while still making it functional.
Any insight welcome. BTW this is running on a Nano V3.
Thanks

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

LiquidCrystal_I2C lcd(0x27, 20, 4);

Button modeButton(2, 50);       // create Button object that attach to pin 2;
Button setButton(3, 50);        // create Button object that attach to pin 3;
const int tankSensor[] = { 14, 15, 16, 17 };    // Tank sensor inputs
const int LED = 13;         // Built in LED

char tankName[4][10];
String tankType[4];
int tankNumber = 0;
int numberOfTanks = 0;

const int numberOfDescripts = 5;
const int maxSize = 7;
char tankDescript [numberOfDescripts] [maxSize] = {
  { "Fresh " },
//  { "Fresh2" },
  { "Galley" },
  { "Bath  " },
  { "Grey  " },
  { "Black " },
};

void setup() {

  // initialize the LCD and Console
  lcd.init();
  lcd.backlight();
  Serial.begin(9600);

  modeButton.begin();
  setButton.begin();
  // initialize the LED pin as an output.
  pinMode(LED, OUTPUT);

  for (int index = 0; index < 4; index++) {
    pinMode(tankSensor[index], INPUT_PULLUP);
  }

  // Wipe EEPROM memory if both Mode and Set buttons are pressed.
  // The only time this can be done is immediately after a reset.
  if(modeButton.read() && setButton.read()) {
    int i;
    for (i = 0; i < 60; i++) {
      EEPROM.write(i, 0);
    }
    while (i) {  // Flash the built in LED when done
      digitalWrite(LED, HIGH);
      delay(500);
      digitalWrite(LED, LOW);
      delay(500);
      lcd.setCursor(0,1);
      lcd.print(" Initialized. Press");
      lcd.setCursor(0,2);
      lcd.print(" RESET to continue.");
      
    }
  }

  for (int t = 0; t < 4; t++) {  // Determine how many tanks are connected. Max 4.
    if (analogRead(t) < 1001) numberOfTanks++;
  }
  if (numberOfTanks == 0) {
        lcd.setCursor(0, 0);
        lcd.print("No tanks detected");
        lcd.setCursor(0, 1);
        lcd.print("Connect a tank");
        lcd.setCursor(0, 2);
        lcd.print("and press 'Reset'");
        bool i = true;
        while (i);
  }
  int knownTanks = EEPROM.read(17); // EEPROM location 17 contains the number of known tanks
  if (knownTanks < numberOfTanks) { // If new tanks detected, configure them
        lcd.print(numberOfTanks);
        lcd.print(" tanks detected.");
        lcd.setCursor(0, 1);
        lcd.print(numberOfTanks - knownTanks);
        lcd.print(" new tanks detected");
        tankNumber = 0;
        knownTanks = numberOfTanks;  // Update the EEPROM with the new knownTanks
        EEPROM.write(17, knownTanks);

        // tankNumber is the number of the tank being programmed
        while (tankNumber < numberOfTanks) {
              lcd.setCursor(0, 2);
              lcd.print("Select tank #");
              lcd.print(tankNumber + 1);
              lcd.print(" type");

              bool buttonPress = false;
              bool set = false;
              int t = 0;  // ''t' is an index to the name of the tank being programmed

              while (t < numberOfDescripts) {
                    lcd.setCursor(0, 3);
                    lcd.print(tankDescript[t]);

                     // The Mode button will step through the tank types. When the proper tank
                     // type is displayed, pressing the Set button will lock it in.
                     while (!buttonPress) { // Wait for a button press
                           modeButton.read();
                           setButton.read();
                           if (modeButton.wasReleased()) {
                                 Serial.println("Mode pressed");
                                 buttonPress = true;
                           }

                           if (setButton.wasReleased()) {
                                 Serial.println("Set pressed");
                                 int ADDR = 20 + (tankNumber * 10);  // Memory locations 20 -> 59
                                 tankType[t] = tankDescript[t];
                                 EEPROM.put(ADDR, tankDescript[t]);
                                 Serial.print("Wrote ");
                                 Serial.print(tankDescript[t]);
                                 Serial.print(" at location ");
                                 Serial.println(ADDR);
                                 t = 0;
                                 set = true;
                                 buttonPress = true;
                           } // End if (setButton.released())

                     } // End while (!buttonPress)
                     buttonPress = false;
                     t++;
                     if (set) t = numberOfDescripts; // Exit while (t < numberOfDescripts)
                     
              } // end of while (t < numberOfDescripts)
              if (set) tankNumber++;

        } // end of while (tankNumber < numberOfTanks)

  } // end of if (knownTanks < numberOfTanks)

  Serial.println("Setup Complete");
  lcd.clear();
  lcd.print(" Complete ");
}  // end of setup
void loop() {

} // End of void(loop)
String tankType[4];

What happens here when numberOfDescripts is greater than 4?

tankType[t] = tankDescript[t];

This isn't good:

           tankType[t] = tankDescript[t];

t iterates through the number of descriptions, of which there are five. You use that to index tankType, which only has four. A crash is likely.

wildbill:
This isn't good:

           tankType[t] = tankDescript[t];

t iterates through the number of descriptions, of which there are five. You use that to index tankType, which only has four. A crash is likely.

Ah! Yes, that is a strong possibility. I will have to look at that one. Been staring at this so long, the obvious becomes anything but!
Thanks :slight_smile:

kamflyer:
Ah! Yes, that is a strong possibility. I will have to look at that one. Been staring at this so long, the obvious becomes anything but!
Thanks :slight_smile:

This is where you should be using serial-print(t) to display the values you are using.
Paul

Thanks guys! Working great now.
-Rob-

The use of String may result in memory corruption over time. I wrote a sketch for a Nano and used String. After several weeks it hang or started to behave strange. I fixed it by dropping the use of String types.

Use the F() macro for printing literal strings. The F() macro tells the compiler to keep your strings in PROGMEM. All you have to do is to enclose the literal string in the F() macro.

Change:

Serial.print("This is a literal String");

to

Serial.print(F("This is a literal String"));

This will save a few bytes.

The use of String may result in memory corruption over time.

Some simple guidelines can avoid that. This is from the String tutorial I am working on at the moment.

To use Arduino Strings with the minimum of fuss,

  1. Declare long lived Strings as globals and reserve space in setup() and add a StringReserveCheck to check that the reserve was enough.

  2. If you have created Strings in the loop() method, they are long lived, move them to Globals and do step 1.

  3. Pass all Strings arguments to methods, as const String &. For results pass a String &result, that the method can update with the result. i.e. void strProcessing(const String &input1, const String &input2, , String &result) {…}

  4. Do string processing in small compact methods. In these methods use local Strings in preference to local char. These methods will completely recover all the heap and stack used by their local Strings and other variables on exit.

5 )Set the IDE File → Preferences Compiler Warning to ALL and watch for warning: passing ‘const String’ as ‘this’ argument discards qualifiers [-fpermissive] messages in the compile window. This says you are trying to modify a const String& arg.

  1. Do not use the String + operator at all and avoid using the String(…) constructors as these create short lived temporary Strings. Use = and += operators or concat( ), as in result += “str”; result += 5; result += number; etc

  2. If your Arduino program uses the Web (e.g. ESP32 / EPS8266 webserver/httpClient) and is suppose to run for a long time, you have bigger problems then just your code. Jump to Programming Automatic Reboots.

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.