rgblight.c: Prevent wasteful writing to eeprom.

[mtei]

Description

In order to prolong the lifetime of eeprom, write of the same value is omitted.

Types of changes

• Core
• Bugfix
• New Feature
• Enhancement/Optimization
• Keyboard (addition or update)
• Keymap/Layout/Userspace (addition or update)
• Documentation

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document. (https://docs.qmk.fm/#/contributing)
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[mechmerlin]

tagging @drashna as he might be upset about this change. =P

[mtei]

Will be upset ? really?
If he does not like this, I will close this.

[mechmerlin]

Will be upset ? really?
If he does not like this, I will close this.

Drashna is well known for destroying eeproms. It's an inside joke that it is his hobby.

[mechmerlin]

lgtm!

[drashna]

@mechmerlin LOL :D

@mtei Yup. It's an inside joke. A lot of the eeprom changes to the rgblight code are because of me.
Stuff like calling rgblight_mode used to actually write to eeprom twice (it writes the changes to the mode, then called rgblight_sethsv, writing to eeprom again), etc.

I was using these for layer state indication, and probably writing to EEPROM 50+ times a day.

So, anything that saves EEPROM writes, I'm all for!

So I'm all for this change, as it's a great one!

[drashna]

Thanks!

[pelrun]

The native eeprom_update_* functions that come from avr-libc already avoid rewriting identical values (which is the entire point of having them in addition to eeprom_write_*) however in the places we're reimplementing that API for ARM we've just blindly copy-and-pasted the eeprom_write_* code.

I'll raise an issue to track this, ideally we should push all these checks down into eeprom_update_* and eliminate them in the rest of the code.

[mtei]

I finally understood the difference between "write" and "update". And I also found that the implementation under the tmk_core/common/ directory is inconsistent.

If "update" is all correctly implemented, this PR workaround will be unnecessary.

[pelrun]

Eeprom is currently wildly unsafe on ARM, so it doesn't hurt to avoid writes wherever possible, even if redundant checks are made. But ideally we'll fix the lower layers soon and then the entire problem will go away.

[drashna]

@pelrun You're correct. It does appear that AVR will not write if the value is the same. If you are using the "update" function, and not the write function (which appears to be what we're using).

However, ARM doesn't do this. So, adding checks is a good idea.

@mtei for AVR this would be unnecessary, but until the ARM code is updated to do this automatically, we should leave this in place.
I'd rather err on the side of caution here, given the limit on write cycles.