search

MoonModules / WLED

Control WS2812B and many more types of digital RGB LEDs with an ESP8266 or ESP32 over WiFi! MoonModules adds features on top of upstream.
https://mm.kno.wled.ge
GNU General Public License v3.0
187 stars 54 forks source link

The Weather usermod does not display correct temperatures plus documentation is needed #112

Open pedrohbraga opened 5 months ago

pedrohbraga commented 5 months ago

What happened?

The Weather usermod is presenting a few important bugs, which I describe below:"

  1. Current temperatures are inaccurate, off by several degrees. For instance, a call for the temperature for Montréal, Canada (e.g., using https://api.openweathermap.org/data/2.5/weather?lat=45.54&lon=-73.63&units=standard&appid={API_VALUE}), is presenting temperatures of -2.5 C, while it should be -7 C. The temperature in Fahrenheit is also incorrect.
  2. Displaying the temperature in Kelvin has issues with the overlay and lights all LEDs in the matrix;
  3. The predicted temperature chart does not work for Celsius.

To Reproduce Bug

One can set the locations for lat=45.54 and lon=-73.63, activate the Weather usermod, cycle through the unit settings, and compare with what would be expected within openweathermap's website.

Expected Behavior

I would expect the temperatures to be displayed correctly.

Install Method

From https://wled-install.github.io/

What version/release of MM WLED?

The latest MoonModule XL release

Which microcontroller/board are you seeing the problem on?

ESP32

Relevant log/trace output

No response

Anything else?

A few recommendations:

I suspect the issue is within the definition and conversion of Kelvin to Celsius and Fahrenheit, if there are any. One option is to incorporate the formulae for Kelvin to Fahrenheit (F = 1.8*(K-273) + 32), and from Kelvin to Celsius (C = K - 273.15) directly to the code (Kelvin is the standard output from the API).

Alternatively, one could define the units parameter within the API call, using metric or imperial, as in https://api.openweathermap.org/data/2.5/weather?lat=45.54&lon=-73.63&units=metric&appid=INCLUDE_YOUR_API_KEY, and pull the variable directly from there. Perhaps this could be the simplest option.

It is unclear what the bars in the chart describing future weather mean. Knowing the unit and value of each square, and the length of the x-axis is fundamental for this chart.

In addition to these changes, the documentation would greatly benefit from more writing and a more extensive definition of parameters. Without this, it is difficult for someone else to contribute to the project.

Code of Conduct

lost-hope commented 4 months ago

I had a look at your Issue report. the current usermod alreasy uses the units statement in the api call as you recommended (but also did that from the first release if i see that correctly) And i cant replicate your issue as of right now.