Skip to content
This repository was archived by the owner on Aug 11, 2018. It is now read-only.

Reflect changes to the Yahoo API format #143

Closed
wants to merge 3 commits into from

Conversation

ccprog
Copy link

@ccprog ccprog commented Apr 13, 2016

As described in #127, there seem to be changes to the weather API. These are reflected here, and are working for me as of 13-4-2016.

In addition, to enable users to find current WOEIDs, the "Get WOEID" button now points to http://woeid.rosselliot.co.nz/

@AndoOfTheWoods
Copy link

AndoOfTheWoods commented Apr 15, 2016

Works nicely. Thanks!

EDIT: There is one quirk, the windchill value for celsius displays the fahrenheit value.
E.g. Temp of 14_C will have 59_C wind chill

@ccprog
Copy link
Author

ccprog commented Apr 15, 2016

There are more unit inconsistencies. The data delivered by Yahoo for Berlin today:

"units": {
  "distance": "km", "pressure": "mb", "speed": "km/h", "temperature": "C"
},
"wind": {
  "chill": "46", "direction": "185", "speed": "28.97"
},
"atmosphere": {
  "humidity":"88", "pressure":"33830.04", "rising":"0", "visibility":"19.79"
}
  • API call http://query.yahooapis.com/v1/public/yql?format=json&q=select * from weather.forecast where woeid=638242 and u="f"
"units": {
  "distance": "mi", "pressure": "in", "speed": "mph", "temperature": "F"
},
"wind": {
  "chill":"46", "direction": "185", "speed": "18"
},
"atmosphere": {
  "humidity": "88", "pressure":  "999.0", "rising":  "0", "visibility":"12.3"
}
  1. Chill data are obviously in F, unconverted for both. This might be considered consistent with the documentation https://developer.yahoo.com/weather/documentation.html, which says "in degrees (integer)", but without stating the unit
  2. The pressure conversion factor is the right one between inHg and mbar, but the values make no sense - it seems the mbar value is read as if it was inHg.
  3. Same for visibility: the km value is read as if it was mi
  4. Same for wind speed: the km/h value is read as if it was mph

This is a mess.

I think I'll adapt the chill conversion, but leave the other ones alone for now.

@deekej
Copy link

deekej commented Apr 20, 2016

Unfortunately, not working for me... [Cinnamon 2.8.7 @ Fedora 23]
The applet will crash everytime I try to add/load it. Restarting Cinnamon does not help.

@ccprog
Copy link
Author

ccprog commented Apr 20, 2016

Can you take a look at the Looking Glass log? (Super+L, Tab "Log" or in raw form at ~/.cinnamon/glass.log)

@deekej
Copy link

deekej commented Apr 22, 2016

Hmm, I don't know why it was not working before, but it is working for me now. Sorry for the confusion.

But I can confirm that the pressure value in mbar is out of range. Currently it is 1015 mbar, according to my phone, but the applet shows 33220.49 mbar.

Temperature, wind, humidity and wind chill seems to be OK.

@marceloavan
Copy link

I had the same problem and after update with these changes, everything worked like a charm.

@jsa-aerial
Copy link

Where can you get these changes? The repo at the head of these comments shows that its most recent change is 28 days ago and that was just a version bump. I have version 1.12.4, which is what that repo is. I think the change described here may fix this:

TypeError: json.get_object_member(...).get_object_member(...).get_object_member(...) is null

But I can't find where it is!

@jsa-aerial
Copy link

Found it - was looking at the wrong repo (needed ccprog's fork) and branch...

Good news - it works!

@mattst
Copy link

mattst commented Apr 24, 2016

Works perfectly. Thanks.

@corbin-auriti
Copy link

Thanks for the fix, really appreciated :)

@MitchellMcCluskey
Copy link

Thanks for fixing it

@xyzelt
Copy link

xyzelt commented May 6, 2016

At pressure they mess inHg with mbar.
To fix this i divided with WEATHER_CONV_MBAR_IN_INHG

switch (this._pressureUnit) {
case WeatherPressureUnits.MBAR:
if (this._temperatureUnit == WeatherUnits.FAHRENHEIT) {
pressure = pressure * WEATHER_CONV_MBAR_IN_INHG
}
// Otherwise no conversion needed - already in correct units
pressure = pressure / WEATHER_CONV_MBAR_IN_INHG
pressure = parseFloat(pressure).toFixed(2)
break

and so on for the rest

@mockturtl
Copy link
Owner

Chill data are obviously in F, unconverted for both.

Good catch.

The pressure conversion factor is the right one between inHg and mbar, but the values make no sense - it seems the mbar value is read as if it was inHg.
Same for visibility: the km value is read as if it was mi
Same for wind speed: the km/h value is read as if it was mph

Same here.

@mockturtl
Copy link
Owner

Merged. I'd like to figure out the unit inconsistencies before cutting a release. Please feel free to continue discussion here.

@mockturtl mockturtl closed this May 12, 2016
@ikarus13
Copy link

Don't work for me at all.
My system: Linux Mint 17.3, 64 bit
What i had done:
remove ~/.local/share/cinnamon/applets/weather@mockturtl
git clone https://github.com/ccprog/cinnamon-weather.git "weather@mockturtl"

The error in .cinnamon/glass.log:
error t=2016-05-12T09:06:16.534Z weather@mockturtl#: TypeError: json.get_object_member(...).get_object_member(...).get_object_member(...) is null

I tried the old and new weather codes GMXX0088 and 676864 for my city Münster, Germany
Some more hints?

Thanks,
ikarus

@ccprog
Copy link
Author

ccprog commented May 12, 2016

@ikarus13, If you are using my git ccprog/cinnamon-weather, you need to change the branch:
git checkout ccprog-yahoo-new-format

In mockturtl/cinnamon-weather, the changes are now part of the master branch

@mockturtl, the question is whether you would patch errors that are obviously in the realm of Yahoo. These errors are known (here and here) and you would have to track their correction and eventually remove the patches again.

I don't see a way to reliably distinguish correct and garbled data from the API response.

@gilbertohasnofb
Copy link

gilbertohasnofb commented May 14, 2016

Now that this has been merged, is there any plans of a new release of this applet in the near future? Because in the meantime, the applet is broken to everyone (not using git), so I believe a new release should be a high priority.

@mockturtl
Copy link
Owner

Now that this has been merged, is there any plans of a new release of this applet in the near future?

Please don't make me continue to moderate your useless questions.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet