EEPROM::Update doesn't look right...

[dubnom]

The function needs to set _dirty if, and only if, the new data doesn't equal the existing data. It currently doesn't set the _dirty flag for any reason.

[cmaglie]

Can you post a sketch that shows the issue? It's kind of difficult to understand the problem from your description.

[dubnom]

Thanks for the quick response. Here is the definition of what “update” is supposed to do – Write a byte to the EEPROM. The value is written only if differs from the one already saved at the same address.

Here is what I think your code should look like – if (!_initialized) init(); if (read(address) != value) write(address, value);

This way the _dirty flag is only set whenever an actual value is changed. My guess is that the original EEPROM code looks almost identical to what I wrote.

[cmaglie]

aahh OK, now I understand. You're right the update function is totally wrong :astonished: I'm wondering how I have missed that...

I'm working on a fix...

[EduardoG26]

Hi dubnom, hi Christian. Great job. I suggest that write should do the same check as update. (See above). There is almost no penalty since in the case of emulation we are talking about some 4 cycles or so. In the case or real EEPROM with AVR it was a very good idea in it's day to implement different code for write and update. For compatibility reasons I would concentrate on update and let write simply call the same code. In any case you have already taken the decision to add the command "commit". This is a compatibility issue but must be dealt with by the user. That's ok. Best regards. Eduardo

[tuxedo0801]

Update method must look like:

void EEPROMClass::update(int address, uint8_t value)
{
  if (!_initialized) init();
  if (_eeprom.data[address] != value) {
    _dirty = true;

  }
  _eeprom.data[address] = value;  
}

Unless you're calling commit(), all the data is held in RAM. So it does absolutely ok to always write the data to the array. But dirty-flag (whether or not commit is required) must be set when data actually changes. In the initial version, I missed the dirty-flag when doing an "update". And as already said: update is just for "some" backwards-compatibility with AVR eeprom lib.

[EduardoG26]

I suggest to use the same function for booth write and update since penalty is practically zero. Moreover adding the read-check also for "write" avoids unnecessary physical flash accesses when commit is called. This may be crucial in many applications!

void EEPROMClass::update(int address, uint8_t value) { if (!_initialized) init(); if (_eeprom.data[address] != value) { _dirty = true; _eeprom.data[address] = value;
} } void EEPROMClass::write(int address, uint8_t value) { EEPROMClass::update(int address, uint8_t value) }

[tuxedo0801]

Would work as well ... thumbsup

[EduardoG26]

Without tipos, fully tested: void EEPROMClass::update(int address, uint8_t value) { if (!_initialized) init(); if (_eeprom.data[address] != value) { _dirty = true; _eeprom.data[address] = value; } }

void EEPROMClass::write(int address, uint8_t value) { update(address, value); }

[tuxedo0801]

@EduardoG26 Are you aware of markdown to have formatted text/code? Makes your code more readable ....

[EduardoG26]

Thanks.

void EEPROMClass::update(int address, uint8_t value)
{
  if (!_initialized) init();
  if (_eeprom.data[address] != value) {
    _dirty = true;
    _eeprom.data[address] = value;
  }
}

void EEPROMClass::write(int address, uint8_t value)
{
  update(address, value);
}

[tuxedo0801]

much better ;-)

[ardiri]

the codeyou want is this:

void EEPROMClass::update(int address, uint8_t value)
{
  if (!_initialized) init();
  _dirty |= (_eeprom.data[address] != value);
  _eeprom.data[address] = value;
}

[cmaglie]

Fixed by #12