Rename _EEPROM_writeData to EEPROM_writeData. Rename _EEPROM_readData to EEPROM_readData. Add return value to EEPROM_writeData to detect failure to write. Do not mark data as valid, if write has failed. Remove EEPROM_WRITE_VAR and EEPROM_READ_VAR macros. Make pos input only parameter. Convert EEPROM_OFFSET macro to typed constatant EEPROM_M500_base, it was defined in two places, leave it in one place. Use Config_StoreSettings() instead of erase_eeprom_section(). Compare float with 0xff byte by byte to avoid compiler warning "Dereferencing type punned pointer will break strict aliasing rules."

Firmware/ConfigurationStore.cpp

@@ -1,3 +1,5 @@
+//! @file
+
 #include "Marlin.h"
 #include "planner.h"
 #include "temperature.h"
@@ -11,74 +13,67 @@
 
 M500_conf cs;
 
+//! @brief Write data to EEPROM
+//! @param pos destination in EEPROM, 0 is start
+//! @param value value to be written
+//! @param size size of type pointed by value
+//! @param name name of variable written, used only for debug input if DEBUG_EEPROM_WRITE defined
+//! @retval true success
+//! @retval false failed
 #ifdef DEBUG_EEPROM_WRITE
-#define EEPROM_WRITE_VAR(pos, value) _EEPROM_writeData(pos, (uint8_t*)&value, sizeof(value), #value)
+static bool EEPROM_writeData(uint8_t* pos, uint8_t* value, uint8_t size, const char* name)
-static void _EEPROM_writeData(int &pos, uint8_t* value, uint8_t size, char* name)
 #else //DEBUG_EEPROM_WRITE
-#define EEPROM_WRITE_VAR(pos, value) _EEPROM_writeData(pos, (uint8_t*)&value, sizeof(value))
+static bool EEPROM_writeData(uint8_t* pos, uint8_t* value, uint8_t size, const char*)
-static void _EEPROM_writeData(int &pos, uint8_t* value, uint8_t size)
 #endif //DEBUG_EEPROM_WRITE
 {
 #ifdef DEBUG_EEPROM_WRITE
 	printf_P(PSTR("EEPROM_WRITE_VAR addr=0x%04x size=0x%02hhx name=%s\n"), pos, size, name);
 #endif //DEBUG_EEPROM_WRITE
-	while (size--) {
+	while (size--)
-		uint8_t * const p = (uint8_t * const)pos;
+	{
-		uint8_t v = *value;
+
-		// EEPROM has only ~100,000 write cycles,
+        eeprom_update_byte(pos, *value);
-		// so only write bytes that have changed!
+        if (eeprom_read_byte(pos) != *value) {
-		if (v != eeprom_read_byte(p)) {
+            SERIAL_ECHOLNPGM("EEPROM Error");
-			eeprom_write_byte(p, v);
+            return false;
-			if (eeprom_read_byte(p) != v) {
+        }
-				SERIAL_ECHOLNPGM("EEPROM Error");
+
-				return;
-			}
-		}
 		pos++;
 		value++;
-	};
+	}
-
+    return true;
 }
 
 #ifdef DEBUG_EEPROM_READ
-#define EEPROM_READ_VAR(pos, value) _EEPROM_readData(pos, (uint8_t*)&value, sizeof(value), #value)
+static void EEPROM_readData(uint8_t* pos, uint8_t* value, uint8_t size, const char* name)
-static void _EEPROM_readData(int &pos, uint8_t* value, uint8_t size, char* name)
 #else //DEBUG_EEPROM_READ
-#define EEPROM_READ_VAR(pos, value) _EEPROM_readData(pos, (uint8_t*)&value, sizeof(value))
+static void EEPROM_readData(uint8_t* pos, uint8_t* value, uint8_t size, const char*)
-static void _EEPROM_readData(int &pos, uint8_t* value, uint8_t size)
 #endif //DEBUG_EEPROM_READ
 {
 #ifdef DEBUG_EEPROM_READ
 	printf_P(PSTR("EEPROM_READ_VAR addr=0x%04x size=0x%02hhx name=%s\n"), pos, size, name);
 #endif //DEBUG_EEPROM_READ
-    do
+    while(size--)
     {
-        *value = eeprom_read_byte((unsigned char*)pos);
+        *value = eeprom_read_byte(pos);
         pos++;
         value++;
-    }while(--size);
+    }
 }
 
-//======================================================================================
-#define EEPROM_OFFSET 20
-// IMPORTANT:  Whenever there are changes made to the variables stored in EEPROM
-// in the functions below, also increment the version number. This makes sure that
-// the default values are used whenever there is a change to the data, to prevent
-// wrong data being written to the variables.
-// ALSO:  always make sure the variables in the Store and retrieve sections are in the same order.
-
 #define EEPROM_VERSION "V2"
 
 #ifdef EEPROM_SETTINGS
-void Config_StoreSettings(uint16_t offset) 
+void Config_StoreSettings()
 {
-  int i = offset;
   strcpy(cs.version,"000"); //!< invalidate data first @TODO use erase to save one erase cycle
   
-  _EEPROM_writeData(i,reinterpret_cast<uint8_t*>(&cs),sizeof(cs),0);
+  if (EEPROM_writeData(reinterpret_cast<uint8_t*>(EEPROM_M500_base),reinterpret_cast<uint8_t*>(&cs),sizeof(cs),0), "cs, invalid version")
-  strcpy(cs.version,EEPROM_VERSION); // // validate data
+  {
-  i = offset;
+      strcpy(cs.version,EEPROM_VERSION); //!< validate data if write succeed
-  EEPROM_WRITE_VAR(i,cs.version); // validate data
+      EEPROM_writeData(reinterpret_cast<uint8_t*>(EEPROM_M500_base->version), reinterpret_cast<uint8_t*>(cs.version), sizeof(cs.version), "cs.version valid");
+  }
+
   SERIAL_ECHO_START;
   SERIAL_ECHOLNPGM("Settings Stored");
 }
@@ -227,21 +222,19 @@ static const M500_conf default_conf PROGMEM =
     DEFAULT_MAX_ACCELERATION_SILENT,
 };
 
-//!
+//! @brief Read M500 configuration
-//! @retval true Stored settings retrieved or default settings retrieved in case EEPROM has been erased.
+//! @retval true Succeeded. Stored settings retrieved or default settings retrieved in case EEPROM has been erased.
-//! @retval false default settings retrieved, because of older version or corrupted data
+//! @retval false Failed. Default settings has been retrieved, because of older version or corrupted data.
-bool Config_RetrieveSettings(uint16_t offset)
+bool Config_RetrieveSettings()
 {
-    int i=offset;
 	bool previous_settings_retrieved = true;
     char ver[4]=EEPROM_VERSION;
-    EEPROM_READ_VAR(i,cs.version); //read stored version
+    EEPROM_readData(reinterpret_cast<uint8_t*>(EEPROM_M500_base->version), reinterpret_cast<uint8_t*>(cs.version), sizeof(cs.version), "cs.version"); //read stored version
     //  SERIAL_ECHOLN("Version: [" << ver << "] Stored version: [" << cs.version << "]");
     if (strncmp(ver,cs.version,3) == 0)  // version number match
     {
-        i=offset;
 
-        EEPROM_READ_VAR(i,cs);
+        EEPROM_readData(reinterpret_cast<uint8_t*>(EEPROM_M500_base), reinterpret_cast<uint8_t*>(&cs), sizeof(cs), "cs");
 
         
 		if (cs.max_jerk[X_AXIS] > DEFAULT_XJERK) cs.max_jerk[X_AXIS] = DEFAULT_XJERK;
@@ -255,8 +248,11 @@ bool Config_RetrieveSettings(uint16_t offset)
             bool initialized = false;
             for (uint8_t i = 0; i < (sizeof(cs.max_feedrate_silent)/sizeof(cs.max_feedrate_silent[0])); ++i)
             {
-                if(erased != reinterpret_cast<uint32_t&>(cs.max_feedrate_silent[i])) initialized = true;
+                for(uint8_t j = 0; j < sizeof(float); ++j)
-                if(erased != reinterpret_cast<uint32_t&>(cs.max_acceleration_units_per_sq_second_silent[i])) initialized = true;
+                {
+                    if(0xff != reinterpret_cast<uint8_t*>(&(cs.max_feedrate_silent[i]))[j]) initialized = true;
+                }
+                if(erased != cs.max_acceleration_units_per_sq_second_silent[i]) initialized = true;
             }
             if (!initialized)
             {
@@ -292,9 +288,10 @@ bool Config_RetrieveSettings(uint16_t offset)
         Config_ResetDefault();
 		//Return false to inform user that eeprom version was changed and firmware is using default hardcoded settings now.
 		//In case that storing to eeprom was not used yet, do not inform user that hardcoded settings are used.
-		if (eeprom_read_byte((uint8_t *)offset) != 0xFF ||
+		if (eeprom_read_byte(reinterpret_cast<uint8_t*>(&(EEPROM_M500_base->version[0]))) != 0xFF ||
-			eeprom_read_byte((uint8_t *)offset + 1) != 0xFF ||
+			eeprom_read_byte(reinterpret_cast<uint8_t*>(&(EEPROM_M500_base->version[1]))) != 0xFF ||
-			eeprom_read_byte((uint8_t *)offset + 2) != 0xFF) {
+			eeprom_read_byte(reinterpret_cast<uint8_t*>(&(EEPROM_M500_base->version[2]))) != 0xFF)
+		{
 			previous_settings_retrieved = false;
 		}
     }

Firmware/ConfigurationStore.h

@@ -3,6 +3,8 @@
 #define EEPROM_SETTINGS
 
 #include "Configuration.h"
+#include <stdint.h>
+#include <avr/eeprom.h>
 
 typedef struct
 {
@@ -48,8 +50,8 @@ FORCE_INLINE void Config_PrintSettings() {}
 #endif
 
 #ifdef EEPROM_SETTINGS
-void Config_StoreSettings(uint16_t offset);
+void Config_StoreSettings();
-bool Config_RetrieveSettings(uint16_t offset);
+bool Config_RetrieveSettings();
 #else
 FORCE_INLINE void Config_StoreSettings() {}
 FORCE_INLINE void Config_RetrieveSettings() { Config_ResetDefault(); Config_PrintSettings(); }

Firmware/Marlin.h

@@ -385,7 +385,6 @@ float temp_comp_interpolation(float temperature);
 void temp_compensation_apply();
 void temp_compensation_start();
 void show_fw_version_warnings();
-void erase_eeprom_section(uint16_t offset, uint16_t bytes);
 uint8_t check_printer_version();
 
 #ifdef PINDA_THERMISTOR

Firmware/Marlin_main.cpp

@@ -866,11 +866,6 @@ uint8_t check_printer_version()
 	return version_changed;
 }
 
-void erase_eeprom_section(uint16_t offset, uint16_t bytes)
-{
-	for (unsigned int i = offset; i < (offset+bytes); i++) eeprom_write_byte((uint8_t*)i, 0xFF);
-}
-
 #ifdef BOOTAPP
 #include "bootapp.h" //bootloader support
 #endif //BOOTAPP
@@ -1187,7 +1182,7 @@ void setup()
 	bool previous_settings_retrieved = false; 
 	uint8_t hw_changed = check_printer_version();
 	if (!(hw_changed & 0b10)) { //if printer version wasn't changed, check for eeprom version and retrieve settings from eeprom in case that version wasn't changed
-		previous_settings_retrieved = Config_RetrieveSettings(EEPROM_OFFSET);
+		previous_settings_retrieved = Config_RetrieveSettings();
 	} 
 	else { //printer version was changed so use default settings 
 		Config_ResetDefault();
@@ -1485,7 +1480,7 @@ void setup()
 
   if (!previous_settings_retrieved) {
 	  lcd_show_fullscreen_message_and_wait_P(_i("Old settings found. Default PID, Esteps etc. will be set.")); //if EEPROM version or printer type was changed, inform user that default setting were loaded////MSG_DEFAULT_SETTINGS_LOADED c=20 r=4
-	  erase_eeprom_section(EEPROM_OFFSET, 156); 							   //erase M500 part of eeprom
+	  Config_StoreSettings();
   }
   if (eeprom_read_byte((uint8_t*)EEPROM_WIZARD_ACTIVE) == 1) {
 	  lcd_wizard(WizState::Run);
@@ -6310,12 +6305,12 @@ if((eSoundMode==e_SOUND_MODE_LOUD)||(eSoundMode==e_SOUND_MODE_ONCE))
 
     case 500: // M500 Store settings in EEPROM
     {
-        Config_StoreSettings(EEPROM_OFFSET);
+        Config_StoreSettings();
     }
     break;
     case 501: // M501 Read settings from EEPROM
     {
-        Config_RetrieveSettings(EEPROM_OFFSET);
+        Config_RetrieveSettings();
     }
     break;
     case 502: // M502 Revert to default settings

Firmware/eeprom.h

@@ -182,7 +182,10 @@
 // Magic string, indicating that the current or the previous firmware running was the Prusa3D firmware.
 #define EEPROM_FIRMWARE_PRUSA_MAGIC 0
 
-#define EEPROM_OFFSET 20 //offset for storing settings using M500
+#ifdef __cplusplus
-//#define EEPROM_OFFSET 
+#include "ConfigurationStore.h"
+static M500_conf * const EEPROM_M500_base = reinterpret_cast<M500_conf*>(20); //offset for storing settings using M500
+#endif
+
 
 #endif // EEPROM_H