MagicMirror icon indicating copy to clipboard operation
MagicMirror copied to clipboard

weather module, weatherbit provider doesn't use type to set the endpoint, forces user..

Open sdetweil opened this issue 3 years ago • 11 comments

all in title

		{
			module: "weather",
			position: "top_right",
			header: "Weather Forecast",
			config: {
				updateInterval: 15*1000,
				initialLoadDelay: 2,

				weatherProvider: "weatherbit",
				weatherEndpoint:"/forecast/daily",  // < --- leave this off and module doesn't work , isn't that what type is supposed to FIX..
				type: "forecast",   // <--  but provider doesn't use this 'required' parm..
				lat: 30.4548443,
				lon: -97.6222674,
				apiKey: "",
			}
		},

sdetweil avatar Aug 15 '22 03:08 sdetweil

yeah, seems wrong to not use the "type" param which is used in other modules and specifically mentioned in the docs.

openweathermap has both params too but only uses the type only when weatherEndpoint is not specified.

maybe switch the usage in this module to that of openweathermap?

rejas avatar Aug 16 '22 20:08 rejas

yeh, i did local change to use type.. will fix to override that with the actual endpoint.. but how do you tell user specified ? cause its a default value.. in the provider

	defaults: {
		apiBase: "https://api.weatherbit.io/v2.0",
		weatherEndpoint: "/current",

this.config.weatherEndpoint != this.defaults.weatherEndpoint

sdetweil avatar Aug 16 '22 21:08 sdetweil

With my PR the weatherEndpoint is only used if explicitly set in the config, and not per default. That way existing configs still function (if at all) but new users can rely on the type value to work.

rejas avatar Aug 17 '22 07:08 rejas

Looking at the code of other providers it seems that at least darksky should have the same problem...

rejas avatar Aug 17 '22 15:08 rejas

Looking at the code of other providers it seems that at least darksky should have the same problem...

I think we shouldn't investigate in this because "The Dark Sky API and website will continue to function until March 31st, 2023".

Source: https://blog.darksky.net/

khassel avatar Aug 17 '22 20:08 khassel

@khassel I think u meant invest (spend energy).

it's a tiny thing, for consistency sake

sdetweil avatar Aug 18 '22 00:08 sdetweil

thanks, yes ...

khassel avatar Aug 18 '22 16:08 khassel

It might be tiny, but I still would like to test it before commiting any change. So without any darksky credentials I wont be able to do it.

rejas avatar Aug 18 '22 19:08 rejas

send an email to my userid at gmail and I will send my key

sdetweil avatar Aug 18 '22 19:08 sdetweil

Thx for the key, fixed the metric stuff in my WeatherUnitCleanup-PR but somehow darksky seems ok with the "normal" config (type), can you confirm?

rejas avatar Aug 18 '22 21:08 rejas

yes, darksky worked with type and weatherProvider no other settings both types worked..

sdetweil avatar Aug 19 '22 13:08 sdetweil