Arduino uno Internal EEPROM Question !!

Hello Friends

I am working on a simple sketch that to write and to read data to the internal uno EEPROM

the problem that when I save the first Variable fore the address one , I read it correctly
until when I save the second Variable .......
then the two Variables Become the same !!!

#include <Keypad.h>
#include<LiquidCrystal.h>
#include<EEPROM.h>
LiquidCrystal lcd(7, 8, 9, 10, 11, 12);
/////////////////////////////////////////////////////////////////////
char customKey=0;                  // CustomKey Pad val
char customKey2=0;                // CustomKey2 Pad val

int j=0;                         // price1 address TO EEPROM
char eep1[5];                   // price1 val TO EEPROM
char* p1=EEPROM.get(j,eep1);   // Read from Price1 EEPROM
float par=atof (p1);        // EEPROM Char to float  Price1
float prize=par*10;          // Price1 DEC to INT

int r=0;                         // price2 address TO EEPROM
char eep2[5];                   // price2 val TO EEPROM
char* p2=EEPROM.get(r,eep2);   // Read from Price2 EEPROM
float par2=atof (p2);        // EEPROM Char to float  Price2
float prize2=par2*10;          // Price2 DEC to INT


const byte ROWS = 4;
const byte COLS = 4;
char Keys[ROWS][COLS] = {
  {'1','2','3','A'},
  {'4','5','6','B'},
  {'7','8','9','C'},
  {'.','0','#','D'}
};
byte rowPins[ROWS] = {3,2,A5,A4}; //connect to the row pinouts of the keypad
byte colPins[COLS] = {A3,A2,A1,A0}; //connect to the column pinouts of the keypad
//initialize an instance of class NewKeypad
Keypad myKeypad = Keypad( makeKeymap(Keys), rowPins, colPins, ROWS, COLS);

/////////////////////////////////////////////////////////////////////
void setup(){
Serial.begin(9600);
lcd.begin(16, 2); 
lcd.print("Funy Test");
lcd.setCursor(0,1);
lcd.print(prize);     //to print the price1 as int on the screen
lcd.setCursor(8,1);
lcd.print(prize2);   //to print the price1 as int on the screen
}
/////////////////////////////////////////////////////////////////////

void(*resetFunc)(void)=0;

void loop(){
char key = myKeypad.getKey();
   if (key != NO_KEY){
      delay(60); 
      switch (key){
      case 'A': menu();break; 
      case 'B': break; 
      case 'C': break; 
      case 'D': break; 
      case '#': break;
      case '.': break;
      }}
 }
/////////////////////////////////////////////////////////////////////

void price1(){
  lcd.clear();
  lcd.print("A4 Price:");
  lcd.setCursor(0,1);
  j;
  while(j<5){
  char customKey = myKeypad.getKey();
  if((customKey=='1')or(customKey=='2')or(customKey=='3')or(customKey=='4')or(customKey=='5')or(customKey=='6')
  or(customKey=='7')or(customKey=='8')or(customKey=='9')or(customKey=='0')or(customKey=='.')){
  eep1[j]=customKey;
  lcd.print(customKey);
  EEPROM.update(j,eep1[j]);
  j++;
  }
  else if(customKey=='B'){
  menu();
  break;}
  }
  lcd.print(" >> Done...");
  delay(2000);
  resetFunc(); // I use the reset function to update the new entry on lcd
}

/////////////////////////////////////////////////////////////////////

void price2(){
  lcd.clear();
  lcd.print("A3 Price:");
  lcd.setCursor(0,1);
   r;
  while(r<5){
  char customKey2 = myKeypad.getKey();
  if((customKey2=='1')or(customKey2=='2')or(customKey2=='3')or(customKey2=='4')or(customKey2=='5')or(customKey2=='6')
  or(customKey2=='7')or(customKey2=='8')or(customKey2=='9')or(customKey2=='0')or(customKey2=='.')){
  eep2[r]=customKey2;
  lcd.print(customKey2);
  EEPROM.update(r,eep2[r]);
  r++;
  }
  else if(customKey2=='B'){
  menu();
  break;}
  }
  lcd.print(" >> Done...");
  delay(2000);
  resetFunc(); // I use the reset function to update the new entry on lcd
}

/////////////////////////////////////////////////////////////////////

void counter(){
  lcd.clear();
  lcd.print ("Total:");
}
/////////////////////////////////////////////////////////////////////
void menu(){
lcd.clear();
lcd.print ("[1]A4");
lcd.setCursor(0,1);
lcd.print ("[2]A3");
lcd.setCursor(6,0);
lcd.print ("[3]Counter");
lcd.setCursor(6,1);
lcd.print ("[B]Exit");
int c=1;
while(c){
char customKey = myKeypad.getKey();
if(customKey=='1')
  price1();
  if(customKey=='2')
  price2();
  if(customKey=='3')
  counter();
  else if(customKey=='B'){
  setup();
  break;
  }
}
}
/////////////////////////////////////////////////////////////////////

Sooooooo where is the problem??????

Please post your code as one piece; it's easier to copy. Also, currently something like resetFunc() seems to be missing. And also make sure that you have all curlies in there.

So some comments

void price1() {
  lcd.clear();
  lcd.print("Funy Price2:");
  lcd.setCursor(0, 1);
  j;
  ...
  ...

Should 'j' not have a type? And should it not be initialized to zero? Same for 'r' in price2.

And why do you have something like this?

void price1() {
  ...
  ...
  resetFunc();
  setup();
}

Without knowing resetFunc() it's not quite easy to say what happens. But I suspect that setup() will not be called. If resetFunc resets the Arduino, that call to setup() will never be executed.

In this code fragment:

  if((customKey=='1')or(customKey=='2')or(customKey=='3')or(customKey=='4')or(customKey=='5')or(customKey=='6')
  or(customKey=='7')or(customKey=='8')or(customKey=='9')or(customKey=='0')or(customKey=='.')){

C doesn't recognize the word "or". Also, you could simplify your code a little using the isdigit() macro:

   if (customKey == '.' || isdigit(customKey) ) {
      // do the processing
   }

econjack:
C doesn't recognize the word "or". Also, you could simplify your code a little using the isdigit() macro:

good since we are programming in C++ and operator synonyms have been around for a while

bool BullddogIsright = true;
bool econJackIsWrong = true;

void setup() 
{
  Serial.begin(9600);
  if(BullddogIsright or econJackIsWrong)
  {
    Serial.println(F("A slice of humble pie, perhaps?"));
  }
}

void loop() 
{

}

compiled && tested :wink:

thanks all I am new with Arduino and c++
just if any one can help
when I save eeprom price1
the price2 get the same like price1
and when saving the price2
the price1 get the same like price2

and I need reset function to update the new entry and to get it on the lcd

this is the code

#include <Keypad.h>
#include<LiquidCrystal.h>
#include<EEPROM.h>
LiquidCrystal lcd(7, 8, 9, 10, 11, 12);
/////////////////////////////////////////////////////////////////////
char customKey=0;                  // CustomKey Pad val
char customKey2=0;                // CustomKey2 Pad val

int j=0;                         // price1 address TO EEPROM
char eep1[5];                   // price1 val TO EEPROM
char* p1=EEPROM.get(j,eep1);   // Read from Price1 EEPROM
float par=atof (p1);        // EEPROM Char to float  Price1
float prize=par*10;          // Price1 DEC to INT

int r=0;                         // price2 address TO EEPROM
char eep2[5];                   // price2 val TO EEPROM
char* p2=EEPROM.get(r,eep2);   // Read from Price2 EEPROM
float par2=atof (p2);        // EEPROM Char to float  Price2
float prize2=par2*10;          // Price2 DEC to INT


const byte ROWS = 4;
const byte COLS = 4;
char Keys[ROWS][COLS] = {
  {'1','2','3','A'},
  {'4','5','6','B'},
  {'7','8','9','C'},
  {'.','0','#','D'}
};
byte rowPins[ROWS] = {3,2,A5,A4}; //connect to the row pinouts of the keypad
byte colPins[COLS] = {A3,A2,A1,A0}; //connect to the column pinouts of the keypad
//initialize an instance of class NewKeypad
Keypad myKeypad = Keypad( makeKeymap(Keys), rowPins, colPins, ROWS, COLS);

/////////////////////////////////////////////////////////////////////
void setup(){
Serial.begin(9600);
lcd.begin(16, 2); 
lcd.print("Funy Test");
lcd.setCursor(0,1);
lcd.print(prize);     //to print the price1 as int on the screen
lcd.setCursor(8,1);
lcd.print(prize2);   //to print the price1 as int on the screen
}
/////////////////////////////////////////////////////////////////////

void(*resetFunc)(void)=0;

void loop(){
char key = myKeypad.getKey();
   if (key != NO_KEY){
      delay(60); 
      switch (key){
      case 'A': menu();break; 
      case 'B': break; 
      case 'C': break; 
      case 'D': break; 
      case '#': break;
      case '.': break;
      }}
 }
/////////////////////////////////////////////////////////////////////

void price1(){
  lcd.clear();
  lcd.print("A4 Price:");
  lcd.setCursor(0,1);
  j;
  while(j<5){
  char customKey = myKeypad.getKey();
  if((customKey=='1')or(customKey=='2')or(customKey=='3')or(customKey=='4')or(customKey=='5')or(customKey=='6')
  or(customKey=='7')or(customKey=='8')or(customKey=='9')or(customKey=='0')or(customKey=='.')){
  eep1[j]=customKey;
  lcd.print(customKey);
  EEPROM.update(j,eep1[j]);
  j++;
  }
  else if(customKey=='B'){
  menu();
  break;}
  }
  lcd.print(" >> Done...");
  delay(2000);
  resetFunc(); // I use the reset function to update the new entry on lcd
}

/////////////////////////////////////////////////////////////////////

void price2(){
  lcd.clear();
  lcd.print("A3 Price:");
  lcd.setCursor(0,1);
   r;
  while(r<5){
  char customKey2 = myKeypad.getKey();
  if((customKey2=='1')or(customKey2=='2')or(customKey2=='3')or(customKey2=='4')or(customKey2=='5')or(customKey2=='6')
  or(customKey2=='7')or(customKey2=='8')or(customKey2=='9')or(customKey2=='0')or(customKey2=='.')){
  eep2[r]=customKey2;
  lcd.print(customKey2);
  EEPROM.update(r,eep2[r]);
  r++;
  }
  else if(customKey2=='B'){
  menu();
  break;}
  }
  lcd.print(" >> Done...");
  delay(2000);
  resetFunc(); // I use the reset function to update the new entry on lcd
}

/////////////////////////////////////////////////////////////////////

void counter(){
  lcd.clear();
  lcd.print ("Total:");
}
/////////////////////////////////////////////////////////////////////
void menu(){
lcd.clear();
lcd.print ("[1]A4");
lcd.setCursor(0,1);
lcd.print ("[2]A3");
lcd.setCursor(6,0);
lcd.print ("[3]Counter");
lcd.setCursor(6,1);
lcd.print ("[B]Exit");
int c=1;
while(c){
char customKey = myKeypad.getKey();
if(customKey=='1')
  price1();
  if(customKey=='2')
  price2();
  if(customKey=='3')
  counter();
  else if(customKey=='B'){
  setup();
  break;
  }
}
}
/////////////////////////////////////////////////////////////////////

thanks all

 lcd.setCursor(0,1);
  j;

Evaluate j, then throw away the result.
Did you forget something?

AWOL:

 lcd.setCursor(0,1);

j;



Evaluate j, then throw away the result.
Did you forget something?

Ammmm not understanding your point

mkdirs:
Ammmm not understanding your point

What does
j;do?

aarg:
What does
j;do?

for nothing forget to clear (_)

friends I can write to eeprom and read ,when I write price value with keypad I can read it
but the same value be the second price read value

mkdirs:
for nothing forget to clear (_)

friends I can write to eeprom and read ,when I write price value with keypad I can read it
but the same value be the second price read value

Are you ensuring the index you are reading the EEPROM with is correct. Also if the type of data you are writing is more than one byte, you'll need to increase your index by the same amount to ensure you aren't overlapping different writes.

@BulldogLowell. Ummm...good humble pie! I had forgotten about the C++ macros. Had I seen the iso646.h header file might have remembered...and, maybe not...I'm old.

mkdirs:
and I need reset function to update the new entry and to get it on the lcd

No, you don't need it. It's ultimate nonsense.

pYro_65:
Are you ensuring the index you are reading the EEPROM with is correct. Also if the type of data you are writing is more than one byte, you'll need to increase your index by the same amount to ensure you aren't overlapping different writes.

yes I read the correct data
and the data type 11111 or 00.00 that's it some thing like that

I know what's wrong. But I'm not going to tell you until you give all the variables meaningful names, and repost it. I was going to ignore it, as you ignored my signature message and the forum guidelines and messaged code to me. But it is so simple, that you deserve a chance. Actually, I think that if you do give the variables meaningful names, you will spot the error yourself. Here is an example of a completely meaningless variable name:

int j=0;                         // price1 address TO EEPROM

Here is a meaningful variable name:

int price1AddressEE = 0;                         // price1 address TO EEPROM

An excerpt of your menu code

    else if (customKey == 'B') {
      setup();
      break;
    }

Instead of calling setup(), why not return to loop(), retrieve the stored data and display it

    else if (customKey == 'B') {
      return;
    }

And in loop()

void loop() {
  // read your prices from eeprom
  ...
  ...
  // display prices on LCD

  // your original code below
  char key = myKeypad.getKey();
  if (key != NO_KEY) {
    delay(60);
    switch (key) {
      case 'A': menu(); break;
      case 'B': break;
      case 'C': break;
      case 'D': break;
      case '#': break;
      case '.': break;
    }
  }
}

Note: I'm not sure of the side effect of calling Serial.begin and lcd.begin multiple times as is happening with your current code.

An other problem

You call menu() to display a menu. From there you call e.g. price1(). In price1() you call menu() again. In there you call either price1() or price2() again and so on and so on.

Do that a number of times and you will run out of memory causing all kinds of funny behaviour :wink:

You should return to menu once you're done with price1() or price2(). resetFunc() saves you in this case but it's a stupid way. Look at resetFunc() this way: it's like changing the channel on a TV and having to do a power cycle to make the change take affect.

Below would be an updated version of price2(); it's the same as yours with the omission of resetFunc().

void price2() {
  lcd.clear();
  lcd.print("A3 Price:");
  lcd.setCursor(0, 1);
  r;
  while (r < 5) {
    char customKey2 = myKeypad.getKey();
    if ((customKey2 == '1') or (customKey2 == '2') or (customKey2 == '3') or (customKey2 == '4') or (customKey2 == '5') or (customKey2 == '6')
        or (customKey2 == '7') or (customKey2 == '8') or (customKey2 == '9') or (customKey2 == '0') or (customKey2 == '.')) {
      eep2[r] = customKey2;
      lcd.print(customKey2);
      EEPROM.update(r, eep2[r]);
      r++;
    }
    else if (customKey2 == 'B') {
      menu();
      break;
    }
  }
  lcd.print(" >> Done...");
  delay(2000);
}

And the menu can then be like below

void menu() {
  // flag to indicate if menu must be displayed
  bool fRefresh = true;

  int c = 1;

  while (c)
  {
    if (fResfresh == true)
    {
      lcd.clear();
      lcd.print ("[1]A4");
      lcd.setCursor(0, 1);
      lcd.print ("[2]A3");
      lcd.setCursor(6, 0);
      lcd.print ("[3]Counter");
      lcd.setCursor(6, 1);
      lcd.print ("[B]Exit");

      // indicate that menu is displayed and does not need refresh
      fRefresh = false;
    }
    char customKey = myKeypad.getKey();

    if (customKey == '1')
    {
      price1();
      // indicate that menu must be refreshed
      fRefresh = true;
    }
    if (customKey == '2')
    {
      price2();
      // indicate that menu must be refreshed
      fRefresh = true;
    }
    if (customKey == '3')
    {
      counter();
      // indicate that menu must be refreshed
      fRefresh = true;
    }
    else if (customKey == 'B') {
      return;
    }
  } // end_of_while (c)
}

It uses a boolean flag to indicate if the menu needs refresh (e.g. after entry of a price). Because the displaying of the menu is now inside the while(c), it prevents flickering of the display.

Hope this helps.

@sterretje:

Given the way the switch/case blocks work, wouldn't it be more efficient to use:

    char customKey = toupper(myKeypad.getKey());
    switch (customeKey) {
       case '1':
           price1();
           fRefresh = true;
           break;

       case '2':
           price2();
           fRefresh = true;
           break;

       case '3':
           counter();
           fRefresh = true;
           break;

       case 'B':
           return;
 
       default:
           Serial.print("I shouldn't be here. customKey = ");
           Serial.println(customKey);
           break;

    }

The use of the toupper() macro would prevent failure if they entered a lower case 'b'. Numerics pass through unaffected. Because the switch statement produces a jump table, it's faster than falling through a series of nested if blocks.

@econjack

I tried to base the code as much as possible on the original. Little steps so it's easier to follow the changes. I did consider it as well as the fact that there are a few ifs instead of elseifs.

I would indeed have used a switch/case for this, just because it feels cleaner.

No idea about efficiency; it's something that I can't judge with my current knowledge.

@econjack and @sterretje

thanks so much that help me and all is ok now