aaronbieber / sunshine.el

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

sunshine-forecast prompts for username and password #12

Open diamond-lizard opened 6 years ago

diamond-lizard commented 6 years ago

When I run sunshine-forcast, I get prompted for:

Username [for http://api.openweathermap.org/data/2.5/forecast/daily?q=Seattle,%20WA&APPID=deadbeefdeadbeefdeadbeefdeadbeef&mode=json&units=imperial&cnt=5]:

and then a password. If I leave those blank, I get prompted for them again.

Shouldn't there be no need for entering a username and password since an API key is used?

aaronbieber commented 6 years ago

That is true; it is possible that you haven't configured the key correctly. I am still able to get my forecast without entering credentials (and I set up my key a year ago, so nothing has changed on the OpenWeatherMap side).

On Sun, Sep 3, 2017 at 12:34 PM diamond-lizard notifications@github.com wrote:

When I run sunshine-forcast, I get prompted for:

Username [for http://api.openweathermap.org/data/2.5/forecast/daily?q=Seattle,%20WA&APPID=deadbeefdeadbeefdeadbeefdeadbeef&mode=json&units=imperial&cnt=5]:

and then a password. If I leave those blank, I get prompted for them again.

Shouldn't there be no need for entering a username and password since an API key is used?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/aaronbieber/sunshine.el/issues/12, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJn4FS7vRstJEGpAbAIFc6lzuICZcEBks5setUqgaJpZM4PLO4W .

aaronbieber commented 6 years ago

Ensure that your APPID is correctly set by pressing C-h v sunshine-appid RET and note the variable's value.

On Mon, Sep 4, 2017 at 6:36 AM Aaron Bieber aaron@aaronbieber.com wrote:

That is true; it is possible that you haven't configured the key correctly. I am still able to get my forecast without entering credentials (and I set up my key a year ago, so nothing has changed on the OpenWeatherMap side).

On Sun, Sep 3, 2017 at 12:34 PM diamond-lizard notifications@github.com wrote:

When I run sunshine-forcast, I get prompted for:

Username [for http://api.openweathermap.org/data/2.5/forecast/daily?q=Seattle,%20WA&APPID=deadbeefdeadbeefdeadbeefdeadbeef&mode=json&units=imperial&cnt=5]:

and then a password. If I leave those blank, I get prompted for them again.

Shouldn't there be no need for entering a username and password since an API key is used?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/aaronbieber/sunshine.el/issues/12, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJn4FS7vRstJEGpAbAIFc6lzuICZcEBks5setUqgaJpZM4PLO4W .

diamond-lizard commented 6 years ago

C-h v sunshine-appid returns:

sunshine-appid is a variable defined in `sunshine.el'.
Its value is "deadbeefdeadbeefdeadbeefdeadbeef"
Original value was ""

Documentation:
You can get an APPID by logging into your OpenWeather account.
You should get it by loging-in to your account and pasting the API key here.

You can customize this variable.

For more information check the manuals.

Where the actual value is what I got from the OpenWeather website after registering.

aaronbieber commented 6 years ago

I'm not sure what to tell you; Sunshine is requesting the URL from your original issue using url-retrieve; there is no magic to it. If you paste that URL into a browser, does it also prompt for authentication?

When was the API key generated? It takes some time for keys on free accounts to become active, though I would think it should be working within an hour. I'm just looking at what it says on their website, and I'm afraid I can't be much more help in debugging authentication issues on their side.

diamond-lizard commented 6 years ago

When I tried http://api.openweathermap.org/data/2.5/forecast/daily?q=Seattle,%20WA&APPID=deadbeefdeadbeefdeadbeefdeadbeef&mode=json&units=imperial&cnt=5 (but with my actual API key) from a web browser I got:

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

But the key is valid. It's the default key on https://home.openweathermap.org/api_keys for my account. I created my OpenWeatherMap account yesterday, and according to their docs "Activation of an API key for Free and Startup accounts takes 10 minutes," so it should have been active by now (a day later).

Then I went to their API usage documentation page, and saw that their example URL looked quite different: http://api.openweathermap.org/data/2.5/forecast?id=524901&APPID={APIKEY} Also, they said "Call API by city ID instead of city name, city coordinates or zip code. In this case you get precise respond exactly for your city. The cities' IDs can be found in the following file: Cities' IDs list."

So I got the Cities' IDs list (city.list.json.gz from the above link), found that Seattle's ID was 5809844, and tried the following URL (only with my real API key): http://api.openweathermap.org/data/2.5/forecast?id=5809844&APPID=deadbeefdeadbeefdeadbeefdeadbeef and it worked (ie. I got what looks like a big JSON formatted file that's full of entries like "weather":[{"id":800,"main":"Clear","description":"clear sky","icon":"01d"}] etc).

romario89 commented 6 years ago

It looks like sunshine.el doesn't work with the City ID, is that right?

aaronbieber commented 6 years ago

Sunshine uses the ?q={city_name},{country_code} format forecast URL, which requires a city name rather than city ID.

If you'd like to be able to use a city ID, I suppose we would need either a configurable Openweathermap API URL, or configuration variables for selecting the query type. I'm open to accepting pull requests for such features.

aaronbieber commented 6 years ago

Getting back to @diamond-lizard, I did a little more testing and I realized that the URL format does specify that "country code" needs to be included in the value of q. This URL worked for me:

http://api.openweathermap.org/data/2.5/forecast/daily?q=Seattle,%20WA,%20US&APPID=deadbeefdeadbeefdeadbeef&mode=json&units=imperial&cnt=5

The only thing I changed was to add ,%20US to the end so that the resulting decoded city name becomes Seattle, WA, US. I also tried USA, which worked as well.

Let me know if that works for you.

lpatrick commented 6 years ago

I'm running into the same issue. I was perusing the api docs -- is /daily still supported/required?

This works for me:

http://api.openweathermap.org/data/2.5/forecast?q=Chicago,US&APPID=xxxxxxxxxxxxxxxxxxxx

This does not:

http://api.openweathermap.org/data/2.5/forecast/daily?q=Chicago,US&APPID=xxxxxxxxxxxxxxxxxxxx

lpatrick commented 6 years ago

Ok -- did some hunting around and it looks like the second form is part of the 16 day forecast which was available for free until recently (even though it is trimmed down to 5 days in sunshine-make-url)

https://openweathermap.desk.com/customer/portal/questions/17136552-api-not-allowing-daily-forecast

@aaronbieber , any chance we can make this configurable?

MAliNaqvi commented 6 years ago

@lpatrick I tried to modify the code here to remove the /daily part, however emacs still complains about

sunshine-build-simple-forecast: Wrong type argument: numberp, nil
lpatrick commented 6 years ago

The 5 day (3 hourly) json message is likely much different. I took a quick look at the code and I think one would need to tweak sunshine-build-simple-forecast to match.

aaronbieber commented 6 years ago

Everything is still working for me, so I would want to figure out what the actual root problem is before making changes. As noted above, all I did on my end was add the country code to my request (e.g., Chicago, IL, US), but Chicago, US is also working for me (with the /daily part).

aaronbieber commented 6 years ago

When did you all create your accounts on OpenWeatherMap? I wonder if my account/APPID are "gradfathered" in to be able to use this particular request.

aaronbieber commented 6 years ago

I created a new account and I'm seeing what you guys are seeing, so I think that's the issue. I am going to see if I can pivot to the 5-day forecast URL format.

aaronbieber commented 6 years ago

OK, I've gotten the request and API key to work again by calling for the "5 day/3 hour" forecast (omitting /daily). Unfortunately, that endpoint returns the forecast every three hours and is not configurable, so rather than collecting each of the listed forecast data elements, I'll need to filter it down to one element per day to make a five-day forecast structure.

This is now more complicated than just changing the elements I'm looking for, so be patient (or feel free to post a PR!)

aaronbieber commented 6 years ago

The other issue with this forecast format is that the forecast actually varies throughout the day, so which element do I choose to accurately represent what it will be like on that day? Should I always try to find the "noon" value? That is likely to be the warmest.

For today's date, the API won't return values in the past, either, so I would have to take the first value for today and the noon value for other days. You can get a sense of how complex this is getting.

If anyone knows a different/better free weather API, I'd be happy to evaluate changing entirely, which is a lot more work on the plumbing of the package but may produce a better end result.

I should also note that I wrote this as a toy for learning elisp and async tasks and I actually don't use it, so I'm happy that folks are getting some value out of it, but there is a limit to how much time I want to spend on maintaining it.

aaronbieber commented 6 years ago

This APIXU weather forecast looks promising and also has PNG icons: https://www.apixu.com/api-explorer.aspx

I may consider pivoting to this, although I don't know how accurate their data is.

aaronbieber commented 6 years ago

I had a while today to poke at this so I've implemented APIXU for the most part and it works nicely. I have an issue with the forecast not starting with today's date and the forecast itself seems a little less accurate (predicts heavy snow today in Boston, which is not happening).

aaronbieber commented 6 years ago

OK this is all working now. I'd appreciate it if you could pull the convert-to-apixu branch and try it yourselves. https://github.com/aaronbieber/sunshine.el/tree/convert-to-apixu

MAliNaqvi commented 6 years ago

@aaronbieber verified that it works!

aaronbieber commented 6 years ago

I will likely just merge this because it seems that OpenWeatherMap will never support an easier-to-use response format on the free tier and on balance their documentation is crap. APIXU's isn't that much better but at least it's simple and free and works.

MAliNaqvi commented 6 years ago

@aaronbieber there seems to be a little bug. The section for displaying icons is showing the text "ng" beside icons:

https://www.dropbox.com/s/pdt05gqas8m3f6u/Screenshot%202018-03-26%2019.39.09.png?dl=0

Are you getting this as well?

aaronbieber commented 6 years ago

Thanks I'll have a look!

On Mon, Mar 26, 2018 at 7:39 PM M. Ali Naqvi notifications@github.com wrote:

@aaronbieber https://github.com/aaronbieber there seems to be a little bug. The section for displaying icons is showing the text "ng" beside icons:

https://www.dropbox.com/s/pdt05gqas8m3f6u/Screenshot%202018-03-26%2019.39.09.png?dl=0

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/aaronbieber/sunshine.el/issues/12#issuecomment-376346965, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJn4JtkQdTjSe_csjG6sSL4ilviKBlJks5tiXwlgaJpZM4PLO4W .

MAliNaqvi commented 6 years ago

@aaronbieber I am not sure how to reproduce this error. Today the text "ng" is not showing up.

In any case, this issue can be closed.

aaronbieber commented 6 years ago

I suspect the "ng" was from the end of the filename, "XXX.png"; it is possible that there are very long filenames that I haven't seen because I've only experienced the actual weather in the past week.

On Tue, Mar 27, 2018 at 6:47 PM M. Ali Naqvi notifications@github.com wrote:

@aaronbieber https://github.com/aaronbieber I am not sure how to reproduce this error. Today the text "ng" is not showing up.

In any case, this issue can be closed.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/aaronbieber/sunshine.el/issues/12#issuecomment-376700687, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJn4DytJlzBMZsLBocSJX_EsCqar3kEks5tisGVgaJpZM4PLO4W .

eklitzke commented 6 years ago

I just sent a pull request to include the country code in the default location, but now that I have had a chance to read this thread fully I realize the APIXU branch is probably the better approach. I was able to generate an APIXU key and test the convert-to-apixu branch, and can confirm that it works. I also prefer the new icons and formatting of the output in that branch. The convert-to-apixu branch also parses "San Francisco, CA" (without explicitly setting a US country code) to the city in California, which seems like better and more intuitive behavior to me.

One suggestion: the APIXU branch is going to break if people upgrade to a newer sunshine.el version without reading the release notes. If it's not too much work I would suggest changing the variable name, e.g. to sunshine-apixu-appid. Another possible way to do this (but with significantly more complexity) would be to add code to detect if the API key fails with APIXU, then test the same key against openweathermaps, and then prompt the user if it looks like they have a key configured for the wrong service.

My vote would be to either merge the convert-to-apixu branch nowish and then close my PR #13 without merging it, or to merge my PR if you don't plan to integrate the APIXU changes soonish.

eklitzke commented 6 years ago

One more note: I am seeing "g" appended to the new icons, rather than "ng" as noted in the previous comment. Screenshot here. This is with the latest commit in that branch, b9f292f28dd3f587647850980c027553d33f63b7

The icon names I am fetching are like http://cdn.apixu.com/weather/64x64/day/113.png so I'm guessing that this is an issue with the ordinal number of the current day of the year being three digits, v.s. two digits when you created the branch.

aaronbieber commented 6 years ago

The "g" or "ng" appearing after the icons was reported up above in this thread and I just haven't had time to look into it further. I have a fairly good idea of what might be happening, but it wasn't happening for me.

The tl;dr is that the filename of the icon from the API response is placed in a text property at the icon location, and then a separate function finds those values and asynchronously downloads, caches, and displays the icons themselves.

Probably some off-by-one type of mixup is causing the text property to get mangled, or something like that. The "g" or "ng" is the last characters of whatever.png, the icon filename itself.