Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New weather module: forecast and forecast/daily support #1513

Closed
fwitte opened this issue Jan 5, 2019 · 12 comments
Closed

New weather module: forecast and forecast/daily support #1513

fwitte opened this issue Jan 5, 2019 · 12 comments
Labels

Comments

@fwitte
Copy link
Contributor

fwitte commented Jan 5, 2019

Hi everybody,
I implemented the support for forecast and forecast/daily using openweathermap in PR #1514. As I did not have a apiKey to test the forecast/daily part, I downloaded a sample file from their website and included it in my testing. I am not sure if all cases are handled well, please do check, if you find any other issues.

Thank you very much @fewieden and @vincep5 for you hints and support!

Also before merging my code, we should discuss, if my code is looking as it should:

This part appears twice in the hourly forecast part and a third time (similarly) in the daily forecast part.

minTemp.push(forecast.main.temp_min);
maxTemp.push(forecast.main.temp_max);

if (forecast.hasOwnProperty("rain")) {
	if (this.config.units === "imperial" && !isNaN(forecast.rain["3h"])) {
		rain += forecast.rain["3h"] / 25.4;
	} else if (!isNaN(forecast.rain["3h"])){
		rain += forecast.rain["3h"];
	} else {
		rain += 0;
	}
} else {
	rain += 0;
}

Have a nice weekend!

@vincep5
Copy link
Contributor

vincep5 commented Jan 5, 2019

Thanks, looks great. I took the code and put in directly in my directory. It works for my /forecast/daily. Just 1 small thing. When the rain is 0 we should hide it from displaying: in weather.js line 201 : we can put if (isNaN(value) || value == 0) {

@fwitte
Copy link
Contributor Author

fwitte commented Jan 6, 2019

Rain values of 0 should not be displayed anymore, also I simplified the code. One appearance of the code mentioned above was not required!

@rts58
Copy link

rts58 commented Jan 14, 2019

I've been trying to get the forecast to work with the new weather module. Weather works fine (for openweathermap not darksky), but if I add config for "forecast" in using either openweathermap or darksky I get "Loading …" as the header above the daily forecast.

		{
			module: "weather",
			position: "top_right",
			config: {
				// See 'Configuration options' for more information.
				type: 'forecast',
				weatherProvider: "openweathermap",
				location: "Holliston",
				locationID: "4939693",  
				apiKey: mykeyxxx,
				weatherEndpoint: "/forecast",
				useBeaufort: false,
				roundTemp: true
			}
		},
		{
			module: "weather",
			position: "top_right",
			config: {
				type: "forecast",
				weatherProvider: "darksky",
				apiBase: 'https://cors-anywhere.herokuapp.com/https://api.darksky.net',
				apiKey: mykeyxxx,
				lat: 42.184,
				lon: -71.431,
				weatherEndPoint: "/forecast"
			}
		},

Any suggestions for how to research this further??

@fwitte
Copy link
Contributor Author

fwitte commented Jan 15, 2019

Are you on the dev-branch? If not, the "/forecast"-Endpoint will not work until the next release.

@rts58
Copy link

rts58 commented Jan 15, 2019

@fwitte thanks, no I was not. I'll give it a try again.

@sebien0077
Copy link

i try this new module , and i have same problem "Loading …"
when i want forecast with openwatermap:
module: "weather", position: "top_right", config: { // See 'Configuration options' for more information. type: 'forecast', colored: true, apiKey: "myapiKey", }

Else, i think it's better if you change in weather.css
line-height: 65px; by
line-height: 30px;

@rts58
Copy link

rts58 commented Jan 23, 2019

I've pulled down the latest update from the develop branch (#1538). This cleans up the Loading … but now just shows Loading .... I can get the forecast from darksky, but not from openweathermap. I uncommented the line in forecast.njk to show the dump of forecast, but this just returns null. My config entry looks like:

		{
			module: "weather",
			position: "top_left",
			config: {
				type: 'forecast',
				weatherProvider: "openweathermap",
				locationID: "4939693",  
				apiKey: "mykeyxxx",
				weatherEndpoint: "/forecast"
			}
		},

What should I look at next?

@vincep5
Copy link
Contributor

vincep5 commented Jan 23, 2019

Try browsing to your MM via IP address and check the developer toolbar or similar in your browser. It might have some JS errors or more information

@rts58
Copy link

rts58 commented Jan 23, 2019

Thanks, I see this error:

openweathermap.js:55 Could not load data ...  TypeError: Cannot read property '3h' of undefined
    at Class.generateWeatherObjectsFromForecast (openweathermap.js:139)
    at fetchData.then.data (openweathermap.js:51)

@rts58
Copy link

rts58 commented Jan 23, 2019

Okay, I played around with the config settings. I looked at the API and it recommended only passing the location ID to insure an accurate result. Although not shown above apparently I also had location: . I removed it as above and it is now returning a result. Sorry for the distraction. Although, you may want to consider removing location: from config settings. @vincep5 Thanks for your help.

@stale
Copy link

stale bot commented Jul 2, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants
@rts58 @fwitte @vincep5 @sebien0077 and others