Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Cleanup precipation usage in weather module #2953

Closed
rejas opened this issue Oct 19, 2022 · 0 comments
Closed

Cleanup precipation usage in weather module #2953

rejas opened this issue Oct 19, 2022 · 0 comments
Labels
ready (coming with next release)
Milestone
2.23

Comments

@rejas
Copy link
Collaborator

rejas commented Oct 19, 2022

Preipation seems to be used in different ways in the weatherproviders:

some are using it as amount of precipation, some are using it as a percentage (risk of rain)

Whoever takes this ticket should look if the usage and display of these values can be optimized:

  • split up in two config options?
  • update diplsay layout (german word for riskofrain is waaaaaaaays to long, making me think nobody really uses this feature)
  • whatever they else find
@rejas rejas mentioned this issue Oct 19, 2022
Merged
14 tasks
khassel pushed a commit that referenced this issue Oct 24, 2022
@rejas
Use metric units internally in all weatherproviders (#2849)
2d3940a 
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>
@MagMar94 MagMar94 mentioned this issue Jan 22, 2023
Merged
rejas added a commit that referenced this issue Feb 4, 2023
@MagMar94 @rejas
Tidy up precipitation (#3023)
bf279d9 
Fixes #2953

This is an attempt to fix the issue with precipitation amount and
percentage mixup. I have created a separate
`precipitationPercentage`-variable where the probability of rain can be
stored.

The config options now has the old `showPrecipitationAmount` in addition
to a new setting: `showPrecipitationProbability` (shows the likelihood
of rain).

<details>
  <summary>Examples</summary>
  
  ### Yr

I tested the Yr weather provider for a Norwegian city Bergen that has a
lot of rain. I have removed properties that are irrelevant for this demo
from the config-samples below.

Config:
  ```js
{
	module: "weather",
	config: {
			weatherProvider: "yr",
			type: "current",
			showPrecipitationAmount: true,
			showPrecipitationProbability: true
	}
},
{
	module: "weather",
	config: {
			weatherProvider: "yr",
			type: "hourly",
			showPrecipitationAmount: true,
			showPrecipitationProbability: true
	}
},
{
	module: "weather",
	config: {
			weatherProvider: "yr",
			type: "daily",
			showPrecipitationAmount: true,
			showPrecipitationProbability: true
	}
}
  ```

Result:<br/>
<img width="444" alt="screenshot"
src="https://user-images.githubusercontent.com/34011212/216775423-4e37345c-f915-47e5-8551-7c544ebd24b1.png">

</details>

---------

Co-authored-by: Magnus Marthinsen <magmar@online.no>
Co-authored-by: Veeck <github@veeck.de>
@rejas rejas added this to the 2.23 milestone Feb 18, 2023
@khassel khassel closed this as completed Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ready (coming with next release)
Projects
None yet
Milestone
2.23
Development

No branches or pull requests
2 participants
@rejas @khassel