weathergov forecast not loading

[apiontek]

I found that regardless of config, the weathergov provider for the weather module would stall at "loading" for the type: 'forecast' weather config, although it worked fine for the 'current' weather config.

After searching issues I couldn't see any mention of it nor a solution, so I went through the code.

The issue seems to lie in the fetchWxGovURLs() function. For weather.gov, the provider first needs to determine, from the API, what the correct API URLs are for various kinds of data. It waits until the required URLs are populated, and then goes to fetch the actual weather data.

However, the finally() only calls this.fetchCurrentWeather() which does nothing if the user has configured the weather module to get the forecast.

I found that replacing the simple call to this.fetchCurrentWeather() as below fixes the issue for me:

        // handle both 'current' and 'forecast' config types
        if (config.type == 'forecast') {
          this.fetchWeatherForecast();
        } else if (config.type == 'current') {
          this.fetchCurrentWeather();
        }

This seems good enough. The provider doesn't support the further types of 'hourly' or 'daily' as far as I can tell, and I don't need them.

Cosmetically, the default apiBase is also wrong in the module, which shouldn't matter as long as users correctly configure the apiBase in config.js, but it ought to be "https://api.weather.gov/points/" instead of "https://api.weatherbit.io/v2.0"

I've can do a pull request for both of the above, just want to post the issue first.

[sdetweil]

change the initialLoadDelay config to 100?

the code does this, but timer of 0 is bad

scheduleUpdate: function (delay = null) { let nextLoad = this.config.updateInterval; if (delay !== null && delay >= 0) { nextLoad = delay; }

I had opened issue back in may, https://github.com/MichMich/MagicMirror/issues/2505

[apiontek]

Changing the load delays isn't necessary if the fetchWxGovURLs() method is fixed. It already properly calls this.fetchCurrentWeather() when it has the correct URLs. But if user config is for 'forecast' rather than 'current' it should call this.fetchWeatherForecast() instead, which the above modification handles.

To be clear I'm specifically talking about the weathergov.js provider only, nothing general to the weather module.

[sdetweil]

the problem is that the fetch and construction of the proper endpoint urls is variable length time..

SOMETIMEs your solution will work.. there is no callback to make it certain the have completed before attempting to use the constructed urls

[apiontek]

I thought that's why the this.fetchCurrentWeather() was placed in the finally clause. I'm not sure why it's there if it's not to try to get weather once the URLs are present. It seems if fetching the forecast doesn't belong there, then neither does fetching current weather.

[apiontek]

Ideally I am loading both current and forecast. For me, current always loads, and forecast stalls. Based on the idea of the "initialLoadDelay" I imagine if I did nothing, then on the updateInterval it might work.

I can confirm that if I set the initialLoadDelay for the forecast config to something much higher like 750, it loads shortly after the current weather, without the modifications to weather.js

That said, I'm still not sure why the code calls fetchCurrentWeather there but not fetchWeatherForecast. It just seems like a giant "why not?" to me.

[sdetweil]

@apiontek

I imagine if I did nothing, then on the updateInterval it might work.

correct.. if u change the updateInterval to shorter period then forecast updates sooner I didn't write code code and don't understand the intent of the design or api endpoint design in concert with the module.. or why initialLoadDelay defaults to 0, if its needed to make forecast work..

[apiontek]

In my read of the code, it's specifically checking if it has the URLs it needs, and then calling a weather data fetch, but only current. My suggestion is simply to get the forecast instead of current if user has that configured. Might alleviate some future headaches for people who don't know about the initialLoadDelay workaround.

[stale[bot]]

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.

[khassel]

already marked as stale, should be closed @MichMich