Page MenuHomePhabricator

Add json-c dependency
AbandonedPublic

Authored by raster on Mar 4 2019, 2:19 AM.

Details

Summary

Old API retired => use yahoo weather JSON API

The new API is returning data in JSON format.
This commit is switching it over (currently SSL server) and parsing
to have the data in par with previous data.

Example data:
https://www.yahoo.com/news/_tdnews/api/resource/WeatherService;woeids=839722

Fix millibar to inches of mercury divider

Merge branch 'json-api'

Diff Detail

Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 11389
Build 8638: arc lint + arc unit
manio requested review of this revision.Mar 4 2019, 2:19 AM
manio created this revision.
vtorri requested changes to this revision.Mar 8 2019, 12:35 AM
vtorri added a subscriber: vtorri.

add meson support in addition to autotools

This revision now requires changes to proceed.Mar 8 2019, 12:35 AM
manio added a comment.Mar 8 2019, 3:20 AM

Guys!
Could you please merge this one?
I have now prepared a meson transition but I would like to have it done on top of this.
Please now merge it, I'll then prepare another phabricator Diff.

Not sure why i am here as a reviewer, I don't know anything about forecast or anything. In regards of that module, I did not even use it a single time.

manio added a comment.Mar 8 2019, 3:35 AM

09:46 < vtorri_> add bu5hm4n[m] as reviewer
09:47 < vtorri_> i haven't looked at e's code for a long time

raster added a subscriber: raster.Mar 8 2019, 7:27 AM

forecasts doesn't work for me - only says it supports us zip codes (and i don't live there) and can't enter anything in the entry box there in settings anyway, so can't test it.

manio updated this revision to Diff 20350.Mar 8 2019, 4:02 PM
  • Merge branch 'json-api'
  • Fix entering forecasts code
  • Setting dialog: update URL and description
manio added a comment.Mar 8 2019, 4:06 PM

@raster
Fixed now, please test again... :)

stephenmhouston requested changes to this revision.Mar 11 2019, 7:05 AM

Not going to merge this until the meson files are updated appropriately.

src/e_mod_config.c
115–116

You need to be more descriptive in your instructions here. For instance if I put in http://weather.yahoo.com it just takes me to https://www.yahoo.com/news/weather and has no code in it and just shows my location weather. If I actually click on a city, then the code is in the url. So beyond that you will need to explain that the code can be copied out of the url and explaining that it is the last few numbers after the hyphen. The way it is written right now makes it sound like they should just copy the entire url. Alternatively the yahoo weather api does take locations such as location=sunnyvale,ca or location=sunnyvale, or it can take latitude/longitude lat=37.372&lon=-122.038 ... Personally I think people inputting their city name or lat/long is much easier for them to find than getting yahoo's weather code for their area... but if you are going to stay with weather code you need to explain to the users in more detail how to get that code.

This revision now requires changes to proceed.Mar 11 2019, 7:05 AM
manio updated this revision to Diff 20416.Mar 11 2019, 9:17 AM
  • Setting dialog: update URL and description
  • Merge branch 'meson_support'
  • e_mod_main.c: remove unused variables, silent compiler warnings
vtorri accepted this revision.EditedMay 7 2019, 9:48 PM

meson is done so I accept, but i've not tested the module

raster requested changes to this revision.May 13 2019, 7:44 AM

i had some stale forecasts config that didn't have a server to contact in it....

so it does work. i'd advise using ecore_con_url tho so it handles proxies automatically with curl and redirects and any other http stuff needed in between... :) do you want to move to using that? the original code didn't use it either but it'd make for less code to handle proxies and to even handle the http header - only the json payload need be handled then...

?

This revision now requires changes to proceed.May 13 2019, 7:44 AM

oh while i'm at it...

_forecasts_popup_content_create() ...

inst->popup = e_gadcon_popup_new(inst->gcc, E_COMP_OBJECT_TYPE_POPUP);

should be

inst->popup = e_gadcon_popup_new(inst->gcc, 0);

that at the end is a bool - not an enum and its true for "don't give a shadow". you only want that if your content is going to have alpha content itself ... not in this case, so 0/EINA_FALSE will be what is needed.

manio updated this revision to Diff 22135.May 13 2019, 9:53 AM
  • e_mod_main.c: fix e_gadcon_popup_new() call
manio added a comment.May 13 2019, 9:57 AM

@raster
Thank you for your review :)
Can we please postpone the curl stuff? I'd like to have this whole thing commited as there are many changes currently in this phab diff and at least it will work for everyone which is cloning this...
The e_gadcon_popup is now also fixed, thanks for spotting this :)

raster accepted this revision.Jun 3 2019, 8:49 AM

we're good now. can be better using ecore_con_url and probably have some things be real buttons/links to yahoo.com so u just click the link or a built-in place name search/browser... but... this is a good step so thumbs up. :)

raster commandeered this revision.Jul 30 2019, 3:50 AM
raster edited reviewers, added: manio; removed: raster.

Going to close as this actually has been pushed...

raster abandoned this revision.Jul 30 2019, 3:50 AM

it's been pushed. phab didnt figure it out.