EEPROM Misreading (solved)

Ok, so I built a project to read the entries/exits into our school library. It remembers the count and it 11:30PM each night it should save that information....

Well I tested it by changing the clock time and it worked first time. It saved the MM/DD/YY | Count.
So i ran it again, it went to next set of addressing and saved same information.... But wait now the previous count number has change =(
if i write again it does the same thing, the new number is correct and all previous are wrong.....
I tried separating the data with an extra bit and it still did it. I'm not sure if its me or an issue with avr/eeprom.h .
any ideas appreciated.

and here is the code. (this is just the part dealing with writing to eeprom. The entire code is over 40 pages....) Good thing its a simple counter right? :stuck_out_tongue:

If you want my current full code i can send it to you. only 33kb, ill post pics of it later I have to leave for a meeting.

void saveTheCount(){         // this should only run once per day
 
 if(savecount == 1 && saved != 1){
   
  Wire.beginTransmission(DS1307_ADDRESS);
  Wire.write(zero);
  Wire.endTransmission();
  
  Wire.requestFrom(DS1307_ADDRESS, 7);
  
  int second = bcdToDec(Wire.read());
  int minute = bcdToDec(Wire.read());
  int hour = bcdToDec(Wire.read() & 0b111111);    //24 hour time
  int weekDay = bcdToDec(Wire.read());   //0-6 -> sunday - saturday
  int monthDay = bcdToDec(Wire.read());
  int month = bcdToDec(Wire.read());
  int year = bcdToDec(Wire.read());
    
    
    savedAddress = eeprom_read_word((uint16_t*)10);      //read address for starting position
    
    // saves in mm/dd/yy|count       order
      eeprom_write_byte((uint8_t*)savedAddress, month);                //eg address 100
      eeprom_write_byte((uint8_t*)savedAddress+1, monthDay);           //101
      eeprom_write_byte((uint8_t*)savedAddress+2, year);               //102
      eeprom_write_word((uint16_t*)savedAddress+3, counter);           //103 and 104 since its a word (2 bytes)

savedAddress = savedAddress + 5;                   // increment address and prepare to save

  if(savedAddress >= 4090){                        //rollover to starting address 100
   savedAddress = 100;
  }
   eeprom_write_word((uint16_t*)10, savedAddress);
   
//lets add offset to second and save to update clock

second = 1 + offset + second;   //not sure how long the time interval is from the last read to now so add 1 second,
                                //since I know this DS1307 is 2-6s off per day :(  (tested already)
                                
//this is overkill but ya never know
if(second >= 61){    //just incase seconds ends up at 61+
  second = second - 60;
  minute = minute + 1;  // only runs at around min 30-32 so no need to carry to hours
}
//end overkill
  
  
  Wire.beginTransmission(DS1307_ADDRESS);
  Wire.write(zero);  //stop oscillator
  Wire.write(decToBcd(second));
  Wire.write(decToBcd(minute));
  Wire.write(decToBcd(hour));
  Wire.write(decToBcd(weekDay));
  Wire.write(decToBcd(monthDay));
  Wire.write(decToBcd(month));
  Wire.write(decToBcd(year));
  
  Wire.write(zero);  //start
  
  Wire.endTransmission();
   
    saved = 1;             // this way it only runs once per day, after that it is returned 
                           // to 0 a few minutes later after savecount statement is no longer true
    
}
}

LRC_Restartmegawbackup3.ino (31.9 KB)

eeprom_read_word((uint16_t*)10);

This looks funky. One does not generally want to cast a constant as a pointer! There is no good reason for eeprom_read_word() to need a pointer. Perhaps you should check the eeprom functions to se if they write through those pointers. If they do then that could all sorts of oddness.

savedAddress = eeprom_read_word((uint16_t*)10);

eeprom_write_word((uint16_t*)10, savedAddress);

im using this to set the starting point for the next address save. and its how the tutorials and the library info says it should appear.
it just tells the mega where to start next time it saves the date/count. im using first 100 bytes of eeprom for settings and the rest for date/count data.
1 byte for month(9), 1 byte for day(9), 1 byte for year(12), 1 word for count (up to 9999)

which seems to work just fine from what i can tell, im saving rgb settings for an lcd, and a few other things with eeprom.

here is the part that displays the information on an lcd menu.
yes it could be alot smaller, this is like my version 0.5

If i can get it to save properly I can hang it at school and work on refining and other coding.

if(page == 3){

if(s == 0){     //startup purposes, dont want it to point to addressing not being used for counter
  s = savedAddress;        
}

  
    if(pointer1 >= 4){
      pointer1 = 1;
       } 
        if(pointer1 <= 0){
          pointer1 = 3;  
            }
            
            
  if(buttondepress == 1){
    if(buttonpress == 1 && pointer1 == 1){  //moving forward on dates
     s = s + 35; 
     buttonpress = 0;
     if(s >=4086){
        s = 130;
        }}
    if(buttonpress == 1 && pointer1 == 2){
     s = s - 35;
     buttonpress = 0;
     if(s <= 129){
        s = 4085;
        }} 
      
  if(buttonpress == 1 && pointer1 == 3){
    page = 0;
    buttonpress = 0;
    s = 0;
  }}
  
  
if(s <= 99){   
 s = 4060;        
} 
if(s >=4061){
  s = 100;
}  

uint8_t m;  //month    //  these change during each run
uint8_t d;  //day
uint8_t y;  //year
uint16_t c; //count

   
itoa (s, array, 10);               //debugging
glcd.drawstring(88, 0, array);     //debugging

  //////////First run
      m = eeprom_read_byte((uint8_t*)s);         
      d = eeprom_read_byte((uint8_t*)s+1);      
      y = eeprom_read_byte((uint8_t*)s+2);       
      c = eeprom_read_word((uint16_t*)s+3);

itoa (m, array,10);
glcd.drawstring(0, 1, array);
  itoa (d, array,10);
  glcd.drawstring(12,1, "/");
  glcd.drawstring(18, 1, array);
    itoa (y, array,10);
    glcd.drawstring(30,1, "/");
    glcd.drawstring(36, 1, array);
      itoa (c, array,10);
      glcd.drawstring(52,1, "|");
      glcd.drawstring(57, 1, array);
s = s + 5;


  
  //////////Second run
      m = eeprom_read_byte((uint8_t*)s);         
      d = eeprom_read_byte((uint8_t*)s+1);      
      y = eeprom_read_byte((uint8_t*)s+2);       
      c = eeprom_read_word((uint16_t*)s+3);

itoa (m, array,10);
glcd.drawstring(0, 2, array);
  itoa (d, array,10);
  glcd.drawstring(12,2, "/");
  glcd.drawstring(18, 2, array);
    itoa (y, array,10);
    glcd.drawstring(30,2, "/");
    glcd.drawstring(36, 2, array);
      itoa (c, array,10);
      glcd.drawstring(52,2, "|");
      glcd.drawstring(57, 2, array);
s = s + 5;

  
  
  //////////Third run
      m = eeprom_read_byte((uint8_t*)s);         
      d = eeprom_read_byte((uint8_t*)s+1);      
      y = eeprom_read_byte((uint8_t*)s+2);       
      c = eeprom_read_word((uint16_t*)s+3);

itoa (m, array,10);
glcd.drawstring(0, 3, array);
  itoa (d, array,10);
  glcd.drawstring(12,3, "/");
  glcd.drawstring(18, 3, array);
    itoa (y, array,10);
    glcd.drawstring(30,3, "/");
    glcd.drawstring(36, 3, array);
      itoa (c, array,10);
      glcd.drawstring(52,3, "|");
      glcd.drawstring(57, 3, array);
s = s + 5;



  //////////Forth run
      m = eeprom_read_byte((uint8_t*)s);         
      d = eeprom_read_byte((uint8_t*)s+1);      
      y = eeprom_read_byte((uint8_t*)s+2);       
      c = eeprom_read_word((uint16_t*)s+3);

itoa (m, array,10);
glcd.drawstring(0, 4, array);
  itoa (d, array,10);
  glcd.drawstring(12,4, "/");
  glcd.drawstring(18, 4, array);
    itoa (y, array,10);
    glcd.drawstring(30,4, "/");
    glcd.drawstring(36, 4, array);
      itoa (c, array,10);
      glcd.drawstring(52,4, "|");
      glcd.drawstring(57, 4, array);
s = s + 5;



  //////////Fifth run
      m = eeprom_read_byte((uint8_t*)s);         
      d = eeprom_read_byte((uint8_t*)s+1);      
      y = eeprom_read_byte((uint8_t*)s+2);       
      c = eeprom_read_word((uint16_t*)s+3);

itoa (m, array,10);
glcd.drawstring(0, 5, array);
  itoa (d, array,10);
  glcd.drawstring(12,5, "/");
  glcd.drawstring(18, 5, array);
    itoa (y, array,10);
    glcd.drawstring(30,5, "/");
    glcd.drawstring(36, 5, array);
      itoa (c, array,10);
      glcd.drawstring(52,5, "|");
      glcd.drawstring(57, 5, array);
s = s + 5;


  //////////Sixth run
      m = eeprom_read_byte((uint8_t*)s);         
      d = eeprom_read_byte((uint8_t*)s+1);      
      y = eeprom_read_byte((uint8_t*)s+2);       
      c = eeprom_read_word((uint16_t*)s+3);

itoa (m, array,10);
glcd.drawstring(0, 6, array);
  itoa (d, array,10);
  glcd.drawstring(12,6, "/");
  glcd.drawstring(18, 6, array);
    itoa (y, array,10);
    glcd.drawstring(30,6, "/");
    glcd.drawstring(36, 6, array);
      itoa (c, array,10);
      glcd.drawstring(52,6, "|");
      glcd.drawstring(57, 6, array);
s = s + 5;



  //////////Seventh run
      m = eeprom_read_byte((uint8_t*)s);         
      d = eeprom_read_byte((uint8_t*)s+1);      
      y = eeprom_read_byte((uint8_t*)s+2);       
      c = eeprom_read_word((uint16_t*)s+3);

itoa (m, array,10);
glcd.drawstring(0, 7, array);
  itoa (d, array,10);
  glcd.drawstring(12,7, "/");
  glcd.drawstring(18, 7, array);
    itoa (y, array,10);
    glcd.drawstring(30,7, "/");
    glcd.drawstring(36, 7, array);
      itoa (c, array,10);
      glcd.drawstring(52,7, "|");
      glcd.drawstring(57, 7, array);

s = s - 35;




  
  
glcd.drawstring(0, 0, "Count History");    //display 7 days of a week, will be able to look back several weeks if not more
                                            //mega has 4096 KB to work with, using addresses 100-4095 for counter
//glcd.drawstring(0, 1, "0432  06/14/12");     //example

glcd.drawstring(103, 2, "Next");
glcd.drawstring(103, 4, "Prev");
glcd.drawstring(103, 6, "Back");
glcd.drawrect(100, pointer1*16-2, 28, 11, BLACK);


glcd.display();

  glcd.clear();
    return;
}

1 byte for month(9), 1 byte for day(9), 1 byte for year(12), 1 word for count (up to 9999)

No, you're not:

  int monthDay = bcdToDec(Wire.read());
  int month = bcdToDec(Wire.read());
  int year = bcdToDec(Wire.read());

ints are two bytes each.

      eeprom_write_byte((uint8_t*)savedAddress, month);                //eg address 100
      eeprom_write_byte((uint8_t*)savedAddress+1, monthDay);           //101
      eeprom_write_byte((uint8_t*)savedAddress+2, year);               //102
      eeprom_write_word((uint16_t*)savedAddress+3, counter);           //103 and 104 since its a word (2 bytes)

You are casting the address to a byte, not the value.

Why you are casting the address to a byte sometimes and to an int other times is a mystery.

Hmm, ill have to dig deeper into it tonight.

The example codes I was coming off of where written in the same manners so I thought it was ok.
I tried changing some at one point with mixed results.
Always seems like you fix 1 thing and break another...

This part goes to an external RTC. Im using alot of different components in the build.

  int monthDay = bcdToDec(Wire.read());
  int month = bcdToDec(Wire.read());
  int year = bcdToDec(Wire.read());

Sorry for the long absence, long week :frowning:
Changed all the int's to bytes on the clock parts but no change.
also changed counter to uint16_t counter;

When i set the count to 373 it saves.
But only appears as 373 on newest run. Older numbers apear as 3101, and 3098. also parts of the next set of byte's that have not been writen to or should not have been appear changed.

void saveTheCount(){         // this should only run once per day
 
 if(savecount == 1 && saved == 0){
   
  Wire.beginTransmission(DS1307_ADDRESS);
  Wire.write(zero);
  Wire.endTransmission();
  
  Wire.requestFrom(DS1307_ADDRESS, 7);
  
  byte second = bcdToDec(Wire.read());
  byte minute = bcdToDec(Wire.read());
  byte hour = bcdToDec(Wire.read() & 0b111111);    //24 hour time
  byte weekDay = bcdToDec(Wire.read());   //0-6 -> sunday - saturday
  byte monthDay = bcdToDec(Wire.read());
  byte month = bcdToDec(Wire.read());
  byte year = bcdToDec(Wire.read());
    
    
    savedAddress = eeprom_read_word((uint16_t*)10);      //read address for starting position
  
    // saves in mm/dd/yy|count       order
      eeprom_write_byte((uint8_t*)savedAddress, month);                //eg address 100
      eeprom_write_byte((uint8_t*)savedAddress + 1, monthDay);           //101
      eeprom_write_byte((uint8_t*)savedAddress + 2, year);               //102
      eeprom_write_word((uint16_t*)savedAddress + 3, counter);           //103 and 104 since its a word (2 bytes)

savedAddress = savedAddress + 5;                   // increment address and prepare to save

  if(savedAddress >= 4090){                        //rollover to starting address 100
   savedAddress = 100;
  }
   eeprom_write_word((uint16_t*)10, savedAddress);
   
//lets add offset to second and save to update clock

second = 1 + offset + second;   //not sure how long the time interval is from the last read to now so add 1 second,
                                //since I know this DS1307 is 2-6s off per day :(  (tested already)
                                
//this is overkill but ya never know
if(second >= 61){    //just incase seconds ends up at 61+
  second = second - 60;
  minute = minute + 1;  // only runs at around min 30-32 so no need to carry to hours
}
//end overkill
  
  
  Wire.beginTransmission(DS1307_ADDRESS);
  Wire.write(zero);  //stop oscillator
  Wire.write(decToBcd(second));
  Wire.write(decToBcd(minute));
  Wire.write(decToBcd(hour));
  Wire.write(decToBcd(weekDay));
  Wire.write(decToBcd(monthDay));
  Wire.write(decToBcd(month));
  Wire.write(decToBcd(year));
  
  Wire.write(zero);  //start
  
  Wire.endTransmission();
   
    saved = 1;             // this way it only runs once per day, after that it is returned 
                           // to 0 a few minutes later after savecount statement is no longer true
    
}
}

this part is the part shown in the pics, it runs 7 times, once for each row on the screen

uint8_t m;  //month    //  these change during each run
uint8_t d;  //day
uint8_t y;  //year
uint16_t c; //count


itoa (savedAddress, array, 10);               //debugging
glcd.drawstring(88, 0, array);     //debugging

  //////////First run
      m = eeprom_read_byte((uint8_t*)savedAddress);         
      d = eeprom_read_byte((uint8_t*)savedAddress+1);      
      y = eeprom_read_byte((uint8_t*)savedAddress+2);       
      c = eeprom_read_word((uint16_t*)savedAddress+3);

itoa (m, array,10);
glcd.drawstring(0, 1, array);
  itoa (d, array,10);
  glcd.drawstring(12,1, "/");
  glcd.drawstring(18, 1, array);
    itoa (y, array,10);
    glcd.drawstring(30,1, "/");
    glcd.drawstring(36, 1, array);
      itoa (c, array,10);
      glcd.drawstring(52,1, "|");
      glcd.drawstring(57, 1, array);
savedAddress = savedAddress + 5;

any ideas welcome

btw
johnwasser, if i dont use uint16_t* it throws errors.

Donziboy2:
johnwasser, if i dont use uint16_t* it throws errors.

Then I suspect that the eeprom_read_byte() function is not doing what you expect it is doing. That would explain why the values you are getting out of EEPROM are not the same as the values you put in.

I would recommend getting rid of whatever EEPROM library you are using and trying the built-in EEPROM functions.

Im using <avr/eeprom.h>
If i use the built in eeprom functions can i use all 4kb's of eeprom?

Donziboy2:
Im using <avr/eeprom.h>

Those functions should work but are designed strangely. They require casting an int EEPROM address into an SRAM pointer. I'm sure they cast that number back to an int before using it.

What is causing your problem is the resulting pointer math. What you wrote is:

  eeprom_write_word((uint16_t*)savedAddress + 3, counter);

What the compiler sees is:

  eeprom_write_word(&((uint16_t*)savedAddress)[3], counter);

Because you are indexing a WORD pointer it adds 6 (three words worth of bytes) instead of 3.

To fix it, do the addition BEFORE the cast:

  eeprom_write_word((uint16_t*)(savedAddress + 3), counter);

Be sure to fix that wherever you read or write a word.

:astonished:
That seems to have fixed it....
I never even considered that cause the date information worked every time.
You sir are a coding god.