Is there a better way to write this to save space/memory? NOOB here just learnin

So I have 30 bits of this code (goes up to “rpm165”), wondering how you guys would rewrite this to condense the code

if (server.args() > 0 ) {
    for ( byte i = 0; i < server.args(); i++ ) {
      if (server.argName(i) == "rpm20") {
        String qrpm20 = server.arg(i);
        // do something here with value from server.arg(i);
        myByte20 = qrpm20.toInt();
        Serial.println(server.arg(i));
        Serial.println(myByte20);
        EEPROM.write(20, myByte20);
        EEPROM.commit();
      }
      if (server.argName(i) == "rpm25") {
        String qrpm25 = server.arg(i);
        // do something here with value from server.arg(i);
        myByte25 = qrpm25.toInt();
        Serial.println(server.arg(i));
        Serial.println(myByte25);
        EEPROM.write(25, myByte25);
        EEPROM.commit();
      }
      if (server.argName(i) == "rpm30") {
        String qrpm30 = server.arg(i);
        // do something here with value from server.arg(i);
        myByte30 = qrpm30.toInt();
        Serial.println(server.arg(i));
        Serial.println(myByte30);
        EEPROM.write(30, myByte30);
        EEPROM.commit();
      }
...
...
....
......
........

Hard to say from that snippet. I assume you want to get all the rpmxx strings into an array so you can loop through them but you can't because each section does different things. Consider an array of structs with the rpm string associated with a pointer to a function that does the work for that particular argument.

Given as Wildbill says there is jus a snippet of code why use different varaibles in each section? qrmp25 seems to only be used to get the arg(i) and them to allow it to produe myByte25 - the same appears in the next section using 30 rather than 25 - could you not use the same variable in all cases which would go some way to simplifying matters. IF you preprossed server.argName(i) to get the integer from it, then perhaps all could be reduced to just a single set of calls - get the int from server.arg(i), print statements and call to eeprom.write with the two values as arguments. No if statement needed nor several blocks of almost identical code. If we could see the whole lot of code then things maybe slightly different hence the reason posting all the code is normally recommended.

Sorry guys Im a total noob so I dont really know what I'm doing, but im trying :). My project is pages long so i don't know what to include here. Whole project is bits and pieces of other peoples sample code. I'm surprised the device works considering my skills and length of the project...

But yes server.argName(i) if found should return a integer between 0-100. All I'm trying to do is save it to the eeprom.

qrpm25, qrpm30, qrpm35...... They only gets used once, that's the way I found that bit of the code and didn't want to mess anything up. Ok so can I do this to save a bit of space?

     if (server.argName(i) == "rpm20") {
        // String qrpm20 = server.arg(i);
        // do something here with value from server.arg(i);
        // myByte20 = qrpm20.toInt();
        EEPROM.write(20, server.arg(i).toInt());
        EEPROM.commit();
      }

PS main thing is that integer rpm20 gets saved to eeprom 20
rpm25 to eeprom 25
rpm30 to eeprom 30.... this goes all the way up to rpm165 --> myByte165 ---> eeprom 165

The main thing is to write to EEPROM. Is there anything else?

Also, you should probably convert each string to an integer and then save it. Rpm20 will have to go to position 40 or thereabouts as each setting will occupy two bytes.

Here is a crude approximation of how you would get the EEPROM address from the argname.

    if (server.argName(i).startsWith("rpm")) 
    {
        char *name = server.argName(i).toCharArray();

        // Ths should probably use atoi(name+3) but I don't recall the syntax
        String indexStr = name+3;
        int index = indexStr.toInt(); 

        // This would be another call to atoi() if server.arg() returns a character pointer, not String
        byte value = server.arg(i).toInt();

        Serial.println(server.arg(i));
        Serial.println(value);
        EEPROM.write(index, value);
        EEPROM.commit();
    }

wildbill:
The main thing is to write to EEPROM. Is there anything else?

Also, you should probably convert each string to an integer and then save it. Rpm20 will have to go to position 40 or thereabouts as each setting will occupy two bytes.

Maybe its me, but I thought there was only a value every 5rpm so every 5 bytes?

countrypaul:
Maybe its me, but I thought there was only a value every 5rpm so every 5 bytes?

Yes it is every 5 starting at rpm20, rpm25, rpm30..... all the way to rpm165
eeprom memory is the same 20, 25, 30..... all the way to eeprom 165

Instead of using every 5th byte of EEPROM from 20 to 165, I would use "index / 5" to use every byte from 4 to 33.

What would be the final code if I was using consecutive bytes? I can change the code if I have to

You saidearlier "But yes server.argName(i) if found should return a integer between 0-100. All I'm trying to do is save it to the eeprom. " But you also say "Yes it is every 5 starting at rpm20, rpm25, rpm30..... all the way to rpm165" which is between 20 and 165. Did you mean server.arg(i).toint()) returns between 1 and 100?

If the value you want to store is between 0 and 155 you can store that in a byte, which means you can for example use rpm20 -> 20/5 = 4, rpm 165->165/5=33 as the index for storage. If the value to store is greater and requires and int (2 bytes) you will have to multiply those values by 2. ie. rpm20 -> 20 * 2 / 5 = 8. Make sure you mutiply before dividing if use integer arithmetic.

Once you have stored a set of values it would be better to clear (set all to zero) the EEPROM before storing in a differnt pattern or you might make debugging a nightmare.

myele:
What would be the final code if I was using consecutive bytes? I can change the code if I have to

        EEPROM.write(index / 5, value);

Is there a reason to write to eeprom in sequence? Sorry but I don't understand why switch to writing in sequence, I dont see how that simplifies my code in the end

The only reason to write the eeprom in sequence is that is the order the for loop will give you and as such you don't need to use if statements just use the loop counter and have one block of code. By using the eeprom in sequence without gaps leaves more for later if you ever need that option for any reason. You could save using the rpm number instead if you want, I think it is just those that probably have more experience have learnt the hard way that keeping things tight from early on can save a lot of time and effort by getting each part to work without having to change it again later.

I see, I’ll worry about that next. For right now when I run the code below I get this error: no matching function for call to ‘String::toCharArray() const’

  if (server.argName(i).startsWith("rpm")) 
    {
        char *name = server.argName(i).toCharArray();

        // Ths should probably use atoi(name+3) but I don't recall the syntax
        String indexStr = name+3;
        int index = indexStr.toInt(); 

        // This would be another call to atoi() if server.arg() returns a character pointer, not String
        byte value = server.arg(i).toInt();

        EEPROM.write(index, value);
        EEPROM.commit();
    }

Hey guys ended up using this code so now i don't have save code repeating over and over, seems to work. Thoughts?

       if (server.argName(i).startsWith("rpm")) 
          {
              String name = server.argName(i);
              name.remove(0,3);
              byte bracket = name.toInt();
              name.remove(0,3);
              byte newsetting = server.arg(i).toInt();
              EEPROM.write(bracket , newsetting);
              EEPROM.commit();
          }

myele:
Hey guys ended up using this code so now i don't have save code repeating over and over, seems to work. Thoughts?

       if (server.argName(i).startsWith("rpm")) 

{
             String name = server.argName(i);
             name.remove(0,3);
             byte bracket = name.toInt();
             name.remove(0,3);
             byte newsetting = server.arg(i).toInt();
             EEPROM.write(bracket , newsetting);
             EEPROM.commit();
         }

Looks good to me. If it does what you want it to, even better.

Actually now that I look at it again I had: name.remove(0,3); twice

if (server.argName(i).startsWith("rpm")) 
          {
              String name = server.argName(i);
              name.remove(0,3);
              byte bracket = name.toInt();
              byte newsetting = server.arg(i).toInt();
              EEPROM.write(bracket , newsetting);
              EEPROM.commit();
          }

myele:
Actually now that I look at it again I had: name.remove(0,3); twice

Fortunately, the second one was after you were done with the variable 'name'. :slight_smile:

How often are you expecting to receive new values...?
If it’s quite often, you may ‘wear out’ the EEPROM !

If that’s the case, once you get your code sorted, you might like to revise your storage strategy...

What I often do, is store the frequently changing values in RAM, and in the background monitor the raw power supply... if it drops below a threshold, quickly save the values to EEPROM, and restore them from EEPROM back to RAM when the processor restarts.
A small capacitor & diode should be enough to hold the supply long enough for the SAVE operation. ( I save about 1K of persistent data in my projects.)