Arduinization of code

[safocl]

cleaner code using Arduino functionality.
#4

[safocl]

potentially, this code will be more efficient than the previous one

[latchdevel]

This looks great!

I'm going to test it on actual hardware just to be fully confident about its correct behavior, and then I'll go ahead and merge the changes.

Thanks a lot!

[latchdevel]

The <cstring> library is not required nor included in the C++ AVR toolchain.

And I insist, previously using flag.length() meant that 8 characters were read, written, and compared. However, in the updated version, sizeof(flag) returns 9 because char flag[] = ESR_METER_FLAG; includes the null terminator (\0) at the end.

This change breaks compatibility for users upgrading from older firmware versions, since their EEPROM data contains only the original 8 characters and lacks the null terminator at the ninth byte.

Fortunately, the fix is simple: subtract one byte from the comparison length to ensure only the original 8 characters are checked.

if (strncmp(flag, flag_read, sizeof(flag)-1) == 0)

Thanks.

[safocl]

The <cstring> library is not required nor included in the C++ AVR toolchain.

which header contains strncmp?

[latchdevel]

Arduino AVR toolchain includes AVR-LibC

https://github.com/avrdudes/avr-libc/blob/3eed8ade37607c8e617eac1cb13f82cb9cfa4409/include/string.h#L476

https://github.com/avrdudes/avr-libc/blob/3eed8ade37607c8e617eac1cb13f82cb9cfa4409/libc/string/strncmp.S

[safocl]

Using these functions from EEPROM (get and put), it is not possible to use the rest normally (the same lines) - it will clearly not look the best - for this reason, I am now not at all sure that any moderately normal type of code is possible with the Arduino API.

[latchdevel]

I think this should work because the sizeof(T) in the template of put method will resolve the correct size, just like the get method does for the char flag_read[sizeof(ESR_METER_FLAG)];

https://github.com/arduino/ArduinoCore-avr/blob/855ea0100ba0f6b9036a19a6e71517125473b014/libraries/EEPROM/src/EEPROM.h#L137-L142

template<typename T>
const T &put(int idx, const T &t) {
    EEPtr e = idx;
    const uint8_t *ptr = (const uint8_t*) &t;
    for (int count = sizeof(T); count; --count, ++e) {
        (*e).update(*ptr++);
    }
    return t;
}

I'll test it on actual hardware and let you know.
Thanks a lot!

[safocl]

I'll test it on actual hardware and let you know.
Thanks a lot!

Perhaps you yourself will suggest a more elegant option, or of course you can refuse to accept such changes at all)))
I initially wanted to make the code simpler, but as I now understand, using the Arduino API, I have difficulty reducing it to such a simple form.

[latchdevel]

Hi @safocl,

I've tested the changes — they work perfectly, are elegantly implemented, and save over 1KB of flash. The only thing I'd suggest is removing the unnecessary #include <string.h>. Once that's gone, I'd be happy to merge your contribution.

Thanks a lot!