Skip to content
This repository has been archived by the owner on Jun 9, 2018. It is now read-only.

Googleapi fix #30

Merged
merged 8 commits into from
Feb 26, 2018
Merged

Googleapi fix #30

merged 8 commits into from
Feb 26, 2018

Conversation

danner26
Copy link
Contributor

@danner26 danner26 commented Feb 23, 2018

Fixed the API for Google and added handling. Also added another CLI switch for manual key input
#27

@danner26
Copy link
Contributor Author

I think i'm also going to add in a fallback switch/option. The way I see it, if the Apple lookup fails, or the Google lookup fails, you can specify the switch to try and fall back to the other service. If both fails, then report so, otherwise return the proper data.

Just an idea.. not too sure how practical this is

@clburlison
Copy link
Owner

@danner26 Thank you for working on this. I can see people liking the fallback option. I'll review what you have end of day. On first glance everything looks pretty.

Removed the requests module so it doesnt require any other modules
@danner26
Copy link
Contributor Author

I changed it around a little so there is logging and it doesn't depend on the requests module (AKA no extra module installs)
Otherwise everything appears to be working

@danner26
Copy link
Contributor Author

Tested with sudo ./pinpoint -f -l google -a google -k {MY_API_KEY} -vv for reference

Copy link
Owner

@clburlison clburlison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danner26 Overall this is great and works properly. I have a few style nits that I'd like to get cleaned up before merging. Once that is done I'll cut a release with this update.

.gitignore Outdated
@@ -3,4 +3,7 @@ tmp/
.pyc
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this was a mistake on my part. Could you please change this line to be *.pyc and remove lines 7-9 that you added?

val.get('BSSID'),
val.get('SSID_STR'),
val.get('RSSI')))
request_data['wifiAccessPoints'].append({'macAddress':val.get('BSSID'), 'signalStrength':val.get('RSSI'), 'signalToNoiseRatio':0})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is too long and fails linting. Please change to:

            request_data['wifiAccessPoints'].append({
                'macAddress': val.get('BSSID'),
                'signalStrength': val.get('RSSI'),
                'signalToNoiseRatio': 0}

obj = json.loads(data)
dump = json.dumps(request_data).encode("utf8")
try:
request = Request(api_url, dump, {'Content-type': 'application/json'})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long and fails linting. Please change to:

            request = Request(api_url, dump,
                              {'Content-type': 'application/json'}

@danner26
Copy link
Contributor Author

Changes have been implemented. Should be good now.

I will create a new pull request once I finish the fallback option.

@clburlison clburlison merged commit 269a124 into clburlison:master Feb 26, 2018
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.

3 participants