Use metric units internally in all weatherproviders
This is just a draft to get feedback on my refactoring related to #2812
- 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
- Use moment.unix for creating dates based on unix seconds in openweathermap provider
Precipation seems also to be a candidate for cleanup but I didnt look deeply into the code.
So, any comments and suggestions would be greatly welcome now before I go any further.
Todos:
- [x] look how beaufort calculation uses metrics
- [ ] cleanup precipations:
- [ ] split between precipation and chaneofprecipation
- [ ] cleanup unit usage
- [ ] convert more places from
moment(timestamp, "X")tomoment.unix(timestamp) - [ ] 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
Codecov Report
Merging #2849 (6cd8557) into develop (7bd9443) will decrease coverage by
0.07%. The diff coverage isn/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%) |
:arrow_down: |
| 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.
I love this PR. This is so much cleaner. Thanks! Let me know when it's ready to be merged.
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....
No worries. It can easily wait till next release. 👍🏻
Verified that the SMHI provider works as expected using the branch from this pull request. Great job.
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!
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 https://github.com/MichMich/MagicMirror/issues/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 :-)
I have never tried the darksky provider, so I can't help with that.
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 :-)
Might I suggest to change the
useBeaufortetc towindUnitsalong the lines oftempUnits?
I am not an expert in wind stuff, so how would the windunits look like at the end?
Might I suggest to change the
useBeaufortetc towindUnitsalong the lines oftempUnits?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)
Might I suggest to change the
useBeaufortetc towindUnitsalong the lines oftempUnits?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 ;-)
Not sure if the other two will review here, so feel free to merge @khassel :-)