GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — weather: Add new app with forecast by vkareh · Pull Request #1995 · InfiniTimeOrg/InfiniTime Conversation
|
|
Build size and comparison to main:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
One small thing, for the rest this looks good!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Oh oopsie, that was my code in GB 😅 Thanks for finding and submitting a fix. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Unfortunately no. With the ° symbol things start wrapping when you get to double-digit negative temperatures. This is also already the smallest font available.
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. Thoughts? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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.) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@FintasticMan thanks - formatting is fixed now. The sim build breaks due to a new operator override in the Forecast struct. Should be fixed by InfiniTimeOrg/InfiniSim#138 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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... |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Do positive numbers above 100 display fine? That is possible with Fahrenheit I think... |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Yep, they look fine. The problem would be if it gets to 100 below zero (e.g. Acceptable display range would then be |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
I'd be more than happy to take suggestions on how to improve the thing. Here are two options using the InfiniTimeTheme colors: 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... |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
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'? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Thanks! I love this idea.
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
What are your thoughts on this? Anything 0°C or below is blue, 40°C or above is red, anything else is yellowish: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Maybe increase the space between the big temperature and the description above? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I tried putting the current day/time (last updated time, really) but it was hard to align everything. It was just too cluttered... |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
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:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@MoonlightWave-12
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@MoonlightWave-12
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) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
698ae9a to
ce5f0d9
Compare
|
We could center the cells and pad them with spaces if necessary. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Good thinking! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@FintasticMan @minacode - success! I managed to use character padding to align temperatures: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I'm very happy to see this new app since the protocol allows for forecast data, but we had no way of displaying those info so far!
This feature looks good overall and I guess we'll be able to merge it as soon as the remaining code change requests and review are addressed :)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| tempUnit = 'F'; | ||
| } | ||
| temp = temp / 100 + (temp % 100 >= 50 ? 1 : 0); | ||
| maxTemp = maxTemp / 100 + (maxTemp % 100 >= 50 ? 1 : 0); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This calculation is done multiple times (and it's not easy to understand what it does - rounding?). Consider moving it to a dedicated function. The name of this function will also help understanding the meaning of it.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is so that it gets rounded properly. I added it to the PTS watch face, but maybe we should add a function to SimpleWeatherService that does this calculation.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I've just created that function in #2015.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I added this function directly in the weather app to avoid having to push a new change to InfiniSim, but let me know if we should change this.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to have the function in the weather service tbh, but it doesn't matter so much.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
df9306a to
a259df9
Compare
This allows it to be used outside of the current datetime context and makes it consistent with the MonthShortToStringLow function.
This ensures temperatures are correctly aligned with one another
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for your work @vkareh !
I've just tested it with Tiny Weather Forecast Germany and Gadgetbridge :

Reminder : the current version of Gadgetbridge (0.78) has a bug that prevents forecast from being sent to the watch (https://codeberg.org/Freeyourgadget/Gadgetbridge/pulls/3551). The fix is merged, so I guess it'll be available in 0.79 (which is already tagged but not available in f-droid).
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Out of curiosity: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.