PlummersSoftwareLLC / NightDriverStrip

NightDriver client for ESP32
https://plummerssoftwarellc.github.io/NightDriverStrip/
GNU General Public License v3.0
1.32k stars 213 forks source link

Fix and remove weather images #256

Closed davepl closed 10 months ago

davepl commented 1 year ago

Bug report

We need a good set of open-source weather images from somewhere, or someone needs to draw a consistent set. Then remove and unused images from bmp/data

lavolla commented 1 year ago

will this project work for weather icon replacements?

weather icons

VeniceInventors commented 1 year ago

will this project work for weather icon replacements?

Not directly, the icons from your link actually are monochrome glyphs from TrueType fonts, the weather images used in this project are color bitmaps.

I'll create a set of bitmaps and post it here soon.

Fingez commented 1 year ago

16 X 16 Weather Icons.zip

I had a crack at making some

davepl commented 1 year ago

I think these are great, but I was wondering if you'd consider doing a second set with color for the daytime, and we could use these for evening? I think they look great, but want to take advantage of the RGB screen as well!

Fingez commented 1 year ago

16 x 16 Weather Icons colorized.zip

I originally thought the 'Monochrome' would help in making the icons more clear, but I was clearly wrong. The subtle color certainly helps differentiate the snow from the rain for example. This set has inconsitencies between each icon, but I left them that way for now to see if anyone prefers the feel of a particular one, I can then go and fix the others to match better.

davepl commented 1 year ago

I like them a lot, but any chance you’d consider using color? Or if you’re willing to do both, we could use these as the “dark” night theme and the colored ones for daytime. That’d make the sun yellow, the snow light blue, and so on…

Let me know what you think!

Dave

On May 29, 2023, at 12:46 PM, Fingez @.***> wrote:

16 X 16 Weather Icons.zip https://github.com/PlummersSoftwareLLC/NightDriverStrip/files/11593880/16.X.16.Weather.Icons.zip I had a crack at making some

— Reply to this email directly, view it on GitHub https://github.com/PlummersSoftwareLLC/NightDriverStrip/issues/256#issuecomment-1567461835, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4HCF2FO66PI4R7CRPKSS3XIT4LBANCNFSM6AAAAAAXQPCD4E. You are receiving this because you were assigned.

VeniceInventors commented 1 year ago

weather.zip Here a set I made in Paint.net with some extra icons. I'm not sure how they'll look on LEDs, hopefully there's no flickering from PWM with lower intensity shades.

Fingez commented 1 year ago

I like them a lot, but any chance you’d consider using color? Or if you’re willing to do both, we could use these as the “dark” night theme and the colored ones for daytime. That’d make the sun yellow, the snow light blue, and so on… Let me know what you think! - Dave On May 29, 2023, at 12:46 PM, Fingez @.***> wrote: 16 X 16 Weather Icons.zip https://github.com/PlummersSoftwareLLC/NightDriverStrip/files/11593880/16.X.16.Weather.Icons.zip I had a crack at making some — Reply to this email directly, view it on GitHub <#256 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4HCF2FO66PI4R7CRPKSS3XIT4LBANCNFSM6AAAAAAXQPCD4E. You are receiving this because you were assigned.

I attached the colored set on last message, unless you mean you would like them even more colorful? There is only a few colors you can relate to weather though. :D

mggates39 commented 11 months ago

I think I will take the are that @VeniceInventors posted here a couple of months ago and get them into the effect. I will see if I can get day and night too as those images are mostly there too.

Marshall

mggates39 commented 11 months ago

Can someone pull this branch https://github.com/mggates39/NightDriverStrip/tree/feature/New-Weather-Icons and see if it looks okay. The branch compiles, but it is going to be this weekend before I have anything cobbled together to try to look at it. If it looks good, I will put up a PR for it. @rbergen or @robertlipe

rbergen commented 11 months ago

@mggates39 I only just noticed this, which is the reason it took a while before I responded.

I have checked out the branch and looked at the code, which already raises an interesting question, which is what is the definition of "tomorrow". This question is triggered by the fact you propose adding night-time icons, which I intuitively feel doesn't make sense for the "tomorrow" scenario. That is to say that I think the "tomorrow" part of the panel should show the weather for tomorrow day time, but I'm not sure exactly what time of the day to pick/aim for. It also means the current logic to extract the weather for tomorrow is quite possibly flawed: it picks the first weather entry that has tomorrow's date attached to it, which may well be a slot in the middle of the night.

On the other hand, showing night-time icons for the current weather does seem sensible.

I'm going to have to let this simmer for a bit to figure out what time of day is the most logical one to show tomorrow's weather for. I'll report back here when I've come to an opinion about this.

rbergen commented 11 months ago

@mggates39 Ok, so after giving it some thought and looking at the OpenWeatherMap API, I'd say the following:

I'd propose we make these changes before I start looking at the actual appearance of the weather image set; at least for me it'll be the comprehensive visualization that I'll base my conclusions on.

mggates39 commented 11 months ago

@rbergen That sounds like a solid plan. I will start on that tonight.

Also, the hardware I plan on using only has 4MB of Flash and even with the web server disabled I can't get it to fit so I have not been able to test it yet. I turned off every other effect to get it to fit. Now to get the hub connected to see it and then I can do more development.

mggates39 commented 11 months ago

I got it down to 4MB and can run it now. I had to turn off the web server and a bunch of effects. I will not be committing the changes to do that.

I found an issue with how I saved the new images breaking the Tiny JPEG library and fixed that. It does not like the Progressive format.

I have it going through all the record for the next day. I do that by taking the UTC epoch time as local time to compare to the date of tomorrow. I will probably average out the weather icons to get a good one for the day and force it to day time.

I should be able to get that buttoned up in the next day or so. I am going through Service Now Admin Training at work and it is taking up some of my spare time this week.

rbergen commented 11 months ago

I got it down to 4MB and can run it now.

Glad to hear that!

I have it going through all the record for the next day. I do that by taking the UTC epoch time as local time to compare to the date of tomorrow. I will probably average out the weather icons to get a good one for the day and force it to day time.

I like the idea, but I do wonder how you plan to "average out" weather icons? I live in a country where "four seasons in one day" is a thing, so I genuinely wouldn't know how to capture that in one icon. That's part of why I suggested we pick a particular time as "tomorrow's" weather; it may not always represent the day, but at least it's consistent concerning what part of the day it does show.

I should be able to get that buttoned up in the next day or so. I am going through Service Now Admin Training at work and it is taking up some of my spare time this week.

It's a small world once again. My current CEO used to be the Service Now CMO for the EMEA region.

mggates39 commented 11 months ago

My thought was to count the number of each of the icons, just using the number ignoring the day/night indicator and get the one with the most hits. Though always taking the icon for the noontime block would be easier.

rbergen commented 11 months ago

Yeah, that might work. We could just give that a try, monitor what it yields on the matrix for a while and then see if we feel it's so far off we need to reconsider.

mggates39 commented 11 months ago

Okay, I have the 'final' code up on my branch. I have pulled out my debugs and cleaned up the comments. It should be good to go now. I WILL get my matrix hooked up this weekend to look at it too.

Thank you for all your feedback.

mggates39 commented 11 months ago

Based on the images from https://github.com/PlummersSoftwareLLC/NightDriverStrip/discussions/483#discussioncomment-7590878 of my code running on the prototype, if no one has any objections I will put in a pull request this evening when I get home.

rbergen commented 11 months ago

As far as I'm concerned there will never be an objection to any PR being opened that is rooted in a genuine intent to contribute something useful. It's also when I - and I am only talking for myself - really start looking at the code (change) that's proposed. Any discussion and polishing that may lead to is just part of the process.

rbergen commented 11 months ago

The old weather icons have been replaced through the merge of #537. As @mggates39 intends to apply further improvements to the weather visualization I will keep this open to track those. Once all visualization PRs in the (upcoming) sequence have been merged I'll close this, even though further improvements of a different nature to the Weather effect may follow after that.

mggates39 commented 11 months ago

I think the next Pull Request with fix the issue with not properly getting the tomorrow's temperature range and weather icon. I will also take this opportunity to convert to std::chrono for the time base as well as fix a few debug messages severities.

mggates39 commented 10 months ago

I believe I have made the final Pull Request #558 for this Issue. I have tied up all my loose ends and put in what I think is a good set of comments for all the methods in the class. As always, I welcome any and all feedback.

mggates39 commented 10 months ago

@davepl if you are good with these changes, I think this issue can be closed.

rbergen commented 10 months ago

@mggates39 I'm going to close this now. I thought the merge of #558 would close this automatically, but I now see the phrasing used in the reference from that PR to this issue doesn't use the exact wording GitHub is looking for (in case you're interested: you put the word "issue" between "close" and the issue number, which breaks the link).

Thanks for pointing out this issue is still open.