PRosenb / EEPROMWearLevel

Arduino EEPROMWearLevel reduces EEPROM wear by writing a new value to an other EEPROM location.
Apache License 2.0
78 stars 13 forks source link

Minor correction in example: 18 -> 19 #15

Closed eddyp closed 3 years ago

eddyp commented 3 years ago

The example given says there are 18 bytes used, but actually there are 19: 1 version, 2 control, 16 for data (index 0 to 15).

AIUI, either we can say the region allows using actually 16 bytes to store data, and that requires 3 extra bytes, or the entire thing has 19 bytes in total, but never 18.

Am I missing something?

PRosenb commented 3 years ago

Hi @eddyp , thanks for looking into it in details. Do you refer to the text A partition with a length of 18 bytes is represented as follows? With a partition I refer to the 18 bytes after the version byte. These 18 bytes of a partition are needed for every partition while the version byte is used only once in the beginning. You are totally right that it's could be written in a way that this is better visible.

What about changing the sentence from EEPROMWearLevel uses one partition for every idx you use. A partition with a length of 18 bytes is represented as follows: to EEPROMWearLevel uses one partition for every idx you use. Before the partitions, there is one byte used for the version. After that one version byte, the first partition starts. A partition with a length of 18 bytes is represented as follows: ?

eddyp commented 3 years ago

Hi @eddyp , thanks for looking into it in details. Do you refer to the text A partition with a length of 18 bytes is represented as follows? With a partition I refer to the 18 bytes after the version byte.

Yes

EEPROMWearLevel uses one partition for every idx you use. Before the partitions, there is one byte used for the version. After that one version byte, the first partition starts. A partition with a length of 18 bytes is represented as follows: ?

That "Before the partitions, there is one byte..." for my non native ears still sounds ambiguous.

Before proposing anything, I still find it confusing the details on how is the number of bytes allocated to a partition of an index since the control bytes suggest they should be a multiple of 8 (or at least the number of entries, assuming a multi byte data type)?

I get it that the begin() API has a form including lengths, but the uncertainty and lack of details, plus my innate will of not depending on some assumptions, make this rub me the wrong way.

If I got this correctly, may I suggest something to the effect of your comment? ;) Something like this:

"EEPROMWearLevel uses one partition for every idx you use, but just a single byte before the first partition to store the version. Assuming a configuration with a single partition of 18 bytes, it will be represented in memory a such: "

What do you think?

PRosenb commented 3 years ago

I see what you mean... What about:

EEPROMWearLevel first uses one byte to store the version. After that, the first partition starts. For every idx you use, one partition is allocated.
Assuming a configuration with a single partition of 18 bytes, it will be represented in EEPROM a follows:

eddyp commented 3 years ago

I see what you mean... What about:

EEPROMWearLevel first uses one byte to store the version. After that, the first partition starts. For every idx you use, one partition is allocated. Assuming a configuration with a single partition of 18 bytes, it will be represented in EEPROM a follows:

Sounds very clear.

Are you willing to also clarify the other thing? This new wording in conjunction with the first begin API that uses the entire EEPROM, will raise the obvious question of what will be the layout in various practical configuration.

Since examples help me a lot, I'll put it the same form: What will be the actual layout with 2 partitions used in a system with a 1K EEPROM, while both partitions correspond to indexes with sizeof = 3 bytes?

Since 1 byte is used for the version, how are the remaining 1023 bytes used? Assuming 1 byte is unused, we, theoretically, get 2 partitions of 511 bytes each? Right? Then is each total partition space and the sizeof(index) used to calculate how many "versions" of the index you can have?

if that's the case, then is this calculation roughly correct?

Number of maximum different idx writes before needing to erase = trunc (511 bytes / ( sizeof(index) 8 + 1 )) 8 = 160 Number of bytes used to store data = 160 * 3 = 480 Number of control bytes = 160 / 8 = 20

Meaning that the actually used bytes are 160 * 3 + 20 = 500, leading to 11 bytes not being used in each partition, which means 23 total bytes wasted?

Is this correct? Are you a little more efficient and also use the last 11 bytes to store 3 more values and have just 1 wasted byte and 5 bits in the last control byte not ever used?

PRosenb commented 3 years ago

Great, thanks for reviewing it, I updated the page accordingly.

For giving devs an overview on how it works, I think this simple example with one partition is a good start. If they like to get more details, the implementation should help them as it contains comments and is not overly complicated.

I don't understand all details of your example but think you are generally right. Does it match with the following implementation?

init() getControlBytesCount() findIndex()

eddyp commented 3 years ago

Great, thanks for reviewing it, I updated the page accordingly.

Thanks!

For giving devs an overview on how it works, I think this simple example with one partition is a good start. If they like to get more details, the implementation should help them as it contains comments and is not overly complicated.

I don't understand all details of your example but think you are generally right. Does it match with the following implementation?

init() getControlBytesCount() findIndex()

Ah, even better! Is a little different in the sense you're actually trying to minimize the number of wasted bits, while my example was wasting bytes.

BTW, I think you can optimize even more for multi-byte indexes, so you use one bit per valid value, instead of per byte:

Assume an index of 3 bytes with a partition of 17 bytes. In this proposal getControlBytesCount() would calculate like this:

Total number of bits in partition = 17 8 = 136 bits Bits per value = sizeof(index) 8 + 1 = 3 * 8 +1 = 25 bits Maximum no values in partition = trunc( 136 / 25) = 5 values

=>

1 control byte 5 values max, hence 5 control bits and 3 unused

This can help with multi-byte indexes, and has no change for single byte data, while needing to store the index type size in EEPROMConfig.

What do you think?

PRosenb commented 3 years ago

Interesting thoughts.. Optimising it more is a trade off between easy to understand memory layout/code and optimal use of EEPROM space. On the approach above I also see the drawback, that the written/read data length needs to be fixed and cannot be changed without loosing data. The original approach allows to store different size of data every time, as long as the user knows how long it was when reading it back. Let's keep it as is to leave it as an easy to understand and stable solution.

eddyp commented 3 years ago

Fair enough. I really like your idea as is very similar in purpose to my GeneROMst idea, while I would really like in GeneROMst to have autodiscovery possible and no ambiguities, so my plan was all layout information was written into the entries:

https://github.com/uCautomation/plant-watering-system#generational-eeprom-storage-generomst

I haven't yet decided on the layout, but I'm leaning towards a magic number per entry, the data and a checksum (BSD checksum seems simple and robust enough), but picking your brain was useful. Thanks.

PRosenb commented 3 years ago

That's great to hear could I contribute to your project, good luck @eddyp :-).