Use metric units internally in all weatherproviders #2849

rejas · 2022-05-10T20:35:52Z

So finally I think this refactorin is ready to be reviewed :-)

DONE:

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

Checked providers:

TODO in different tickets:

check weatherproviders for usage of weatherEndpoint (as seen in weathergov Weather Provider Doc Update MagicMirror-Documentation#131) -> see weathergov Documentation Update #2926
cleanup precipations -> Cleanup precipation usage in weather module #2953

codecov-commenter · 2022-05-10T20:38:44Z

Codecov Report

Merging #2849 (6cd8557) into develop (7bd9443) will decrease coverage by 0.07%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #2849      +/-   ##
===========================================
- Coverage    65.28%   65.21%   -0.08%     
===========================================
  Files           14       14              
  Lines          726      733       +7     
===========================================
+ Hits           474      478       +4     
- Misses         252      255       +3

Impacted Files	Coverage Δ
js/app.js	`69.53% <0.00%> (-0.72%)`	⬇️
js/server.js	`67.56% <0.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

MichMich · 2022-05-18T07:16:40Z

I love this PR. This is so much cleaner. Thanks! Let me know when it's ready to be merged.

rejas · 2022-06-20T08:34:18Z

I think this will not be ready for the next release, mainly due to not being sure if there arent any bugs in it. Would be nice to have people test this PR with their repsective providers...
Of course more tests would also be nice, which I might be ablke to do but probably not before August....

MichMich · 2022-06-20T10:35:17Z

No worries. It can easily wait till next release. 👍🏻

buxxi · 2022-07-12T19:33:25Z

Verified that the SMHI provider works as expected using the branch from this pull request. Great job.

berlincount · 2022-08-06T18:32:21Z

This is awesome, and truly so much cleaner! 👍

Might I suggest to change the useBeaufort etc to windUnitsalong the lines of tempUnits?

As a pilot, I'd love support for knots there ...

P.S: found this PR while researching the integration of UV Index, but I'll hold off on that until this is merged. Great work!

rejas · 2022-08-17T16:03:51Z

Since darksky is bought by apple and will stop in 2023 they dont allow new signups for their webapi anymore. Can anyone of the original developers check out the code (and maybe check out #2899 too if that is a proble there too?)

Also, probably have to deprecate the provider sooner or later :-) Not sure if the Apple WeatherKit is something to follow up since there isnt a real free plan as far as I can see (https://developer.apple.com/weatherkit/get-started/)

So, regarding checking darksky out, I look at @nhubbard @fewieden and @buxxi since they might have api access still :-)

buxxi · 2022-08-28T13:16:04Z

I have never tried the darksky provider, so I can't help with that.

rejas · 2022-08-29T16:23:06Z

so I hopefully fixed all providers with respect to temperature and wind.

only with weatherflow I had to blindly "fix" it since I dont have a station for that api. maybe the original author @10bias can test this?

next I will take a look at the other open points. still, any testing is welcome :-)

rejas · 2022-08-29T16:25:01Z

Might I suggest to change the useBeaufort etc to windUnitsalong the lines of tempUnits?

I am not an expert in wind stuff, so how would the windunits look like at the end?

berlincount · 2022-09-04T21:25:35Z

Might I suggest to change the useBeaufort etc to windUnitsalong the lines of tempUnits?

I am not an expert in wind stuff, so how would the windunits look like at the end?

if you've got m/s, it's comparatively simple.

https://www.convert-measurement-units.com/convert+Meters+per+second+to+Beaufort.php

is a great reference :) I guess we'd love km/h, Beaufort, and knots.

(if I get to it, I can write that code, but not today)

rejas · 2022-09-16T21:53:23Z

Might I suggest to change the useBeaufort etc to windUnitsalong the lines of tempUnits?

I am not an expert in wind stuff, so how would the windunits look like at the end?

if you've got m/s, it's comparatively simple.

https://www.convert-measurement-units.com/convert+Meters+per+second+to+Beaufort.php

is a great reference :) I guess we'd love km/h, Beaufort, and knots.

(if I get to it, I can write that code, but not today)

So I worked a little tonight at the windunits on my branch:

useKmh and useBeafort is deprecated, internally the config windUnits is used instead
windUnits allows "metric" (m/s), "imperial" (mph), "kmh", "beaufort" for now, plus also "knots"

Slowly getting to the finish line with this PR ;-)

also convert wind to m/s remove dead code

fix some typos too

also convert km/h to m/s for wind value

especially move sunset/sunrise to e2e tests

buxxi

Looking good!

rejas · 2022-10-24T16:22:00Z

Not sure if the other two will review here, so feel free to merge @khassel :-)

@angeldeejay

## [2.22.0] - 2023-01-01 Thanks to: @angeldeejay, @buxxi, @dariom, @dWoolridge, @KristjanESPERANTO, @MagMar94, @naveensrinivasan, @retroflex, @SkySails and @tom. Special thanks to @khassel, @rejas and @sdetweil for taking over most (if not all) of the work on this release as project collaborators. This version would not be there without their effort. Thank you! ### Added - Added test for remoteFile option in compliments module - Added hourlyWeather functionality to Weather.gov weather provider - Removed weatherEndpoint definition from weathergov.js (not used) - Added css class names "today" and "tomorrow" for default calendar - Added Collaboration.md - Added new github action for dependency review (#2862) - Added a WeatherProvider for Open-Meteo - Added Yr as a weather provider - Added config options "ignoreXOriginHeader" and "ignoreContentSecurityPolicy" ### Removed - Removed usage of internal fetch function of node until it is more stable ### Updated - Cleaned up test directory (#2937) and jest config (#2959) - Wait for all modules to start before declaring the system ready (#2487) - Updated e2e tests (moved `done()` in helper functions) and use es6 syntax in all tests - Updated da translation - Rework weather module - Make sure smhi provider api only gets a maximum of 6 digits coordinates (#2955) - Use fetch instead of XMLHttpRequest in weatherprovider (#2935) - Reworked how weatherproviders handle units (#2849) - Use unix() method for parsing times, fix suntimes on the way (#2950) - Refactor conversion functions into utils class (#2958) - The `cors`-method in `server.js` now supports sending and recieving HTTP headers - Replace `…` by `…` - Cleanup compliments module - Updated dependencies including electron to v22 (#2903) ### Fixed - Correctly show apparent temperature in SMHI weather provider - Ensure updatenotification module isn't shown when local is _ahead_ of remote - Handle node_helper errors during startup (#2944) - Possibility to change FontAwesome class in calendar, so icons like `fab fa-facebook-square` works. - Fix cors problems with newsfeed articles (as far as possible), allow disabling cors per feed with option `useCorsProxy: false` (#2840) - Tests not waiting for the application to start and stop before starting the next test - Fix electron tests failing sometimes in github workflow - Fixed gap in clock module when displayed on the left side with displayType=digital - Fixed playwright issue by upgrading to v1.29.1 (#2969) Signed-off-by: naveen <[email protected]> Co-authored-by: Karsten Hassel <[email protected]> Co-authored-by: Malte Hallström <[email protected]> Co-authored-by: Veeck <[email protected]> Co-authored-by: veeck <[email protected]> Co-authored-by: dWoolridge <[email protected]> Co-authored-by: Johan <[email protected]> Co-authored-by: Dario Mratovich <[email protected]> Co-authored-by: Dario Mratovich <[email protected]> Co-authored-by: Magnus <[email protected]> Co-authored-by: Naveen <[email protected]> Co-authored-by: buxxi <[email protected]> Co-authored-by: Thomas Hirschberger <[email protected]> Co-authored-by: Kristjan ESPERANTO <[email protected]> Co-authored-by: Andrés Vanegas Jiménez <[email protected]>

rejas force-pushed the issue_2812 branch from 6d90d11 to d46e0aa Compare June 20, 2022 08:25

rejas force-pushed the issue_2812 branch from a9687da to 2f8dcce Compare July 3, 2022 13:15

rejas force-pushed the issue_2812 branch from 2f8dcce to 797e45d Compare August 6, 2022 14:05

rejas force-pushed the issue_2812 branch from 797e45d to aded595 Compare August 16, 2022 20:01

rejas force-pushed the issue_2812 branch from a23a474 to dc23635 Compare August 19, 2022 11:26

rejas force-pushed the issue_2812 branch from d5a7149 to c4bc3a6 Compare August 30, 2022 08:42

rejas force-pushed the issue_2812 branch from 5c09bdd to 765b164 Compare September 13, 2022 19:47

rejas force-pushed the issue_2812 branch 2 times, most recently from db97ef8 to a554dcb Compare September 21, 2022 04:44

rejas force-pushed the issue_2812 branch 9 times, most recently from 68f0e02 to 6de8769 Compare October 4, 2022 13:36

veeck added 8 commits October 19, 2022 09:21

Use metric units when calling weathergov api

220471b

also convert wind to m/s remove dead code

Fix wind speed display

2bc7109

Convert wind kmh from envcanada to m/s

89ab15a

fix some typos too

Cleanup weatherflow call, use metric always

9116799

also convert km/h to m/s for wind value

Deprecate useKmh and useBeafort, use windUnits instead

02ff38b

Add knots to possible windUnits

e68a9ab

Remove last usage of units from weatherobject, simplify constructor

fc16b3e

Update CHANGELOG

a9fecbe

rejas force-pushed the issue_2812 branch from 9281fc7 to 0ae9bdb Compare October 19, 2022 07:35

Update and fix weather tests

cf8eb16

especially move sunset/sunrise to e2e tests

rejas force-pushed the issue_2812 branch from 0ae9bdb to cf8eb16 Compare October 19, 2022 09:33

Cleanup logging

d51f155

rejas marked this pull request as ready for review October 19, 2022 10:16

rejas requested review from khassel, MichMich and sdetweil October 19, 2022 10:16

Update jsdoc

4094ebb

rejas force-pushed the issue_2812 branch from a0488b8 to 4094ebb Compare October 19, 2022 14:59

khassel approved these changes Oct 19, 2022

View reviewed changes

rejas mentioned this pull request Oct 19, 2022

Add error handling to node_helper startup sequence #2945

Merged

buxxi approved these changes Oct 19, 2022

View reviewed changes

rejas force-pushed the issue_2812 branch from b065241 to f6c1513 Compare October 20, 2022 09:17

Final cleanups

6cd8557

rejas force-pushed the issue_2812 branch from f6c1513 to 6cd8557 Compare October 20, 2022 09:35

khassel merged commit 2d3940a into MagicMirrorOrg:develop Oct 24, 2022

rejas deleted the issue_2812 branch October 24, 2022 18:13

rejas mentioned this pull request Nov 26, 2022

Update WeatherProvider documentation MagicMirrorOrg/MagicMirror-Documentation#138

Merged

MichMich mentioned this pull request Jan 1, 2023

Release 2.22.0 #2983

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use metric units internally in all weatherproviders #2849

Use metric units internally in all weatherproviders #2849

rejas commented May 10, 2022 •

edited

Loading

codecov-commenter commented May 10, 2022 •

edited

Loading

MichMich commented May 18, 2022

rejas commented Jun 20, 2022

MichMich commented Jun 20, 2022

buxxi commented Jul 12, 2022

berlincount commented Aug 6, 2022

rejas commented Aug 17, 2022

buxxi commented Aug 28, 2022

rejas commented Aug 29, 2022

rejas commented Aug 29, 2022

berlincount commented Sep 4, 2022 •

edited

Loading

rejas commented Sep 16, 2022

buxxi left a comment

rejas commented Oct 24, 2022

Use metric units internally in all weatherproviders #2849

Use metric units internally in all weatherproviders #2849

Conversation

rejas commented May 10, 2022 • edited Loading

codecov-commenter commented May 10, 2022 • edited Loading

Codecov Report

MichMich commented May 18, 2022

rejas commented Jun 20, 2022

MichMich commented Jun 20, 2022

buxxi commented Jul 12, 2022

berlincount commented Aug 6, 2022

rejas commented Aug 17, 2022

buxxi commented Aug 28, 2022

rejas commented Aug 29, 2022

rejas commented Aug 29, 2022

berlincount commented Sep 4, 2022 • edited Loading

rejas commented Sep 16, 2022

buxxi left a comment

Choose a reason for hiding this comment

rejas commented Oct 24, 2022

rejas commented May 10, 2022 •

edited

Loading

codecov-commenter commented May 10, 2022 •

edited

Loading

berlincount commented Sep 4, 2022 •

edited

Loading