How to improve my for / do / while loops

Using Arduino Programming Questions
#1

I have a series of messages incoming to Arduino over 6 different MQTT topics, they look like this:

Topic                       Payload
--------------------------------------------------------------------------------
/home/weath/now   -> Overcast;5.90;78.00;1005.00;08:04;16:12                // Weather now
/home/weath/0     -> 2.00;7.00;60.00;Overcast                               // Weather today
/home/weath/1     -> Wednesday;2.00;7.00;0.00;Partly Cloudy;77.00           // Weather tomorrow
/home/weath/2     -> Thursday;1.00;5.00;20.00;Partly Cloudy;79.00           // etc.
/home/weath/3     -> Friday;0.00;6.00;20.00;Clear;73.00
/home/weath/4     -> Saturday;-1.00;6.00;0.00;Clear;72.00

There’s no specific reason for them to come in on different topics, it was an arbitrary decision.

The message format I’ve created for myself is, rightly or wrongly, consistent for the last 4 messages (day name;min temp;max temp;chance of precip;text desc;humidity), but not for the first two, which don’t require the day name, and also have barometric pressure, sunrise and sunset times.

MQTT library for Arduino presents the incoming payload as a character array, and there’s no reason for me to convert numerical values to ints/floats as I’m not doing anything with them except display on a screen.

So here’s a struct which holds the values (supports the latter 4 payloads mentioned, not the first 2):

struct weather {
  char name[10];           // this is the DAY name
  char min[7];             // e.g. -11.35 + null terminator
  char max[9];
  char precip[4];          // Percentage, e.g. up to 3 chars plus null terminator
  char conditiontext[25];  // Longest = "scattered thunderstorms"
  char humid[4];           // Percentage
};

typedef struct weather Weather;
Weather weatherDay[5];

I wanted to avoid memcpy’ing the payload to a dedicated buffer, as I don’t want to waste sram.

My horrendous / unmaintainable solution is as follows, but can someone point me in the direction of a more sensible / economic use of code?!

For each day (starting on array index 1 not 0), iterate through the payload (int i) and write the character array for the setting (int j), resetting j each time I start writing to the new setting. Each setting finishes with null terminator being added, and i is incremented again to skip over the delimiter ready for the next setting. As there’s no delimiter at the end, use length, which is provided by the MQTT callback function to determine when to stop writing the last setting.

It works, but it’s ugly. Please be as brutal as possible in suggesting better ways to do this:

  for (int h=1;h<5;h++) {                                      // start at weatherDay[1], 0 is reserved
    sprintf (buff,"%s/%s/%i", rootvector, weathervector, h);   // i.e. topic "home/weath/1" to "...4"
      if(strcmp(topic, buff) == 0) {
        int i=0;
        int j=0;
        do {
          weatherDay[h].name[j] = payload[i];
          i++;
          j++;
        } while (payload[i] != ';');
        weatherDay[h].name[j] = '\0';
        i++;
        j=0;
        do {
          weatherDay[h].min[j] = payload[i];
          i++;
          j++;
        } while (payload[i] != ';');
        weatherDay[h].min[j] = '\0';
        i++;
        j=0;
        do {
          weatherDay[h].max[j] = payload[i];
          i++;
          j++;
        } while (payload[i] != ';');
        weatherDay[h].max[j] = '\0';
        i++;
        j=0;
        do {
          weatherDay[h].precip[j] = payload[i];
          i++;
          j++;
        } while (payload[i] != ';');
        weatherDay[h].precip[j] = '\0';
        i++;
        j=0;
        do {
          weatherDay[h].conditiontext[j] = payload[i];
          i++;
          j++;
        } while (payload[i] != ';');
        weatherDay[h].conditiontext[j] = '\0';
        i++;
        j=0;
        do {
          weatherDay[h].humid[j] = payload[i];
          i++;
          j++;
        } while (i <= length);
        weatherDay[h].humid[j] = '\0';
      }
    }
#2

hazymat:
... and there's no reason for me to convert numerical values to ints/floats as I'm not doing anything with them except display on a screen.

well no reason but to move data out of SRAM and into FLASH memory.

instead of using the day of the week just strcmp() your day of the week versus a char array stored in program space and use a total of 6 bytes of SRAM...

Same for your other variables, by not converting them to signed ints or floats which would use only 4 bytes on your Arduino, you are using up to nine.

if the weather is forecasted at "scattered thunderstorms" every day this summer.... well you are using a lot of SRAM... think about a strcmp() there too...

Then, make the struct the same for every day, even the first two... and for them, parse differently into the same struct.

#3

Is there a reason why you aren’t using strtok():

void setup() {
  char testInput[] = "Wednesday;2.00;7.00;0.00;Partly Cloudy;77.00";
  char *parts[6];
  char *ptr;
 
  Serial.begin(9600);
  ptr = strtok(testInput, ";");
  int i = 0;
  while (ptr != NULL) {
    parts[i++] = ptr;
    ptr = strtok(NULL, ";");
  }

  for (i = 0; i < 6; i++) {
    Serial.println(parts[i]);
  }
}

void loop() {
}
#4