NewEEPROMClass - wrong use of ARIADNE_SIGPOS [ EEPROM_IMG_STAT @ pos 2 ]

[hagaigold]

the code below seem wrong to me. ARIADNE_SIGPOS which is the same as EEPROM_IMG_STAT from neteeprom.h, use as status, if the image uploaded by the booloader is OK or not.

While in the below code, it look like it is used as if the ARIADNE boolader itself is been used... in the system, and if not use all the EEPROM (start from offset 0).

byw: the ARIADNE_OFFSET also seem not right. in neteeprom.h, last used eeprom address is 63.

NewEEPROM.h:

#define ARIADNE_SIGPOS (0x02)
#define ARIADNE_SIGVAL (0xEE)
#define ARIADNE_OFFSET (0x40)
#define NO_OFFSET      (0x00)

NewEEPROM.cpp:

NewEEPROMClass::NewEEPROMClass(void)
{
    if(read(ARIADNE_SIGPOS, 0) == ARIADNE_SIGVAL) _offset = ARIADNE_OFFSET;
    else _offset = NO_OFFSET;
}

[loathingKernel]

About ARIADNE_OFFSET, I don't see an error, the last used address is 63, which means 64 is the first one we can write into, which in hex is 0x40. Am I missing something?

About ARIADNE_SIGPOS, yes, you are right that it is used for determining both the correct upload and the existence of ariadne in the chip. The reason that happens is because if the sketch was uploaded by ariadne, the value in ARIADNE_SIGPOS should be set to ARIADNE_SIGVAL, otherwise either it was not uploaded by ariadne or it wasn't uploaded correctly, in which case we either don't care about the ariadne settings or they won't be accessed at all respectively. At least that is what i was thinking back then. Do you have a different use case where this behaviour is causing you problems?

[hagaigold]

oops.. my bad about the 0x40 :(

ARIADNE_SIGPOS - it just feel a bit confusing.

if you are using NewEEPROM, it is because you want the "protection" of the first 0x40 bytes. isn't it ? Why using NewEEPROM without an ariadne , and if someone do use it without ariadne, and his application write 0xEE to position 2.. next time his chip will reset, his view of the EEPROM will be with the offset.

At the end, you are right, that it will not cause a "real life" problem. but "protect" eeprom area base on eeprom value witch "sit" in the same protected area seems to me wrong design.

[loathingKernel]

Well, I agree that it is ugly, but it was good enough. The proper solution would be to have a .text area in the bootloader, like the version of the bootloader or a magic string, which then can be tested. Actually optiboot does it that way, but I never got around doing it in ariadne as I couldn't figure out, at the time, some compilation error involving it, and then it just faded from my memory.

[hagaigold]

we can remove the conditioned protection and always use the offset of 0x40 bytes if NewEEPROMClass is used. less confusing and more straightforward.

or we can close the issue and forget about it.