InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.71k stars 925 forks source link

weather: Add new app with forecast #1995

Closed vkareh closed 7 months ago

vkareh commented 8 months ago

Added a new weather app that shows the current weather and forecast. If forecast is not available, it just shows the current weather.

InfiniSim_2024-02-02_145908

Building the sim requires https://github.com/InfiniTimeOrg/InfiniSim/pull/138

github-actions[bot] commented 8 months ago
Build size and comparison to main: Section Size Difference
text 377080B 3840B
data 940B 0B
bss 63516B 0B
FintasticMan commented 8 months ago

Looks good! Is there enough space to add the ° symbol to the forecast? It looks a bit off without it IMO. It might also be nice to know the current min and max temp?

About the forecast not showing up with Breezy Weather + Gadgetbridge, could you check if it works with Tiny Weather Forecast Germany? The Gadgetbridge side should be working, so I suspect that Breezy Weather just doesn't provide that information.

vkareh commented 8 months ago

Ha! Found the issue. The wrong variable was being referenced in Gadgetbridge and so was not sending forecast data. I've submitted a patch to fix it: https://codeberg.org/Freeyourgadget/Gadgetbridge/pulls/3551

@FintasticMan Thanks for reviewing, I'll start addressing these issues sometime today.

FintasticMan commented 8 months ago

Oh oopsie, that was my code in GB :sweat_smile:

Thanks for finding and submitting a fix.

vkareh commented 8 months ago

@FintasticMan

Looks good! Is there enough space to add the ° symbol to the forecast? It looks a bit off without it IMO.

Unfortunately no. With the ° symbol things start wrapping when you get to double-digit negative temperatures. This is also already the smallest font available.

It might also be nice to know the current min and max temp?

Added that now. Also added the location. Maybe it's too busy?

I also made the temperature yellow so that it stands out a bit more from the rest.

InfiniSim_2024-02-01_102010

Thoughts?

FintasticMan commented 8 months ago

I think that looks good, but I'm not exactly the best at UI design. If some more people chime in, I think that this is good! (You do still need to fix some formatting FYI. You can use the git pre-commit hook to do this automatically.)

vkareh commented 8 months ago

@FintasticMan thanks - formatting is fixed now. The sim build breaks due to a new operator override in the Forecast struct. Should be fixed by https://github.com/InfiniTimeOrg/InfiniSim/pull/138

vkareh commented 8 months ago

I removed the location display. It was causing my watch to crash and reboot. I might debug it later, but easier to remove it for now...

InfiniSim_2024-02-01_112143

FintasticMan commented 8 months ago

Do positive numbers above 100 display fine? That is possible with Fahrenheit I think...

vkareh commented 8 months ago

Yep, they look fine. The problem would be if it gets to 100 below zero (e.g. -100 has 4 characters), but we have far bigger problems if that's the case :laughing:

Acceptable display range would then be -99 to 999, from a purely technical point of view.

InfiniSim_2024-02-01_113440

minacode commented 8 months ago

This looks very good! I am looking forward to use this app once the support in InfiniLink is ready.

One small thing: you could use InfiniTimes theme colors.

vkareh commented 8 months ago

@minacode

One small thing: you could use InfiniTimes theme colors.

I'd be more than happy to take suggestions on how to improve the thing. Here are two options using the InfiniTimeTheme colors:

InfiniSim_2024-02-01_130349 InfiniSim_2024-02-01_130434

Any preference? Other color combinations? I haven't been able to figure out how to change the color of the text in the forecast table...

KaffeinatedKat commented 8 months ago

that looks incredible, great work. Would it be possible to have the color change based on the temp? (eg. blue for cold and yellow/red for hot). That would really help bring it together imo

JustScott commented 8 months ago

Wow this looks great! Do you think it would be possible to fit the date and/or week day above or below each forecast. Example: 'thu', 'fri', '2/1', '2/17'?

vkareh commented 8 months ago

that looks incredible, great work. Would it be possible to have the color change based on the temp? (eg. blue for cold and yellow/red for hot). That would really help bring it together imo

Thanks! I love this idea.

Wow this looks great! Do you think it would be possible to fit the date and/or week day above or below each forecast. Example: 'thu', 'fri', '2/1', '2/17'?

I'll try, but I suspect there won't be much room for this. Also, this app is already annoyingly heavy (3388B as of last build), and calculating the days based on timestamps and then displaying that on a table will eat up a lot more, I think.

vkareh commented 8 months ago

What are your thoughts on this? Anything 0°C or below is blue, 40°C or above is red, anything else is yellowish: InfiniSim_2024-02-01_132859 InfiniSim_2024-02-01_132904 InfiniSim_2024-02-01_133258

JustScott commented 8 months ago

Wow this looks great! Do you think it would be possible to fit the date and/or week day above or below each forecast. Example: 'thu', 'fri', '2/1', '2/17'?

I'll try, but I suspect there won't be much room for this. Also, this app is already annoyingly heavy (3388B as of last build), and calculating the days based on timestamps and then displaying that on a table will eat up a lot more, I think.

Perhaps just adding the weekday and date for today somewhere could work, like is done on the digital watch face where it's a less visible grey.

minacode commented 8 months ago

Maybe increase the space between the big temperature and the description above?

vkareh commented 8 months ago

How about now? InfiniSim_2024-02-01_154859 InfiniSim_2024-02-01_155008

I tried putting the current day/time (last updated time, really) but it was hard to align everything. It was just too cluttered...

KaffeinatedKat commented 8 months ago

that looks really awesome, we don't really need to show the current day/time because that's easily accessible elsewhere. Having the forecasted days shown is more important. Great work

MoonlightWave-12 commented 8 months ago

What are your thoughts on this? Anything 0°C or below is blue, 40°C or above is red, anything else is yellowish:

I think blue for ~10°C and lower, and red for ~30°C and higher would be more useful.

Personally, i would prefer it to be separated somewhat like this:


2 more suggestions:

vkareh commented 8 months ago

@MoonlightWave-12

The maximum should be at the top, because it is higher.

Good idea. Done: InfiniSim_2024-02-01_161858

With regards to temperature colors, anything between 0°C and 40°C starts to become really subjective. Where I live currently 5°C is a very warm winter day, whereas 30°C is a hot summer day. However, I grew up in a place where 90°F is a mild winter day.

I'm okay with your ranges, but would like some consensus before comitting to those numbers. I also propose these:

The 32°C number is from wikipedia but we can also use 27°C

vkareh commented 8 months ago

@MoonlightWave-12

Maybe also colour the forecast-values, so that one can see the temperature-ranges easily at a glance?

That'd be nice, but I haven't figured out how to apply color to the table cells, only the table border (which I made black to blend in with the background)

FintasticMan commented 7 months ago

Wow this is really coming together! I would suggest using 27 for hot rather than 32 personally, but I don't live anywhere very hot 😄. I'd say it'd be best to capitalise the first letter of the weekdays, as they are proper nouns, so it looks strange to me in lower-case.

vkareh commented 7 months ago

@FintasticMan: done!

InfiniSim_2024-02-01_172313 InfiniSim_2024-02-01_172333 InfiniSim_2024-02-01_172319 InfiniSim_2024-02-01_173145

vkareh commented 7 months ago

I studied the lvgl table spec and was able to simplify the table code and re-color some table elements: InfiniSim_2024-02-02_140145 I made them light gray for now but we can change them on an as-need basis.

KaffeinatedKat commented 7 months ago

is it possible to color the table temps based on the same schema as the current? Or do they all have to be the same color. If you can change them all individually see what the color temps look like on the forecast. It'll either look great or not so great

vkareh commented 7 months ago

@KaffeinatedKat - here's a selection with colorized forecast: InfiniSim_2024-02-02_145908 InfiniSim_2024-02-02_145858 InfiniSim_2024-02-02_145905 InfiniSim_2024-02-02_145902 InfiniSim_2024-02-02_150153

KaffeinatedKat commented 7 months ago

I know that this feature is probably outside the scope for this PR, but when this is merged this would be the perfect app for the swipe left action on the clock

vkareh commented 7 months ago

@KaffeinatedKat

I know that this feature is probably outside the scope for this PR, but when this is merged this would be the perfect app for the swipe left action on the clock

Last night I was thinking about a different feature. A setting where you can select a few apps (in a checkbox, for example) and those will be "always open" and you can cycle through them by swiping left. Also adding an auto-cycle option.

Definitely out of scope here. We can start a discussion thread about it outside this PR.

vkareh commented 7 months ago

~Bad news, the last commit (which colorizes the forecast temperatures) crashes my watch when loading the app... will debug and maybe revert depending on what I find.~ edit: Fixed

vkareh commented 7 months ago

Fixed the crash. It was happening due to the task refresh callback being called every 20ms, which is probably not enough time for the fully-colorized table to render correctly before the next callback happened. I bumped that to 1000ms and it fixed the issue - there's no need to refresh the weather data that often anyway.

minacode commented 7 months ago

What do you think about aligning the numbers per day?

Examples:

1
2

 1
12

 1
-3

  0
-11
vkareh commented 7 months ago

@minacode

What do you think about aligning the numbers per day?

not sure I understand this comment and the accompanying example

minacode commented 7 months ago

I'm sorry, I thought about right-aligning the temperatures in the table.

vkareh commented 7 months ago

@minacode

I'm sorry, I thought about right-aligning the temperatures in the table.

Good idea, I just pushed a new version with right-aligned temperatures. I think it looks good!

InfiniSim_2024-02-05_092207

minacode commented 7 months ago

Yes! It's a little easier to the eyes to grasp the structure.

FintasticMan commented 7 months ago

Is it always all the way right aligned? I.e. what will it do when you have a day with a 1-char value and one with a 2-char value?

vkareh commented 7 months ago

Is it always all the way right aligned? I.e. what will it do when you have a day with a 1-char value and one with a 2-char value?

Nothing different. The whole thing is an LVGL table with invisible borders, so it behaves like any numeric table (spreadsheet, etc.):

        +-----+-----+-----+-----+-----+-----+
right:  |    1|   -1|   11|  -11|  111| -111|
        +-----+-----+-----+-----+-----+-----+
center: |  1  |  -1 |  11 | -11 | 111 | -111|
        +-----+-----+-----+-----+-----+-----+
left:   |1    |-1   |11   |-11  |111  |-111 |
        +-----+-----+-----+-----+-----+-----+

The issue with anything other than right-aligned is that things look wrong, which is why spreadsheets typically default to right-aligning numbers.

This is probably a good visualization:

|right|centr|left |
+-----+-----+-----+
|    1|  1  |1    |
+-----+-----+-----+
|   -1|  -1 |-1   |
+-----+-----+-----+
|   11| 11  |11   |
+-----+-----+-----+
|  -11| -11 |-11  |
+-----+-----+-----+
|  111| 111 |111  |
+-----+-----+-----+
| -111| -111|-111 |
+-----+-----+-----+

Notice how the right-aligned column grows progressively from the right, and unit numbers are always aligned, as are the tens and hundreds, etc. Also the negative sign doesn't shift the number from alignment, as it's always away from the alignment.

FintasticMan commented 7 months ago

It would've been nice to have it align right as far as its column is wide, if that makes any sense. But as I understand it that's not possible with LVGL tables.

I was thinking something like this:

+---+  +---+  +---+  +---+
| 5 |  | 1 |  |  5|  | -2|
| 1 |  |-5 |  |-23|  |-30|
+---+  +---+  +---+  +---+

(imagine that the -5 in the second column is centered, and that the 1 is above the 5, not the -)

vkareh commented 7 months ago

But as I understand it that's not possible with LVGL tables.

Nah, doesn't seem to be. And to be honest, there's not a lot of room either. The screen is 240px wide, which makes each column 48px wide. That's barely enough for 3 characters (which is what we need: -99 to 999) and some padding so that things don't blend with one another.

FintasticMan commented 7 months ago

I'm still not sure about the alignment. I think that right-aligning the numbers makes them easier to read, but I think it looks wrong in this situation: InfiniSim_2024-02-08_223348

It also bugs me slightly that the weekday text and icon are also right-aligned.

Ideally, we'd have it look like this: InfiniSim_2024-02-08_223436 InfiniSim_2024-02-08_223504_edited InfiniSim_2024-02-08_223608_edited

Is this possible? If not, we should decide whether right-aligned looks better in most cases or not.

minacode commented 7 months ago

We could center the cells and pad them with spaces if necessary.

FintasticMan commented 7 months ago

Good thinking!

vkareh commented 7 months ago

Sounds like a good idea. I can try, but each cell will need to know how many digits are in the temperature above or below itself and then add padding based on that. Maybe just centering all of them is not that bad after all?

Anyway, I'll see what I can pull off next time I'm playing with this.

vkareh commented 7 months ago

@FintasticMan @minacode - success! I managed to use character padding to align temperatures:

tituscmd commented 7 months ago

Out of curiosity: Are the weekdays on the bottom displaying the next week, or is it the current week? If it is the current week, I think it would be a good idea to color the current day - maybe red?