search

lmarzen / esp32-weather-epd

A low-power E-Paper weather display powered by an ESP32 microcontroller. Utilizes the OpenWeatherMap API.
GNU General Public License v3.0
2.33k stars 179 forks source link

Add daily precipitation feature. #98

Closed asandhu3 closed 2 months ago

asandhu3 commented 2 months ago

I think this adds a valuable piece of data to the forecast section. I've been using this for a while and thought I'd make some proper flags and logic for different units and throw it into a PR if you think it's useful to add. I attempted to add a 16x16 icon beside the number to indicate what it is but it looked a bit cluttered and the very small icons don't render too well IMO.

This is my first exposure to C++ so possibly some issues but feel free to review and offer suggestions!

lmarzen commented 2 months ago

Merged. @asandhu3 Thank you for your contribution!

I altered your PR to use the same PRECIP_UNITS as everywhere else instead of separate DAILY_PRECIP_UNITS. I also (unrelated to your changes) cleaned up the outlook graph while I was at it to only draw the precipitation y-axis labels when precipitation is forecasted since I got the idea from how you only display daily precip if it is actually forecasted.

You did a great job, made this feature ON by default.

asandhu3 commented 2 months ago

Awesome, thanks!

asandhu3 commented 2 months ago

I'm noticing one side affect of your changes that might impact the usefulness of this feature. I remember now why I had separated the units into their own config.

I used this feature to display POP on the graph, and mm on the weekly overview. That way I could at a glance see:

  1. Which days of the week it's expected to rain (and how much), and
  2. At what time of the current day it's expected to rain (on the graph)

By tying those units together it doesn't make that possible.

lmarzen commented 2 months ago

Whoops, should have discussed before changing it. I'll add that functionality back.

lmarzen commented 2 months ago

Reverted that change 3fc7cb121cb81c30aaccab5d5ca78774b3a1e781