@lavolp3 said in MMM-forecast-io -- Localized up to the minute weather:

Guys, going through the thread, I see that a lot of good ideas were implemented in forks but not sent as PRs. I think it would be a good idea to have some of these included into the original. dmcinnes/MMM-forecast-io, since this is the one referenced in the original MagicMirror.

@dmcinnes what do you think?
Do you want this rep to be bloated up or do you want it to be kept mostly as it is and leave major changes to forks?

I would expect at least to have some of @morozgrafix very good ideas implemented as optional (sunset, precipitation probability).
Also @rak 's ideas look very very promising but I admit might be a bit much as changes for the master.

I just submitted a PR for displaying Sunrise and Sunset times. https://github.com/dmcinnes/MMM-forecast-io/pull/22

I will try to find some time to improve wind forecast and get another PR submitted sometime this weekend.

I also just submitted a PR for wind speed and direction https://github.com/dmcinnes/MMM-forecast-io/pull/23

Precipitation probability forecast is also submitted as a PR: https://github.com/dmcinnes/MMM-forecast-io/pull/24

I also sneaked another PR that allows easy adjustments to forecast table look and feel: https://github.com/dmcinnes/MMM-forecast-io/pull/25

@lavolp3 One thing that I would suggest is to group your changes into relevant and smaller size chunks and submit separate PRs for those, instead of one massive PR. It’s easier for reviewers and for you in case you need to make changes based on comments. Cheers.

@lavolp3 I haven’t pulled from master repo in a while and not sure if my changes are compatible at this point. I’m happy to clean it up, test it and submit a PR. Just let me know.

I’ve added some of those on my local branch a long time ago. Feel free to pick from there https://github.com/morozgrafix/MMM-forecast-io/commit/972201f64d432c3c0863a7d76364e7f8af1bb817 and https://github.com/morozgrafix/MMM-forecast-io/commit/13ff8b78c86dc0905e46c11702d7173571517e79

My fork is fairly old and hasn’t been synced in a while, though.

It looks like this:

@Iseknutz I haven’t touched that code in about 9 months. I can certainly look at the URL and see if that can be implemented.

@Mykle1 invite sent to email listed in your profile. Thanks.

@Hein-Jan sorry it didn’t work, as I mentioned earlier I’m not familiar with ModuleScheduler.

@Maxzovirax Good point. It will not work with 2 modules running simultaneously because they both will pick up same broadcast message from node_helper.js. I wonder if modification of the module to allow display of multiple horoscope predictions in the same module would be sufficient. It will take a bit of time to do that modification and I’m going on a small vacation tomorrow. I will think on how it can be addressed. Thanks for bringing it up.