Performance, memory leak - code review

Hello friends.

I work on a hobby measuring station. For that i use an ESP32 and a Nextion display. The code works ok i guess.
Now i am reviewing my code that i wrote and ask myself if there is a way to improve the code or even some faulty stuff.

I will post a function of my code. its quiet simple. I read values from my nextion and store this values into the EPROM.

I know that there is not "the right answer". But i would love to hear your opinions. Maybe i am messing up with the performance or even i am creating some bad stuff. Since i am still a learner with cpp i am happy for every tip

thank you

void buttonRegler_SavePopCallback(void *ptr)
{


  pageID_global = 24;
  uint32_t zweiPunkt_OnOff;
  uint32_t zweiPunkt_UpDown;
  char _zweiPunkt_Sollwert[100];
  char _zweiPunkt_Hysterese[100];
  uint32_t _zweiPunkt_Regelzeit;
  uint32_t _zweiPunkt_Totzeit;
  float f_zweiPunkt_Sollwert;
  float f_zweiPunkt_Hysterese;
  //---------------------------------
  uint32_t pid_OnOff;
  char pid_Sollwert[100];
  char pid_P[100];
  char pid_I[100];
  char pid_D[100];
  double f_pid_Sollwert;
  double f_pid_P;
  double f_pid_I;
  double f_pid_D;
  char *end;

  // 2 Punkt
  sw_twoPoint_OnOff.getValueDSGlobal("Regler",&zweiPunkt_OnOff);
  sw_TwoPoint_UpDown.getValueDSGlobal("Regler",&zweiPunkt_UpDown);
  memset(_zweiPunkt_Sollwert, 0, sizeof(_zweiPunkt_Sollwert));
  t_Sollwert.getTextGlobal("Regler",_zweiPunkt_Sollwert,sizeof(_zweiPunkt_Sollwert));
  f_zweiPunkt_Sollwert = strtof(_zweiPunkt_Sollwert,&end);
  memset(_zweiPunkt_Hysterese, 0, sizeof(_zweiPunkt_Hysterese));
  t_Hysterese.getTextGlobal("Regler",_zweiPunkt_Hysterese,sizeof(_zweiPunkt_Hysterese));
  f_zweiPunkt_Hysterese = strtof(_zweiPunkt_Hysterese,&end);
  t_Regelzeit.getValueGlobal("Regler",&_zweiPunkt_Regelzeit);
  t_Totzeit.getValueGlobal("Regler",&_zweiPunkt_Totzeit);

  //PID
  sw_PID_OnOff.getValueDSGlobal("Regler",&pid_OnOff);
  memset(pid_Sollwert, 0, sizeof(pid_Sollwert));
  t_SollwertPID.getTextGlobal("Regler",pid_Sollwert,sizeof(pid_Sollwert));
  f_pid_Sollwert = strtod(pid_Sollwert,&end);
  memset(pid_P, 0, sizeof(pid_P));
  t_P.getTextGlobal("Regler",pid_P,sizeof(pid_P));
  f_pid_P = strtod(pid_P,&end);
  memset(pid_I, 0, sizeof(pid_I));
  t_I.getTextGlobal("Regler",pid_I,sizeof(pid_I));
  f_pid_I = strtod(pid_I,&end);
  memset(pid_D, 0, sizeof(pid_D));
  t_D.getTextGlobal("Regler",pid_D,sizeof(pid_D));
  f_pid_D = strtod(pid_D,&end);


  if(!isNumeric(_zweiPunkt_Sollwert) 
    || !isNumeric(_zweiPunkt_Hysterese)
    || !isNumeric(pid_Sollwert)
    || !isNumeric(pid_P)
    || !isNumeric(pid_I)
    || !isNumeric(pid_D))
  { 
    String str4Nextion = String("Notification.")+String("t0")+String(".txt=")+String("\"")+String(popup.popuptxt_wrongformat)+String("\"");
    Serial2.print(str4Nextion);
    Serial2.write(0xFF);
    Serial2.write(0xFF);
    Serial2.write(0xFF);
    Serial2.print("page Notification");Serial2.write(0xFF);Serial2.write(0xFF);Serial2.write(0xFF);
    return;
  }
  else
  {
    if(zweiPunkt_OnOff != generalSettings._sw_twoPoint_OnOff
    ||zweiPunkt_UpDown != generalSettings._sw_TwoPoint_UpDown
    ||f_zweiPunkt_Sollwert != generalSettings.zweipunkt_sollwert
    ||f_zweiPunkt_Hysterese != generalSettings.zweipunkt_hyst
    ||_zweiPunkt_Regelzeit != generalSettings.zweipunkt_regelzeit
    ||_zweiPunkt_Totzeit != generalSettings.zweipunkt_totzeit
    ||pid_OnOff != generalSettings._sw_PID_OnOff
    ||f_pid_Sollwert != generalSettings.PID_sollwert
    ||f_pid_P != generalSettings.P_sollwert
    ||f_pid_I != generalSettings.I_sollwert
    ||f_pid_D != generalSettings.D_sollwert)
    {
      generalSettings._sw_twoPoint_OnOff = zweiPunkt_OnOff;
      generalSettings._sw_TwoPoint_UpDown = zweiPunkt_UpDown;
      generalSettings.zweipunkt_sollwert = f_zweiPunkt_Sollwert;
      generalSettings.zweipunkt_hyst = f_zweiPunkt_Hysterese;
      generalSettings.zweipunkt_regelzeit = _zweiPunkt_Regelzeit;
      generalSettings.zweipunkt_totzeit = _zweiPunkt_Totzeit;
      generalSettings._sw_PID_OnOff = pid_OnOff;
      generalSettings.PID_sollwert = f_pid_Sollwert;
      generalSettings.P_sollwert = f_pid_P;
      generalSettings.I_sollwert = f_pid_I;
      generalSettings.D_sollwert = f_pid_D;

      EEPROM.writeBool(PHVALUEADDR + 17*(sizeof(float)),generalSettings._sw_twoPoint_OnOff);
      EEPROM.commit();
      EEPROM.writeBool(PHVALUEADDR + 18*(sizeof(float)),generalSettings._sw_TwoPoint_UpDown);
      EEPROM.commit();
      EEPROM.writeFloat(PHVALUEADDR + 19*(sizeof(float)), generalSettings.zweipunkt_sollwert);
      EEPROM.commit();
      EEPROM.writeFloat(PHVALUEADDR + 20*(sizeof(float)), generalSettings.zweipunkt_hyst);
      EEPROM.commit();
      EEPROM.writeInt(PHVALUEADDR + 21*(sizeof(float)),generalSettings.zweipunkt_regelzeit);
      EEPROM.commit();
      EEPROM.writeInt(PHVALUEADDR + 22*(sizeof(float)), generalSettings.zweipunkt_totzeit);
      EEPROM.commit();
      EEPROM.writeFloat(PHVALUEADDR + 24*(sizeof(float)), generalSettings.PID_sollwert);
      EEPROM.commit();
      EEPROM.writeFloat(PHVALUEADDR + 25*(sizeof(float)), generalSettings.P_sollwert);
      EEPROM.commit();
      EEPROM.writeFloat(PHVALUEADDR + 26*(sizeof(float)), generalSettings.I_sollwert);
      EEPROM.commit();
      EEPROM.writeFloat(PHVALUEADDR + 27*(sizeof(float)), generalSettings.D_sollwert);
      EEPROM.commit();
      EEPROM.writeBool(PHVALUEADDR + 28*(sizeof(float)),generalSettings._sw_PID_OnOff);
      EEPROM.commit();

 
      if(generalSettings._sw_twoPoint_OnOff == 0)
      {
       digitalWrite(Relais_1,1);     //1 Bedeudet aus
       Serial2.print("Front.p2.pic=57");Serial2.write(0xFF);Serial2.write(0xFF);Serial2.write(0xFF);
      }

      if(generalSettings._sw_PID_OnOff == 0)
      {
        MCP4725.setVoltage(0, false);
      }


      String str4Nextion = String("Notification.")+String("t0")+String(".txt=")+String("\"")+String(popup.popuptxt_changessaved)+String("\"");
      Serial2.print(str4Nextion);
      Serial2.write(0xFF);
      Serial2.write(0xFF);
      Serial2.write(0xFF);
      Serial2.print("page Notification");Serial2.write(0xFF);Serial2.write(0xFF);Serial2.write(0xFF);
    }
    
 
  }
  }

The first thing that I notice is that the way you write to EEPROM is very clumsy

Why not use EEPROM.put() to save the whole struct with a single command ? That would avoid the need for calculation of the EEPROM address for each data item with its possibility of error and calculation time overhead. You can load the whole struct back using EEPROM.get()

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.