Fix forecast attribution priority: prefer actual data source over configured provider - #306
Open
smeisens wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
OSApp.Weather.showForecast()inwww/js/modules/weather.js(line ~798) builds theforecast page's attribution text using:
weather.weatherProvideris a mandatory field on theWeatherDatainterface inOpenSprinkler-Weather — every provider implementation sets it to its own identifier
(
"OpenMeteo","local","Apple", etc.), and the/weatherDataendpoint alwayspasses it through unchanged. It reliably reflects the actual source of the displayed
forecast data.
controller.settings.wto.provider, by contrast, reflects the configured mainprovider — which is checked first here and, being almost always present, wins before
weather.weatherProvideris ever consulted.Why this matters
For every setup to date, these two values have always been identical (a single
provider serves the whole response), so the priority order has been harmless in
practice. However, prioritizing configuration over actual data origin is a latent
inconsistency: it assumes the two always match rather than reflecting what was
genuinely returned.
This stops being harmless once a backend can serve forecast data from a source
different from the main configured provider — for example,
OpenSprinkler/OpenSprinkler-Weather#184 adds an independently
configurable
FORECAST_WEATHER_PROVIDERfor the rain-forecast restriction andforecast display, decoupled from the main
WEATHER_PROVIDERused for Zimmerman/ETo.With that change, a user could have
WEATHER_PROVIDER=local(their own PWS) andFORECAST_WEATHER_PROVIDER=Apple— and the App would display "Powered by your LocalPWS" for data that actually came from Apple WeatherKit, which could be a problem
under Apple's attribution requirements.
Fix
Swap the priority so the actual data source is preferred when available, falling
back to the configured provider only if the response doesn't specify one:
This is a no-op for every existing setup (the two values have always matched), and
fixes the display correctly for backends that can source forecast data
independently.
Testing
Manually verified with a local OpenSprinkler-Weather instance running
WEATHER_PROVIDER=local+FORECAST_WEATHER_PROVIDER=OpenMeteo— attribution nowcorrectly shows "Powered by Open Meteo" instead of "Powered by your Local PWS".