DisplayForecastSection array access error

[squeakyneb]

In several versions of the code, there's a loop like:

  int r = 1;
  do {
    if (Units == "I") pressure_readings[r] = WxForecast[r].Pressure * 0.02953;   else pressure_readings[r] = WxForecast[r].Pressure;
    if (Units == "I") rain_readings[r]     = WxForecast[r].Rainfall * 0.0393701; else rain_readings[r]     = WxForecast[r].Rainfall;
    if (Units == "I") snow_readings[r]     = WxForecast[r].Snowfall * 0.0393701; else snow_readings[r]     = WxForecast[r].Snowfall;
    temperature_readings[r] = WxForecast[r].Temperature;
    humidity_readings[r]    = WxForecast[r].Humidity;
    r++;
  } while (r <= max_readings);

This indexes the range 1..24, when the valid range is 0..23 (with #define max_readings 24 and these arrays being defined as pressure_readings[max_readings] and so on). This (in my case at least) resulted in the timestamp of WxForecast[0].Dt getting clobbered with garbage and the first forecast box having a seemingly semi-random time.

This loop should be as follows, which seems to fix the issue

  int r = 0;
  do {
    if (Units == "I") pressure_readings[r] = WxForecast[r].Pressure * 0.02953;   else pressure_readings[r] = WxForecast[r].Pressure;
    if (Units == "I") rain_readings[r]     = WxForecast[r].Rainfall * 0.0393701; else rain_readings[r]     = WxForecast[r].Rainfall;
    if (Units == "I") snow_readings[r]     = WxForecast[r].Snowfall * 0.0393701; else snow_readings[r]     = WxForecast[r].Snowfall;
    temperature_readings[r] = WxForecast[r].Temperature;
    humidity_readings[r]    = WxForecast[r].Humidity;
    r++;
  } while (r < max_readings);

DrawGraph also seems to have some similar odd behaviour, with index 0 being skipped at multiple points, but I don't believe I really understand everything that this function is doing.

for (int i = 1; i < readings; i++ ) {
      if (DataArray[i] >= maxYscale) maxYscale = DataArray[i];
      if (DataArray[i] <= minYscale) minYscale = DataArray[i];
    }

for (int gx = 1; gx < readings; gx++) {
    x2 = x_pos + gx * gwidth / (readings - 1) - 1 ; // max_readings is the global variable that sets the maximum data that can be plotted
    y2 = y_pos + (Y1Max - constrain(DataArray[gx], Y1Min, Y1Max)) / (Y1Max - Y1Min) * gheight + 1;

It might be the case that index 0 has special meaning (though I don't think so? It's just extracted from WxForecast...), but at any rate, r <= max_readings is CERTAINLY an invalid access to those arrays.

[squeakyneb]

Yeah, the DrawGraph logic is also a bit off. Including this 0th data point actually shoots left out of the graph boundaries (since in the first loop, last_x = + 1 initially and when gx==0 then x2 = -1, (relative to x_pos), so the first line goes leftwards) and then continues right. Offsetting the X positions slightly fixes this and also corrects for the slight blank gap at the tail end of the graph. I think I've got a more correct result like so:

-  last_x = x_pos + 1;
+  last_x = x_pos;
   last_y = y_pos + (Y1Max - constrain(DataArray[1], Y1Min, Y1Max)) / (Y1Max - Y1Min) * gheight;
   display.drawRect(x_pos, y_pos, gwidth + 3, gheight + 2, GxEPD_BLACK);
   drawString(x_pos + gwidth / 2 + 6, y_pos - 16, title, CENTER);
   // Draw the data
-  for (int gx = 1; gx < readings; gx++) {
-    x2 = x_pos + gx * gwidth / (readings - 1) - 1 ; // max_readings is the global variable that sets the maximum data that can be plotted
+  for (int gx = 0; gx < readings; gx++) {
+    x2 = x_pos + gx * gwidth / (readings - 1) + 1; // max_readings is the global variable that sets the maximum data that can be plotted

(I'm doing this with a 3 colour display - making the line red made this one-or-two-pixel error much more obvious)

Before:

After:

[G6EJD]

There are a couple of aspects going on here, OpenWeatherMap do-not publish the Current Hour + 1 forecast value until about (I've seen it vary a lot) anytime between 09:00 and 11:00, often leaving the forecast zero value undefined (index 0). Then it suddenly appears and corrects itself. In some of the versions I introduced an average to cater for this and added a * to denote the forecast was not current.

If I was going to change this I'd want to be sure the change was not going to introduce other errors across time zones, so I'll have a look at it.

In the mean time, if you can correct your code and submit a change I'd appreciate that.

[squeakyneb]

Okay, can't say I'm familiar with the API in that depth. There could certainly be some details I'm overlooking here! That's one of the reasons I didn't submit this as a pull request (the other being that I've only got my somewhat tweaked version of the code and haven't had time to do a branch with just this fix).

I'll do a patch for the out of bounds access when I can.

[G6EJD]

Tested and Updated.