3

I am working on an arduino (based off the AVR platform) and I have a method that takes in a float and writes it to EEPROM. I have to convert the float to a byte array to interact with EEPROM. I have two functions as follow:

void WriteFloatToEEPROM(int address, float value) {
    union {
        byte byteVal[4];
        float floatVal;
    } data;

    data.floatVal = value;

    for (int i = 0; i < 4; i++) {
        EEPROM.update(address + i, data.byteVal[i]);
    }   
}

float ReadFloatFromEEPROM(int address) {
    union {
        byte byteVal[4];
        float floatVal;
    } data;

    for (int i = 0; i < 4; i++) {
        uint8_t readValue = EEPROM.read(address + i);

        data.byteVal[i] = readValue;        
    }

    return data.floatVal;
}

When I print out the results of this I get the following as a few examples:

Read value at address 50 for float read 0
Read value at address 51 for float read 0
Read value at address 52 for float read 0
Read value at address 53 for float read 0
    Returned float val for address 50:0.00
Read value at address 90 for float read 0
Read value at address 91 for float read 0
Read value at address 92 for float read 0
Read value at address 93 for float read 160
    Returned float val for address 90:-0.00
Read value at address 130 for float read 44
Read value at address 131 for float read 113
Read value at address 132 for float read 61
Read value at address 133 for float read 138
    Returned float val for address 130:-0.00
Read value at address 170 for float read 0
Read value at address 171 for float read 0
Read value at address 172 for float read 0
Read value at address 173 for float read 0
    Returned float val for address 170:0.00

Am I using a union wrong/writing to EEPROM backwards or something? Also if anyone has a better way of doing this, I am open to suggestions. Thanks in advance

Phillip McMullen
  • 453
  • 2
  • 5
  • 15
  • 1
    This might help https://stackoverflow.com/questions/11373203/accessing-inactive-union-member-and-undefined-behavior – kiran Biradar Aug 31 '18 at 20:37
  • Arduino programming is outside my expertise, but couldn't you just use [`EEPROM.put()`](https://www.arduino.cc/en/Reference/EEPROMPut) and [`EEPROM.get()`](https://www.arduino.cc/en/Reference/EEPROMGet)? – Bob__ Aug 31 '18 at 21:43
  • EEPROM.update is better to use because it only writes if the data is different at that location which saves a little bit of time (3.3ms) but also prevents writing to that location repeatedly which would burn up the EEPROM cell. Essentially it just does a get before the put as far as I've seen – Phillip McMullen Aug 31 '18 at 21:55
  • Yes, but in the linked reference of `EEPROM.put()` I read: *"Note: This function **uses EEPROM.update()** to perform the write, so does not rewrites the value if it didn't change. "*. To my understanding is `EEPROM.write()` which doesn't check before writing. – Bob__ Aug 31 '18 at 21:59

3 Answers3

7

Reading not the "active" field of the union is UB (Undefined Behavior).

You have to use memcpy:

void WriteFloatToEEPROM(int address, float value) {
    byte byteVal[sizeof(float)];
    memcpy(byteVal, &value, sizeof(float));

    for (int i = 0; i < sizeof(float); i++) {
        EEPROM.update(address + i, byteVal[i]);
    }   
}

float ReadFloatFromEEPROM(int address) {
    byte byteVal[sizeof(float)];

    for (int i = 0; i < sizeof(float); i++) {
        byteVal[i] = EEPROM.read(address + i);
    }

    float f;
    memcpy(&f, byteVal, sizeof(float));
    return f;
}
Jarod42
  • 203,559
  • 14
  • 181
  • 302
  • Wouldn't this be a valid case for usage of `reinterpret_cast`? – Killzone Kid Aug 31 '18 at 20:41
  • @KillzoneKid probably works, but still has wiggles with strict aliasing. – user4581301 Aug 31 '18 at 20:42
  • @user4581301 `AliasedType is std::byte, (since C++17)char, or unsigned char: this permits examination of the object representation of any object as an array of bytes.` should be no problem then? – Killzone Kid Aug 31 '18 at 20:44
  • @KillzoneKid `float` to `char` array is safe, but the `char` array is not necessarily aligned correctly to be used as a `float` – user4581301 Aug 31 '18 at 20:48
  • @Jarod42 `for (int i = 0; i < 4; i++)` should be `for (size_t i = 0; i < sizeof(float); ++i)` in both cases. – Swordfish Aug 31 '18 at 20:50
  • @KillzoneKid: the casting might only works with `float` to `const char*`. The other way would break strict aliasing. – Jarod42 Aug 31 '18 at 20:54
  • @user4581301 Not sure I understand. Since we are copying bytes in a loop into EEPROM and back, under which circumstances we could have alignment problem? Using memcpy seems like doing the copying twice. – Killzone Kid Sep 01 '18 at 04:39
  • 2
    @KillzoneKid I think I know where you are going. You're saying we can safely cast `float *` to `char*`, so kill the intermediary buffer completely and read directly into the `float`. A loop around `float_as_charp[i] = read(i);`. I can't think of a good reason why that wouldn't work. – user4581301 Sep 01 '18 at 16:46
3

Arduino provides .put() and .get() methods with EEPROM object so you don't have to reinvent the wheel. Your code could be as simple as:

void WriteFloatToEEPROM(int address, float value)
{
    EEPROM.put(address, value);
}

float ReadFloatFromEEPROM(int address)
{
    float value;
    EEPROM.get(address, value);
    return value;
}

.put() also acts in a similar way to .update()

This function uses EEPROM.update() to perform the write, so does not rewrites the value if it didn't change.

user4581301
  • 33,082
  • 7
  • 33
  • 54
Killzone Kid
  • 6,171
  • 3
  • 17
  • 37
  • I will test this out on Monday, but I feel stupid for never finding the put/get methods. I've been using read/write/update the entire time. Much appreciated – Phillip McMullen Sep 01 '18 at 23:49
0

It is common problem for struct and union. You need investigate "aligns". Default it usually "word" (2 bytes) so your byte byteVal[4]; can take 8 bytes (byte, empty, byte, empty ...). You must use __attribute__ ((packed)) for avoid it.

Sergey
  • 31
  • 3