Please help me simplifying the code

Using Arduino Programming Questions
#1

Hi

I come to ask you some help. Can you help me giving me tips to simplify my code. It works like a charm, but I am ashamed because of how I get the result. I hard-coded too much. I would like to make something tidy. My experience is only with python and php, then I could make dynamic variables but here...

if ( (now.hour() == hora1 && now.minute() == minuto1 && now.second() == 0) or  (now.hour() == hora2 && now.minute() == minuto2 && now.second() == 0) or (now.hour() == hora3 && now.minute() == minuto3 && now.second() == 0) or (now.hour() == hora4 && now.minute() == minuto4 && now.second() == 0) or (now.hour() == hora5 && now.minute() == minuto5 && now.second() == 0)) {
          Serial.println ("Opening");
          activar_apertura(mascota1, tiempodeapertura1);
    }

     
  if (Serial.available())  {
    char c = Serial.read();  //gets one byte from serial buffer
    if(c== '/'){
    aperturas_programadas();
  }
  if (c == '*') {
      //do stuff
     
      Serial.println();
      Serial.print("La cadena capturada es : ");
      Serial.println(readString); //prints string to serial port out
     
      ind1 = readString.indexOf(',');  //finds location of first ,
      str_mascota1 = readString.substring(0, ind1);   //captures first data String
      ind2 = readString.indexOf(',', ind1+1 );   //finds location of second ,
      str_hora1 = readString.substring(ind1+1, ind2+1);   //captures second data String
      ind3 = readString.indexOf(',', ind2+1 );
      str_minuto1 = readString.substring(ind2+1, ind3+1);
      ind4 = readString.indexOf(',', ind3+1 );
      str_tiempodeapertura1 = readString.substring(ind3+1, ind4+1); //captures remain part of data after last ,
      
      ind5 = readString.indexOf(',', ind4+1 );
      str_mascota2 = readString.substring(ind4+1, ind5+1); //captures remain part of data after last ,
      ind6 = readString.indexOf(',', ind5+1 );
      str_hora2 = readString.substring(ind5+1, ind6+1); //captures remain part of data after last ,
      ind7 = readString.indexOf(',', ind6+1 );
      str_minuto2 = readString.substring(ind6+1, ind7+1); //captures remain part of data after last ,
      ind8 = readString.indexOf(',', ind7+1 );
      str_tiempodeapertura2 = readString.substring(ind7+1, ind8+1); //captures remain part of data after last ,

      ind9 = readString.indexOf(',', ind8+1 );
      str_mascota3 = readString.substring(ind8+1, ind9+1); //captures remain part of data after last ,
      ind10 = readString.indexOf(',', ind9+1 );
      str_hora3 = readString.substring(ind9+1, ind10+1); //captures remain part of data after last ,
      ind11 = readString.indexOf(',', ind10+1 );
      str_minuto3 = readString.substring(ind10+1, ind11+1); //captures remain part of data after last ,
      ind12 = readString.indexOf(',', ind11+1 );
      str_tiempodeapertura3 = readString.substring(ind11+1, ind12+1); //captures remain part of data after last ,

      ind13 = readString.indexOf(',', ind12+1 );
      str_mascota4 = readString.substring(ind12+1, ind13+1); //captures remain part of data after last ,
      ind14 = readString.indexOf(',', ind13+1 );
      str_hora4 = readString.substring(ind13+1, ind14+1); //captures remain part of data after last ,
      ind15 = readString.indexOf(',', ind14+1 );
      str_minuto4 = readString.substring(ind14+1, ind15+1); //captures remain part of data after last ,
      ind16 = readString.indexOf(',', ind15+1 );
      str_tiempodeapertura4 = readString.substring(ind15+1, ind16+1); //captures remain part of data after last ,

      ind17 = readString.indexOf(',', ind16+1 );
      str_mascota5 = readString.substring(ind16+1, ind17+1); //captures remain part of data after last ,
      ind18 = readString.indexOf(',', ind17+1 );
      str_hora5 = readString.substring(ind17+1, ind18+1); //captures remain part of data after last ,
      ind19 = readString.indexOf(',', ind18+1 );
      str_minuto5 = readString.substring(ind18+1, ind19+1); //captures remain part of data after last ,
      ind20 = readString.indexOf(',', ind19+1 );
      str_tiempodeapertura5 = readString.substring(ind19+1); //captures remain part of data after last ,



      Serial.print("Mascota = ");
      Serial.println(str_mascota1);
      mascota1=str_mascota1.toInt();
      
      Serial.print("Hora = ");
      Serial.println(str_hora1);
      hora1=str_hora1.toInt();
      
      Serial.print("Minuto = ");
      Serial.println(str_minuto1);
      minuto1=str_minuto1.toInt();
      
      Serial.print("Tiempo de apertura = ");
      Serial.println(str_tiempodeapertura1);
      tiempodeapertura1=str_tiempodeapertura1.toInt();

      Serial.println();            

      Serial.print("Mascota = ");
      Serial.println(str_mascota2);
      mascota2=str_mascota2.toInt();

      Serial.print("Hora = ");
      Serial.println(str_hora2);
      hora2=str_hora2.toInt();
      Serial.print(hora2);
      Serial.print(str_hora2);
      Serial.print("Minuto = ");
      Serial.println(str_minuto2);
      minuto2=str_minuto2.toInt();
      Serial.print("Tiempo de apertura = ");
      Serial.println(str_tiempodeapertura2);
      tiempodeapertura2=str_tiempodeapertura2.toInt();
            Serial.println();            

      Serial.print("Mascota = ");
      Serial.println(str_mascota3);
      mascota3=str_mascota3.toInt();

      Serial.print("Hora = ");
      Serial.println(str_hora3);
      hora3=str_hora3.toInt();
      
      Serial.print("Minuto = ");
      Serial.println(str_minuto3);
      minuto3=str_minuto3.toInt();
               
      Serial.print("Tiempo de apertura = ");
      Serial.println(str_tiempodeapertura3);
      tiempodeapertura3=str_tiempodeapertura3.toInt();
            Serial.println();            

      Serial.print("Mascota = ");
      Serial.println(str_mascota4);
      mascota=str_mascota4.toInt();

      Serial.print("Hora = ");
      Serial.println(str_hora4);
      hora4=str_hora4.toInt();
      
      Serial.print("Minuto = ");
      Serial.println(str_minuto4);
      minuto4=str_minuto4.toInt();
                
      Serial.print("Tiempo de apertura = ");
      Serial.println(str_tiempodeapertura4);
      tiempodeapertura4=str_tiempodeapertura4.toInt();
      Serial.println();            
      
      Serial.print("Mascota = ");
      Serial.println(str_mascota5);
      mascota=str_mascota5.toInt();

      Serial.print("Hora = ");
      Serial.println(str_hora5);
      hora5=str_hora5.toInt();
      
      Serial.print("Minuto = ");
      Serial.println(str_minuto5);
      minuto5=str_minuto5.toInt();
                
      Serial.print("Tiempo de apertura = ");
      Serial.println(str_tiempodeapertura5);
      tiempodeapertura5=str_tiempodeapertura5.toInt();
      
      
      readString=""; //clears variable for new input
      str_mascota1="99";
      str_hora1="99";
      str_minuto1="99";
      str_tiempodeapertura1="99";

      str_mascota2="99";
      str_hora2="99";
      str_minuto2="99";
      str_tiempodeapertura2="99";

      str_mascota3="99";
      str_hora3="99";
      str_minuto3="99";
      str_tiempodeapertura3="99";

      str_mascota4="";
      str_hora4="99";
      str_minuto4="99";
      str_tiempodeapertura4="99";
      
      str_mascota5="";
      str_hora5="99";
      str_minuto5="99";
      str_tiempodeapertura5="99";     
      
      
    } 
    else {     
      readString += c; //makes the string readString
    }
  }
#2

You have a lot of read & print commands, which could be assembled to functions, that you call with the shifting parameters.

It does not save program memory but it would make it much easier to read and debug.

Since you have only submitted part of the code, it is a bit hard to anticipate what you are doing.
I was thinking along the lines of having all your variables in a couple of arrays, which make the read and print functions even simpler, as then you could read and print the arrays wthin a for loop.

Just my humble ideas.

#3

Hi

Thank you for your help. Yes I just have the printing commands to debug the code. You are right its not save in the program memory because I though its would be too difficult for me to read and write, and so i use a capacitor of 10uF between reset and ground to not lose the variables (lol).

The idea of arrays i thought about it for a moment but how would be the read?

Here is the entire code. I tried to post directly here but I can't because of the size. I upload in pastebin the code
I apologise all the messy code, I know I hard-coded too much!. In my defense I have to say I study medicine.

#4

for the part you have posted, google the C++ function strtok to simplify it.

#5

It's normal to layout this sort of thing,

 (now.hour() == hora1 && now.minute() == minuto1 && now.second() == 0) or  (now.hour() == hora2 && now.minute() == minuto2 && now.second() == 0) or (now.hour() == hora3 && now.minute() == minuto3 && now.second() == 0) or (now.hour() == hora4 && now.minute() == minuto4 && now.second() == 0) or (now.hour() == hora5 && now.minute() == minuto5 && now.second() == 0)) {

to make it readable

 (now.hour() == hora1 && now.minute() == minuto1 && now.second() == 0) 
or  (now.hour() == hora2 && now.minute() == minuto2 && now.second() == 0) 
or (now.hour() == hora3 && now.minute() == minuto3 && now.second() == 0) 
or (now.hour() == hora4 && now.minute() == minuto4 && now.second() == 0) 
or (now.hour() == hora5 && now.minute() == minuto5 && now.second() == 0)) {

After that you can sort out the conditional

if (now.second() == 0){
   //we only need the rest of the conditional if seconds==0

}else {

}

Mark

#6

Thanks for your advice Holmes4.

Arduinodlb I will find out more about that. Thanks

#7

Arduinodlb I spent some hours but I cant figure out how make readString usable with strtok or strtok_r.

#8

basiliomarc:
Arduinodlb I spent some hours but I cant figure out how make readString usable with strtok or strtok_r.

Perhaps because it isn't. :slight_smile:
Those functions don't belong to the String class, and can't work with Strings.

#9

Those functions don't belong to the String class, and can't work with Strings.

@aarg - Wrong again

String won't work with string (unless an op has been overloaded eg ==) but will work with String!

Mark

#10

holmes4:
@aarg - Wrong again

String won't work with string (unless an op has been overloaded eg ==) but will work with String!

Mark

Huh? That isn't what I said.
Perhaps there is some way that strtok() can work with Strings? I said no. Well, I don't know everything. Far from it. But that seems unlikely to me.

#11

aarg:
Perhaps because it isn’t. :slight_smile:
Those functions don’t belong to the String class, and can’t work with Strings.

Well, while true, that’s a little confusing for someone who hasn’t used strtok before.

You need to convert the String object to a character array first, then use strtok on the char array.

Something like (untested)

#include <cstring>
#include <iostream>
....
    String stringToTokenise = "field1, field2, field3";
    int len = stringToTokenise.length() + 1;
    char *charString = new char[len]; 

    stringToTokenise.toCharArray(charString, len);
    char *token = std::strtok(charString, ",");
    while (token != NULL) {
        std::cout << token << '\n';
        token = std::strtok(NULL, ",");
    }

    // don't forget to delete any data we've allocated using "new"
    delete [] charString;
#12