Skip to content

Commit

Permalink
Use metric units internally in all weatherproviders (#2849)
Browse files Browse the repository at this point in the history
So finally I think this refactorin is ready to be reviewed :-)

DONE:
- [x] Removed all conversion functions for wind and temperature from
specific weatherproviders
- [x] Use internally only metric units: celsius for temperature, meters
per seconds for wind
- [x] Convert temp and wind into the configured units when displaying
data on the UI
- [x] look how beaufort calculation uses metrics, added knots as new
windunit
- [x] add more e2e tests 

Checked providers:
- [x] Darksky
- [x] EnvCanada
- [x] OpenWeatherMap
- [x] SMHI provider 
- [x] UK Met Office
- [x] UK Met Office DataHub
- [x] WeatherBit
- [x] WeatherFlow
- [x] WeatherGov

TODO in different tickets:
- check weatherproviders for usage of weatherEndpoint (as seen in
MagicMirrorOrg/MagicMirror-Documentation#131) -> see
#2926
- cleanup precipations -> #2953

Co-authored-by: veeck <michael@veeck.de>
  • Loading branch information
rejas and veeck committed Oct 24, 2022
1 parent 64ed5a5 commit 2d3940a
Show file tree
Hide file tree
Showing 19 changed files with 274 additions and 381 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Special thanks to: @rejas, @sdetweil
- Updated da translation
- Rework weather module
- Use fetch instead of XMLHttpRequest in weatherprovider
- Reworked how weatherproviders handle units
- Use unix() method for parsing times, fix suntimes on the way

### Fixed
Expand Down
10 changes: 1 addition & 9 deletions modules/default/weather/current.njk
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,7 @@
<div class="normal medium">
<span class="wi wi-strong-wind dimmed"></span>
<span>
{% if config.useBeaufort %}
{{ current.beaufortWindSpeed() | round }}
{% else %}
{% if config.useKmh %}
{{ current.kmhWindSpeed() | round }}
{% else %}
{{ current.windSpeed | round }}
{% endif %}
{% endif %}
{{ current.windSpeed | unit("wind") | round }}
{% if config.showWindDirection %}
<sup>
{% if config.showWindDirectionAsArrow %}
Expand Down
12 changes: 3 additions & 9 deletions modules/default/weather/providers/darksky.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,6 @@ WeatherProvider.register("darksky", {
lon: 0
},

units: {
imperial: "us",
metric: "si"
},

fetchCurrentWeather() {
this.fetchData(this.getUrl())
.then((data) => {
Expand Down Expand Up @@ -67,13 +62,12 @@ WeatherProvider.register("darksky", {

// Create a URL from the config and base URL.
getUrl() {
const units = this.units[this.config.units] || "auto";
return `${this.config.apiBase}${this.config.weatherEndpoint}/${this.config.apiKey}/${this.config.lat},${this.config.lon}?units=${units}&lang=${this.config.lang}`;
return `${this.config.apiBase}${this.config.weatherEndpoint}/${this.config.apiKey}/${this.config.lat},${this.config.lon}?units=si&lang=${this.config.lang}`;
},

// Implement WeatherDay generator.
generateWeatherDayFromCurrentWeather(currentWeatherData) {
const currentWeather = new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh);
const currentWeather = new WeatherObject();

currentWeather.date = moment();
currentWeather.humidity = parseFloat(currentWeatherData.currently.humidity);
Expand All @@ -91,7 +85,7 @@ WeatherProvider.register("darksky", {
const days = [];

for (const forecast of forecasts) {
const weather = new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh);
const weather = new WeatherObject();

weather.date = moment.unix(forecast.time);
weather.minTemperature = forecast.temperatureMin;
Expand Down
63 changes: 17 additions & 46 deletions modules/default/weather/providers/envcanada.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@
* https://dd.weather.gc.ca/citypage_weather/schema/
* https://eccc-msc.github.io/open-data/msc-datamart/readme_en/
*
* This module supports Canadian locations only and requires 2 additional config parms:
* This module supports Canadian locations only and requires 2 additional config parameters:
*
* siteCode - the city/town unique identifier for which weather is to be displayed. Format is 's0000000'.
*
* provCode - the 2-character province code for the selected city/town.
*
* Example: for Toronto, Ontario, the following parms would be used
* Example: for Toronto, Ontario, the following parameters would be used
*
* siteCode: 's0000458',
* provCode: 'ON'
Expand Down Expand Up @@ -64,10 +64,6 @@ WeatherProvider.register("envcanada", {
start: function () {
Log.info(`Weather provider: ${this.providerName} started.`);
this.setFetchedLocation(this.config.location);

// Ensure kmH are ignored since these are custom-handled by this Provider

this.config.useKmh = false;
},

//
Expand Down Expand Up @@ -150,7 +146,7 @@ WeatherProvider.register("envcanada", {
//

generateWeatherObjectFromCurrentWeather(ECdoc) {
const currentWeather = new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits);
const currentWeather = new WeatherObject();

// There are instances where EC will update weather data and current temperature will not be
// provided. While this is a defect in the EC systems, we need to accommodate to avoid a current temp
Expand All @@ -161,13 +157,13 @@ WeatherProvider.register("envcanada", {
// EC finds no current temp. In this scenario, MM will end up displaying a current temp of null;

if (ECdoc.querySelector("siteData currentConditions temperature").textContent) {
currentWeather.temperature = this.convertTemp(ECdoc.querySelector("siteData currentConditions temperature").textContent);
currentWeather.temperature = ECdoc.querySelector("siteData currentConditions temperature").textContent;
this.cacheCurrentTemp = currentWeather.temperature;
} else {
currentWeather.temperature = this.cacheCurrentTemp;
}

currentWeather.windSpeed = this.convertWind(ECdoc.querySelector("siteData currentConditions wind speed").textContent);
currentWeather.windSpeed = currentWeather.convertWindToMs(ECdoc.querySelector("siteData currentConditions wind speed").textContent);

currentWeather.windDirection = ECdoc.querySelector("siteData currentConditions wind bearing").textContent;

Expand All @@ -190,11 +186,11 @@ WeatherProvider.register("envcanada", {
currentWeather.feelsLikeTemp = currentWeather.temperature;

if (ECdoc.querySelector("siteData currentConditions windChill")) {
currentWeather.feelsLikeTemp = this.convertTemp(ECdoc.querySelector("siteData currentConditions windChill").textContent);
currentWeather.feelsLikeTemp = ECdoc.querySelector("siteData currentConditions windChill").textContent;
}

if (ECdoc.querySelector("siteData currentConditions humidex")) {
currentWeather.feelsLikeTemp = this.convertTemp(ECdoc.querySelector("siteData currentConditions humidex").textContent);
currentWeather.feelsLikeTemp = ECdoc.querySelector("siteData currentConditions humidex").textContent;
}
}

Expand Down Expand Up @@ -225,7 +221,7 @@ WeatherProvider.register("envcanada", {

const days = [];

const weather = new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits);
const weather = new WeatherObject();

const foreBaseDates = ECdoc.querySelectorAll("siteData forecastGroup dateTime");
const baseDate = foreBaseDates[1].querySelector("timeStamp").textContent;
Expand Down Expand Up @@ -326,7 +322,7 @@ WeatherProvider.register("envcanada", {
days.push(weather);

//
// Now do the the rest of the forecast starting at nextDay. We will process each day using 2 EC
// Now do the rest of the forecast starting at nextDay. We will process each day using 2 EC
// forecast Elements. This will address the fact that the EC forecast always includes Today and
// Tonight for each day. This is why we iterate through the forecast by a a count of 2, with each
// iteration looking at the current Element and the next Element.
Expand All @@ -335,7 +331,7 @@ WeatherProvider.register("envcanada", {
let lastDate = moment(baseDate, "YYYYMMDDhhmmss");

for (let stepDay = nextDay; stepDay < lastDay; stepDay += 2) {
let weather = new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits);
let weather = new WeatherObject();

// Add 1 to the date to reflect the current forecast day we are building

Expand Down Expand Up @@ -389,7 +385,7 @@ WeatherProvider.register("envcanada", {
const hourGroup = ECdoc.querySelectorAll("siteData hourlyForecastGroup hourlyForecast");

for (let stepHour = 0; stepHour < 24; stepHour += 1) {
const weather = new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits);
const weather = new WeatherObject();

// Determine local time by applying UTC offset to the forecast timestamp

Expand All @@ -399,7 +395,7 @@ WeatherProvider.register("envcanada", {

// Capture the temperature

weather.temperature = this.convertTemp(hourGroup[stepHour].querySelector("temperature").textContent);
weather.temperature = hourGroup[stepHour].querySelector("temperature").textContent;

// Capture Likelihood of Precipitation (LOP) and unit-of-measure values

Expand Down Expand Up @@ -450,7 +446,7 @@ WeatherProvider.register("envcanada", {
weather.minTemperature = this.todayTempCacheMin;
weather.maxTemperature = this.todayTempCacheMax;
} else {
weather.minTemperature = this.convertTemp(currentTemp);
weather.minTemperature = currentTemp;
weather.maxTemperature = weather.minTemperature;
}
}
Expand All @@ -463,14 +459,14 @@ WeatherProvider.register("envcanada", {
//

if (todayClass === "low") {
weather.minTemperature = this.convertTemp(todayTemp);
weather.minTemperature = todayTemp;
if (today === 0 && fullDay === true) {
this.todayTempCacheMin = weather.minTemperature;
}
}

if (todayClass === "high") {
weather.maxTemperature = this.convertTemp(todayTemp);
weather.maxTemperature = todayTemp;
if (today === 0 && fullDay === true) {
this.todayTempCacheMax = weather.maxTemperature;
}
Expand All @@ -482,11 +478,11 @@ WeatherProvider.register("envcanada", {

if (fullDay === true) {
if (nextClass === "low") {
weather.minTemperature = this.convertTemp(nextTemp);
weather.minTemperature = nextTemp;
}

if (nextClass === "high") {
weather.maxTemperature = this.convertTemp(nextTemp);
weather.maxTemperature = nextTemp;
}
}
},
Expand Down Expand Up @@ -536,31 +532,6 @@ WeatherProvider.register("envcanada", {
}
},

//
// Unit conversions
//
//
// Convert C to F temps
//
convertTemp(temp) {
if (this.config.tempUnits === "imperial") {
return 1.8 * temp + 32;
} else {
return temp;
}
},

//
// Convert km/h to mph
//
convertWind(kilo) {
if (this.config.windUnits === "imperial") {
return kilo / 1.609344;
} else {
return kilo;
}
},

//
// Convert the icons to a more usable name.
//
Expand Down
45 changes: 24 additions & 21 deletions modules/default/weather/providers/openweathermap.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ WeatherProvider.register("openweathermap", {
fetchCurrentWeather() {
this.fetchData(this.getUrl())
.then((data) => {
let currentWeather;
if (this.config.weatherEndpoint === "/onecall") {
const weatherData = this.generateWeatherObjectsFromOnecall(data);
this.setCurrentWeather(weatherData.current);
currentWeather = this.generateWeatherObjectsFromOnecall(data).current;
this.setFetchedLocation(`${data.timezone}`);
} else {
const currentWeather = this.generateWeatherObjectFromCurrentWeather(data);
this.setCurrentWeather(currentWeather);
currentWeather = this.generateWeatherObjectFromCurrentWeather(data);
}
this.setCurrentWeather(currentWeather);
})
.catch(function (request) {
Log.error("Could not load data ... ", request);
Expand All @@ -49,15 +49,17 @@ WeatherProvider.register("openweathermap", {
fetchWeatherForecast() {
this.fetchData(this.getUrl())
.then((data) => {
let forecast;
let location;
if (this.config.weatherEndpoint === "/onecall") {
const weatherData = this.generateWeatherObjectsFromOnecall(data);
this.setWeatherForecast(weatherData.days);
this.setFetchedLocation(`${data.timezone}`);
forecast = this.generateWeatherObjectsFromOnecall(data).days;
location = `${data.timezone}`;
} else {
const forecast = this.generateWeatherObjectsFromForecast(data.list);
this.setWeatherForecast(forecast);
this.setFetchedLocation(`${data.city.name}, ${data.city.country}`);
forecast = this.generateWeatherObjectsFromForecast(data.list);
location = `${data.city.name}, ${data.city.country}`;
}
this.setWeatherForecast(forecast);
this.setFetchedLocation(location);
})
.catch(function (request) {
Log.error("Could not load data ... ", request);
Expand Down Expand Up @@ -123,8 +125,9 @@ WeatherProvider.register("openweathermap", {
* Generate a WeatherObject based on currentWeatherInformation
*/
generateWeatherObjectFromCurrentWeather(currentWeatherData) {
const currentWeather = new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh);
const currentWeather = new WeatherObject();

currentWeather.date = moment.unix(currentWeatherData.dt);
currentWeather.humidity = currentWeatherData.main.humidity;
currentWeather.temperature = currentWeatherData.main.temp;
currentWeather.feelsLikeTemp = currentWeatherData.main.feels_like;
Expand All @@ -147,7 +150,7 @@ WeatherProvider.register("openweathermap", {
return this.fetchForecastDaily(forecasts);
}
// if weatherEndpoint does not match forecast or forecast/daily, what should be returned?
return [new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh)];
return [new WeatherObject()];
},

/*
Expand All @@ -158,7 +161,7 @@ WeatherProvider.register("openweathermap", {
return this.fetchOnecall(data);
}
// if weatherEndpoint does not match onecall, what should be returned?
return { current: new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh), hours: [], days: [] };
return { current: new WeatherObject(), hours: [], days: [] };
},

/*
Expand All @@ -174,7 +177,7 @@ WeatherProvider.register("openweathermap", {
let snow = 0;
// variable for date
let date = "";
let weather = new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh);
let weather = new WeatherObject();

for (const forecast of forecasts) {
if (date !== moment.unix(forecast.dt).format("YYYY-MM-DD")) {
Expand All @@ -187,7 +190,7 @@ WeatherProvider.register("openweathermap", {
// push weather information to days array
days.push(weather);
// create new weather-object
weather = new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh);
weather = new WeatherObject();

minTemp = [];
maxTemp = [];
Expand Down Expand Up @@ -250,7 +253,7 @@ WeatherProvider.register("openweathermap", {
const days = [];

for (const forecast of forecasts) {
const weather = new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh);
const weather = new WeatherObject();

weather.date = moment.unix(forecast.dt);
weather.minTemperature = forecast.temp.min;
Expand Down Expand Up @@ -296,7 +299,7 @@ WeatherProvider.register("openweathermap", {
let precip = false;

// get current weather, if requested
const current = new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh);
const current = new WeatherObject();
if (data.hasOwnProperty("current")) {
current.date = moment.unix(data.current.dt).utcOffset(data.timezone_offset / 60);
current.windSpeed = data.current.wind_speed;
Expand Down Expand Up @@ -328,7 +331,7 @@ WeatherProvider.register("openweathermap", {
current.feelsLikeTemp = data.current.feels_like;
}

let weather = new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh);
let weather = new WeatherObject();

// get hourly weather, if requested
const hours = [];
Expand Down Expand Up @@ -363,7 +366,7 @@ WeatherProvider.register("openweathermap", {
}

hours.push(weather);
weather = new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh);
weather = new WeatherObject();
}
}

Expand Down Expand Up @@ -402,7 +405,7 @@ WeatherProvider.register("openweathermap", {
}

days.push(weather);
weather = new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh);
weather = new WeatherObject();
}
}

Expand Down Expand Up @@ -471,7 +474,7 @@ WeatherProvider.register("openweathermap", {
return;
}

params += "&units=" + this.config.units;
params += "&units=metric"; // WeatherProviders should use metric internally and use the units only for when displaying data
params += "&lang=" + this.config.lang;
params += "&APPID=" + this.config.apiKey;

Expand Down

0 comments on commit 2d3940a

Please sign in to comment.