Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Smart weather2 #128

Closed
wants to merge 4 commits into from
Closed

Smart weather2 #128

wants to merge 4 commits into from

Conversation

garbear
Copy link
Member

@garbear garbear commented May 3, 2011

Show the current weather based on the user's location.

The Google Gears API is attempted first, as it gives ~20m (~0.0001deg) accuracy regardless of proxy. IP-based geolocation is used as a backup with ~1-40km accuracy (assuming no proxy)

@garbear
Copy link
Member Author

garbear commented May 3, 2011

I prefer to avoid prompting the user at all costs. The code includes a prompt, and 999% of users should never see it, and those who do shouldn't see it twice. (essentially only due to network problems, as IPinfodb always returns at least a country)

Basically this patch tries to undo the paradigm of putting cool features before utility. Showing the weather on every single window is cool, and how useful is it if it's the wrong weather? ;-)

@davilla
Copy link
Contributor

davilla commented May 3, 2011

nice :)

@ghost
Copy link

ghost commented May 4, 2011

this seems to be a fruit salad of a pull request. the wireless stuff for sure doesn't belong here (not saying it's not welcome)

@garbear
Copy link
Member Author

garbear commented May 4, 2011

60 seconds tops, I can split it up into 2 divergent pulls requests ready
now. The first 6commits (bugs Andrea network enhancements) on the left, next
2 comits (location-based lookups) on the right.

Reviewability is 1. Give the word and it'll happen

EDIT: Haha this makes no sense

@ghost
Copy link

ghost commented May 4, 2011

cheers. i just pushed the location fix. if you could split out the network stuff that would be great

@garbear
Copy link
Member Author

garbear commented Jul 25, 2011

A lot's changed in the XBMC codebase over the last 3 months :)

I rebased on top of master (which included 5 of the original 9 commits in this PR). I modernized the JSON stuff to use CVariant and I made sure that the doxygen comments were in tippy top shape.

The 3 wifi commits have been submitted separately in #303. I'll rebase these 3 commits out later if PR #303 is merged.

…hannel and signal strength.

Includes general improvements and readability fixups in NetworkLinux.cpp.
The Google Gears API is attempted first, as it gives ~20m (~0.0001deg) accuracy regardless of proxy. IP-based geolocation is used as a backup with ~1-40km accuracy (assuming no proxy), which is good enough for weather.
@bb10
Copy link
Contributor

bb10 commented Aug 18, 2011

Excuse my interference, but I'd rather have a prompt (at least once on first start) asking for permission to get the user's location. I'm sure there are lots of people who don't want to give away their location to a third party however accurate their weather may become.

Also see: http://www.w3.org/TR/2008/WD-geolocation-API-20081222/#security

@garbear
Copy link
Member Author

garbear commented Aug 18, 2011

You make a fine point, my patch is an example of an engineer putting cool features before privacy considerations, and this should probably bee fixed if the ideas In this are worth using.

I'll check out the link u sent!

One could argue, as well, that all of the scraping services xbmc offers are a similar offense, though less drastic because music and video preferences are a less touchy subject than location. Food for thought

@mkortstiege
Copy link
Member

Closing the PR as weather is now handled via add-ons.

@garbear garbear mentioned this pull request May 5, 2011
popcornmix pushed a commit to popcornmix/xbmc that referenced this pull request Feb 22, 2017
AddonVideoCodec Implementation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants