Skip to content

EEPROM Library: #2487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 15, 2017
Merged

EEPROM Library: #2487

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cac249f
EEPROM Library:
ittacco Sep 4, 2016
68ea5e8
It will grant that the _dirty flag is reset only at EEPROM.commit() a…
ittacco Sep 27, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions libraries/EEPROM/EEPROM.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,11 @@ class EEPROMClass {
if (address < 0 || address + sizeof(T) > _size)
return t;

memcpy(_data + address, (const uint8_t*) &t, sizeof(T));
_dirty = true;
_dirty = (memcmp(_data + address, (const uint8_t*) &t, sizeof(T)) !=0);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really OK to overwrite _dirty? Consider this scenario:

  1. EEPROM.write is called. _dirty is set to true
  2. EEPROM.put is called with a struct that is identical to what is already on the flash. _dirty is set to false
  3. EEPROM.commit is called. Since _dirty is false, commit returns true without actually writing anything.

I'm not an expert c/c++ programmer, so I might have missed something. Sorry for bothering you if that is the case.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is actually happening is that he checks the RAM copy of eeprom against what he is about to write and if different, then dirty is set to true and data is written to the RAM copy, ready for writing to flash.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. So that means that the eeprom.write in step 1 won't be lost in step 3?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct. If in step 2 there is no change, first change will be gone

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could modify the code in this way:
if (memcmp(data + address, (const uint8_t) &t, sizeof(T)) !=0) {
_dirty = true;
memcpy(data + address, (const uint8_t) &t, sizeof(T));
}
It will grant that the _dirty flag is reset only at EEPROM.commit() and no changes are lost

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shorter and better. I like it :)

Copy link

@ppostuma ppostuma Jan 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! Works well. I was confused for a bit when I didn't see get/put in .cpp and thought maybe the only change was the one noted above. Any reason they're in .h?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just changed the lines above.
put and get are function template and are part of class definition.


if (_dirty)
memcpy(_data + address, (const uint8_t*) &t, sizeof(T));

return t;
}

Expand Down