search

aaronbieber / sunshine.el

An Emacs package for displaying the forecast from OpenWeatherMap.
87 stars 15 forks source link

Changes to fit the new API. Before it was every 6h. New daily API giv… #16

Closed azynheira closed 4 years ago

azynheira commented 4 years ago

The old API called now gives weather every 6h. A 'daily' API needs to be used. But the JSON parsing had to be tweaked. Same results as before once this patch is applied.

aaronbieber commented 4 years ago

I'm trusting you that this works, since I haven't used this package in years and I'm too lazy to test the branch. Thanks for contributing!

rgd commented 4 years ago

Backtrace of this (wrong-type-argument listp t) error shows at lowest level:

url-http-generic-filter(#<process api.openweathermap.org> "HTTP/1.1 400 Bad Request\15\nServer: openresty\15\nDate: Mon, 09 Sep 2019 21:00:27 GMT\15\nContent-Type: text/html\15\nContent-Length: 166\15\nConnection: close\15\nVia: HTTP/1.1 proxy10401\15\n\15\n<html>\15\n<head><title>400 Bad Request</title></head>\15\n<body bgcolor=\"white\">\15\n<center><h1>400 Bad Request</h1></center>\15\n<hr><center>nginx</center>\15\n</body>\15\n</html>\15\n")

So if it's not able to get good data, it's no wonder later on things act weird.

rgd commented 4 years ago

Time to test network connectivity (though it can update packages just fine...)

azynheira commented 4 years ago

Hi, I will try to add more error trapping to the data retrieving. Lets make it better :-)

On Mon, 9 Sep 2019 at 22:07, rgd notifications@github.com wrote:

Time to test network connectivity (though it can update packages just fine...)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/aaronbieber/sunshine.el/pull/16?email_source=notifications&email_token=AACUVHCN2URNUVXOKBYCTADQI23KVA5CNFSM4IUANA42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6JBMDQ#issuecomment-529667598, or mute the thread https://github.com/notifications/unsubscribe-auth/AACUVHBWUPIT6UWAYV5D3ZDQI23KVANCNFSM4IUANA4Q .

rgd commented 4 years ago

Hi, I hope so. :)

I tried again at home, no corporate proxy. Still getting wrong-type-argument when trying sunshine-forecast.

(sunshine-make-url) gives: " http://api.openweathermap.org/data/2.5/forecast/daily?q=Akron,US&APPID=1a914105a4f4529151e42b8e8355fb96&mode=json&units=Imperial&cnt=5 "

But in either Chrome or using cURL on the command line, it returns:

{"cod":401, "message": "Invalid API key. Please see http://openweathermap.org/faq#error401 for more info."}

If I remove "/daily", then it returns:

`c:>curl "http://api.openweathermap.org/data/2.5/forecast?q=Akron,US&APPID=1a914105a4f4529151e42b8e8355fb96&mode=json&units=Imperial&cnt=5"

{"cod":"200","message":0.0115,"cnt":5,"list":[{"dt":1568084400,"main":{"temp":63.63,"temp_min":59.63,"temp_max":63.63,"pressure":1023.1,"sea_level":1023.1,"grnd_level":985.41,"humidity":86,"temp_kf":2.22},"weather":[{"id":802,"main":"Clouds","description":"scattered clouds","icon":"03n"}],"clouds":{"all":50},"wind":{"speed":5.64,"deg":94.722},"sys":{"pod":"n"},"dt_txt":"2019-09-10 03:00:00"},{"dt":1568095200,"main":{"temp":61.11,"temp_min":58.1,"temp_max":61.11,"pressure":1023.5,"sea_level":1023.5,"grnd_level":985.74,"humidity":82,"temp_kf":1.67},"weather":[{"id":802,"main":"Clouds","description":"scattered clouds","icon":"03n"}],"clouds":{"all":43},"wind":{"speed":6.29,"deg":127.898},"sys":{"pod":"n"},"dt_txt":"2019-09-10 06:00:00"},{"dt":1568106000,"main":{"temp":59.94,"temp_min":57.93,"temp_max":59.94,"pressure":1023.14,"sea_level":1023.14,"grnd_level":985.45,"humidity":89,"temp_kf":1.11},"weather":[{"id":803,"main":"Clouds","description":"broken clouds","icon":"04n"}],"clouds":{"all":74},"wind":{"speed":7.36,"deg":155.211},"sys":{"pod":"n"},"dt_txt":"2019-09-10 09:00:00"},{"dt":1568116800,"main":{"temp":61.23,"temp_min":60.22,"temp_max":61.23,"pressure":1023.71,"sea_level":1023.71,"grnd_level":985.93,"humidity":90,"temp_kf":0.56},"weather":[{"id":803,"main":"Clouds","description":"broken clouds","icon":"04d"}],"clouds":{"all":72},"wind":{"speed":8.1,"deg":160.262},"sys":{"pod":"d"},"dt_txt":"2019-09-10 12:00:00"},{"dt":1568127600,"main":{"temp":71.04,"temp_min":71.04,"temp_max":71.04,"pressure":1023.1,"sea_level":1023.1,"grnd_level":985.55,"humidity":80,"temp_kf":0},"weather":[{"id":804,"main":"Clouds","description":"overcast clouds","icon":"04d"}],"clouds":{"all":100},"wind":{"speed":9.6,"deg":173.528},"sys":{"pod":"d"},"dt_txt":"2019-09-10 15:00:00"}],"city":{"id":5145476,"name":"Akron","coord":{"lat":41.0831,"lon":-81.5185},"country":"US","population":199110,"timezone":-14400,"sunrise":1568026831,"sunset":1568072808}}`

I notice that it's giving "temp_min" and "temp_max" and the latest change in sunshine-build-forecast changes references to temp_min to min, etc. on lines 277/278. It seems like I'm not getting the same results from the server that you are. The code is now expecting min and max but I'm getting temp_min and temp_max. Very odd. I'd suspect a browser cache but cURL returns it too. I suppose that's because I'm not sending "/daily", it's using the 'old' format. OK. So why does the "/daily" return Invalid API key? Thought I might need a new API key so tried that and got the same result. Does my account need to be upgraded or a paid account or something? Beats me.

If you add some diagnostics, I'll be glad to try them out.

Thanks,

Rob

On Mon, Sep 9, 2019 at 5:41 PM Pedro Gomes notifications@github.com wrote:

Hi, I will try to add more error trapping to the data retrieving. Lets make it better :-)

On Mon, 9 Sep 2019 at 22:07, rgd notifications@github.com wrote:

Time to test network connectivity (though it can update packages just fine...)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < https://github.com/aaronbieber/sunshine.el/pull/16?email_source=notifications&email_token=AACUVHCN2URNUVXOKBYCTADQI23KVA5CNFSM4IUANA42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6JBMDQ#issuecomment-529667598 , or mute the thread < https://github.com/notifications/unsubscribe-auth/AACUVHBWUPIT6UWAYV5D3ZDQI23KVANCNFSM4IUANA4Q

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/aaronbieber/sunshine.el/pull/16?email_source=notifications&email_token=AABTODEGSUBHHWZPGXAZAXTQI27IRA5CNFSM4IUANA42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6JEA7Y#issuecomment-529678463, or mute the thread https://github.com/notifications/unsubscribe-auth/AABTODEQGW6UYONE4S3XRMTQI27IRANCNFSM4IUANA4Q .

aaronbieber commented 4 years ago

Rob, note that you included your API key in that reply.

I was able to confirm the same result on the /daily endpoint using your API key, but when I use mine, it returns the result expected by the code ("min" and "max" are the new temperature key names).

To be sure there is zero ambiguity, here is where my key came from:

openeathermap-api-keys

I think we can say with some certainty that this is an issue with OpenWeather API versioning. We should make the code more robust to the possibility that someone has this same issue, but I'd like for the error to provide steps to resolve, so we have to figure out how to resolve it first.

azynheira commented 4 years ago

Hi, Thanks for the warning. I will generate a new one. Here's mine!

Pedro

P.S. - Regarding, ("min" and "max" are the new temperature key names). that correct! That's why I tweaked the elisp code that extracts the s-expressions from the response.

On Tue, 10 Sep 2019 at 10:35, Aaron Bieber notifications@github.com wrote:

Rob, note that you included your API key in that reply.

I was able to confirm the same result on the /daily endpoint using your API key, but when I use mine, it returns the result expected by the code ("min" and "max" are the new temperature key names).

To be sure there is zero ambiguity, here is where my key came from:

[image: openeathermap-api-keys] https://user-images.githubusercontent.com/157664/64602338-9f1ef880-d38c-11e9-8855-c08aea799d6d.png

I think we can say with some certainty that this is an issue with OpenWeather API versioning. We should make the code more robust to the possibility that someone has this same issue, but I'd like for the error to provide steps to resolve, so we have to figure out how to resolve it first.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/aaronbieber/sunshine.el/pull/16?email_source=notifications&email_token=AACUVHB6A45V3YQN5HLW5SDQI5S7DA5CNFSM4IUANA42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6KPLRI#issuecomment-529855941, or mute the thread https://github.com/notifications/unsubscribe-auth/AACUVHAIIHEUNFT5KF66UATQI5S7DANCNFSM4IUANA4Q .