Help with EEPROM Read/Write

I have a little problem reading and writing data to the EEPROM of an ATiny25 to implement a memory function.

I have looked at the wonderful code examples in the sticky already and more or less used functions from there as a template. The problem I have is that in all the examples a byte is stored while I have too much data to store for a byte so need to store a world.

The masterMode is a number from 0 to 2 so it takes 3 bits. The workMode can be 0 to 31 so takes 5 bits. This makes up my first byte. Problem is that I also need to store brightness in a range from 0 to 8. Thought just shifting the brightness over to the next byte should work, but from my debug attempts it looks like something with these functions doesn't work right.

Maybe I made a mistake casting something?

Anyone got an idea where these functions might go wrong?

void save_state() { // central method for writing (with wear leveling)
uint16_t eep;
uint8_t oldpos=eepos;

eepos = (eepos+2) & (EEPSIZE-2); // wear leveling, use next cell

eep = brightIndex << 8 | workMode << 2 | masterMode;

eeprom_write_word((uint16_t *)(eepos), eep); // save current state
eeprom_write_word((uint16_t *)(oldpos), 0xffff); // erase old state
}

void restore_state() {
uint16_t eep;
// find the config data
for(eepos=0; eepos<EEPSIZE; eepos++) {
eep = eeprom_read_word((const uint16_t *)eepos);
if (eep != 0xffff) break;
}
// unpack the config data
if (eepos < EEPSIZE-1) {
workMode = (uint8_t)eep >> 2;
masterMode = (uint8_t)eep & 0x3;
brightIndex = (uint8_t)(eep >> 8);
fetBright = pgm_read_byte(fetPWMPointer + brightIndex);
ccBright = pgm_read_byte(ccPWMPointer + brightIndex);
}
}

Hi Ryu,

my first hint would be to post this in the thread called firmware repository, where the coding experts regularly come by, who might otherwise overlook this thread.

In the meantime I have some ideas, which may or may not be true, as I’m not an expert with this stuff.

Your masterMode only needs 2 bits, not 3 as you stated (2 bits = 4 values), but &0x3 only fetches 2 significant bits so the coding should be right (but the 3+5 calculation for the first byte is off)

You said masterMode is a number 0 to 2, but it might have the value 3 after the initial restore_state (2 significant bits can get you the values 0/1/2 or 3). Does your code have a validation check for the case that value 3 is returned if the code does not know a masterMode 3?

workMode = (uint8_t)eep >> 2;
looks interesting to me.
Wouldn’t shifting by 2 bits result in 6 bits (instead of the 5 you want for workMode)?
And as eep is 16 bit, does cutting to 8 bit and then a bitwise operation truly work in that order?

Hope that gives some ideas, at least it gives a free bump :slight_smile:

Thanks for the suggestion.
Yeah I mixed something up there with the 5+3. That was the previous iteration of the firmware where the brightness and workmode was stored. But the code should be right, since I only shift 2 bits there.
The rest of the code can cope with the mastermode being 3. That is actually my criteria to detect the first boot up.
Well the cast and shift is something that I am unsure about. I hope it cuts of the bits and then shifts the, but if the left overs from the word gets shifted too this might be the problem.

But I might actually change the UI a bit to make it more intuitive and thus eliminating the need for the mastermode variable. So might be able to go back to storing a byte and that works fine.
Still would like to know what went wrong here to know in the future.