G6EJD / ESP32-e-Paper-Weather-Display

An ESP32 and 2.9", 4.2" or 7.5" ePaper Display reads Weather Underground data via their API and then displays the weather
Other
946 stars 206 forks source link

Graph for snow will not be displayed #196

Closed dreamy1 closed 1 year ago

dreamy1 commented 1 year ago

Hello,

ist seems that the graph for snow an rain is fixed to the rain.

In the code, there is a part for getting the "SumOfPrecip" of the total rain and the total snow an the higher one "wins" to be displayed:

_if (SumOfPrecip(rain_readings, max_readings) >= SumOfPrecip(snow_readings, max_readings)) DrawGraph(gx + 3 gap + 5, gy, gwidth, gheight, 0, 30, Units == "M" ? TXT_RAINFALL_MM : TXT_RAINFALL_IN, rain_readings, max_readings, autoscale_on, barchart_on); else DrawGraph(gx + 3 gap + 5, gy, gwidth, gheight, 0, 30, Units == "M" ? TXT_SNOWFALL_MM : TXT_SNOWFALL_IN, snow_readings, max_readings, autoscale_on, barcharton);

But it seems that this subroutine "SumOfPrecip" misses.

dreamy1 commented 1 year ago

Ah, this part is in the "common functions.h" - but it always returns 0.

dreamy1 commented 1 year ago

Maybe it would be best to cumulate the precipitation (rain + snow) and show it into the graph. So there would be no irritation if snow AND rain occurs within the next 3 days?

G6EJD commented 1 year ago

OWM don’t issue simultaneous rain and snow values only as an OR and it’s all precipitation

dreamy1 commented 1 year ago

It is concerning the graph on the right corner only. If there is only snow in the next 3 days, it will never be displayed as graph because "SumOfPrecip(rain_readings, max_readings)" and SumOfPrecip(snow_readings, max_readings) always return 0.0.

So only the graph for rain is displayed because the "if (SumOfPrecip(rain_readings, max_readings) >= SumOfPrecip(snow_readings, max_readings))" is always true (both routines returns 0.0) and the "else" part is never executed with drawing the graph with snow in mm and headline "snow".

dreamy1 commented 1 year ago

Here is the solution that works correct (sorry, i am a software looser):

Delete this part: float SumOfPrecip(float DataArray[], int readings) { float sum = 0; for (int i = 0; i <= readings; i++) { sum += DataArray[i]; } return sum; }

Replaced by: float SumOfRain(float numberofreadings) { float sumRain = 0; for (int i = 0; i <= numberofreadings; i++){ sumRain += WxForecast[i].Rainfall;} return sumRain; } float SumOfSnow(float numberofreadings) { float sumSnow = 0; for (int i = 0; i <= numberofreadings; i++){ sumSnow += WxForecast[i].Snowfall;} return sumSnow; }

And this part is modified, you can see on the serial output that the addition of the cumulated rain or snow is now correct:

Serial.println("SumRain(mm):" + String(SumOfRain(max_readings))); Serial.println("SumSnow(mm):" + String(SumOfSnow(max_readings))); // if (SumOfPrecip(rain_readings, max_readings) >= SumOfPrecip(snow_readings, max_readings)) if (SumOfRain(max_readings) >= SumOfSnow(max_readings)) DrawGraph(gx + 3 gap + 5, gy, gwidth, gheight, 0, 30, Units == "M" ? TXT_RAINFALL_MM : TXT_RAINFALL_IN, rain_readings, max_readings, autoscale_on, barchart_on); else DrawGraph(gx + 3 gap + 5, gy, gwidth, gheight, 0, 30, Units == "M" ? TXT_SNOWFALL_MM : TXT_SNOWFALL_IN, snow_readings, max_readings, autoscale_on, barchart_on); }

The right graph now shows the higher precipitation for the next 3 days (rain or snow). A cumulation of rain and snow would be the best, but this would be a bigger effort.

jpolton commented 1 year ago

It is concerning the graph on the right corner only. If there is only snow in the next 3 days, it will never be displayed as graph because "SumOfPrecip(rain_readings, max_readings)" and SumOfPrecip(snow_readings, max_readings) always return 0.0.

So only the graph for rain is displayed because the "if (SumOfPrecip(rain_readings, max_readings) >= SumOfPrecip(snow_readings, max_readings))" is always true (both routines returns 0.0) and the "else" part is never executed with drawing the graph with snow in mm and headline "snow".

@dreamy1 The problem you raise is interesting. Could you explain why SumOfPrecip(snow_readings, max_readings) == 0.0 when there is only snow for the next 3 days? Looking through the code I can not see how this happens, but it seems like an important bug observation if true.

dreamy1 commented 1 year ago

It seems that there is a problem with filling the array or with the array declaration itself in the original code, but i am not able to explain what exactly is the reason.

In fact both routines in the original code returns 0.0, i have tested that with a serial output of both values. With my modification both values are correct - you can see that in the serial output.

G6EJD commented 1 year ago

I don't see how this section of code: 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);

Does not work?

Don't forget these are the forecast values. Also OWM does not issue a a forecast until after about 10:00 local time, but it can range from 07:00 to 12:00 and sometimes not at all, this is why in some code examples I place a * on the display and use the last know value for either rain or snow.

dreamy1 commented 1 year ago

The single readings work as intended, but not the summation. You're at the wrong place.

What i found out, it is a problem here: float SumOfPrecip(float DataArray[], int readings). The summation always return 0.0.

And again, simple add a serial output in the void "DisplayForecastSection(int x, int y)" and you will see that the output of the two precip values (original code) always are 0.0 and after modification the two lines (my modification above) now outputs the correct summation of the values.

Before: Serial.println("Sum rain(mm):" + String(SumOfPrecip(rain_readings, max_readings))); Serial.println("Sum snow(mm):" + String(SumOfPrecip(snow_readings, max_readings)));

After: Serial.println("SumRain(mm):" + String(SumOfRain(max_readings))); Serial.println("SumSnow(mm):" + String(SumOfSnow(max_readings)));

And as mentioned above: we had snow some time ago with snow only and no rain and with the original code, the graph showed a "rain" graph with no precipitation. After my mod it shows the snow graph with the actual precipitation, so there is definitely a bug in the code. For me, it seems like a nesting bug with the array declaration.

G6EJD commented 1 year ago

I will run a test and see why the compiler is not parsing by value, it looks initially like the compiler has been changed as it did all work. I could change the code with one letter to make it parse by reference. I’ll see.

dreamy1 commented 1 year ago

Sure it is a small thing in the code, maybe the value "DataArray[]" is defined only inside the void and outside it stays 0, so there is a problem with the handing over to the main loop.

Maybe you will have a look at the additional issue mentioned above: even with correct cumulation of the single values, only the prediction of the higher sum will be displayed in the graph. If you have 20mm rain in the next days and 19mm snow, only the rain values will be displayed in the graph. Here it would be nice to see the general prediction, no matter if it is rain or snow...e.g. rain with a black bar and snow with a grey bar.

G6EJD commented 1 year ago

So it seems in the earlier versions of the compiler it took the sizeof(array) as the parameter by default, now it doesn't, so I corrected it with: const int Rain_array_size = sizeof(rain_readings) / sizeof(float); const int Snow_array_size = sizeof(snow_readings) / sizeof(float);

And now parse the array size rather than max_readings value.

@dreamy1 as far as I know OWM never issues a Rain AND Snow forecast together eithers either one or the other, they classify it all as Precipitation.

dreamy1 commented 1 year ago

Yes, a single 3h api fragment only shows rain OR snow (i think they deliver the higher one). With every API request you will get 40 records (3h values), 24 of them are used in your code (=maxreadings).

What your code does: Every bar in the right graph shows the 3h value (24 in sum over the x axis)- but with your code, only rain or only snow is displayed, depending of the sum of rain or snow over the 3 days.

So if you have 2mm rain in the first record and 2mm snow in the second record and it is more rain than snow within 3 days, the graph shows only the 2mm rain at the first bar and the second bar is 0mm (because it is snow)...and so on. The 3d graph only shows rain readings OR snow readings, but not both at the same time. But the data for each bar is already there...

G6EJD commented 1 year ago

Check the code amendments, SumofPrecip now works. Yes it’s possible to display both, just remove the if statement test, but there isn’t enough screen estate to do that, the title could be changed to precipitation and the sun be altered to both rain and snow.

dreamy1 commented 1 year ago

Thanks, i will update my code (it is a special version for a 4.7" e-paper without wind compass and some modifications).

Removing the if statement...hmmm...so the graph would be drawn two times. Maybe the single values would be overwritten with the second draw, so i am not sure if that would work (e.g. 1mm rain for the first record, 0mm snow for the first record - so the second drawing of the graph would result in a 0.0 bar).

Maybe the easier way would be to use the half width of a single bar - rain uses the first half, snow the second. So there would be no problem with dual drawing.

G6EJD commented 1 year ago

I was meaning draw both charts for rain and snow, but the way it is now it will (OWM make it exclusive) either display as a forecast rain or snow so it should be ok.

dreamy1 commented 1 year ago

Ah ok, that would be a solution too to draw two graphs side by side - but in summer, a snow graph is superfluous and it takes extra space.

In my opinion, drawing a single graph with the title "precipitation" and dedicated bars for snow/rain would be best. The data is already there within the array with the forecast records for each 3h - so all what have been done is to look if the value is rain or snow (one is always zero if the api delivers only one value for each record) before drawing each single bar. Depending of the rain/snow information e.g. a filled bar for rain and a unfilled bar for snow would be implemented and we have a really nice solution.

The actual implementation is suboptimal, i had a lot of empty bars in the past while it was snowing the hole day because the sum of precipitation of rain was higher within 3 days.