-
Notifications
You must be signed in to change notification settings - Fork 9
Added the option to specify the VIN number to Update_cars #3
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -100,7 +100,8 @@ def from_json(self, data): | |
| res = json.loads(data.decode('utf-8')) | ||
|
|
||
| # I've never actually seen serverErrorMsgs, but allow for them | ||
| if res["serverErrorMsgs"] or type(res["data"]) == str: | ||
| # The Error message I was seeing was data = {"messages":[],"serverErrorMsgs":[],"data":"SERVER ERROR"} | ||
| if res["serverErrorMsgs"] or type(res["data"]) == str or res['data'] == 'SERVER ERROR': | ||
| raise ServerError(res) | ||
|
|
||
| d = res["data"] | ||
|
|
@@ -110,7 +111,7 @@ def from_json(self, data): | |
| for a in CAR_ATTRS: | ||
| setattr(self, a, d[a]) | ||
|
|
||
| except json.JSONDecodeError: | ||
| except ValueError: | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeh, JSONDecodeError was added in python 3.5, we can roll it back to value error for earlier versions. Probably also add 3.4 back into travis testing.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at the Travis tests I wasn't aware that this was developed with Python3. I have been doing all of my development and runs with Python2. Since Python2 isn't tested it might be nice to mention Python3 either in the README or put a
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, good idea. I had the trove information up to date, but we could throw a warning there as well. |
||
| _LOGGER.exception("Failure to decode json: %s" % data) | ||
| except KeyError as e: | ||
| _LOGGER.exception("Expected key not found") | ||
|
|
@@ -190,11 +191,18 @@ def get_cars(self): | |
| Location: %s | ||
| """ % (self.cookies, res.content, res.history)) | ||
|
|
||
| def update_cars(self): | ||
| def update_cars(self, vin=None): | ||
| headers = {"user-agent": USER_AGENT} | ||
| for c in self.cars: | ||
| url = EVSTATS_URL.format(c.vin, c.onstar) | ||
| res = requests.get(url, headers=headers, | ||
| cookies=self.cookies, allow_redirects=False) | ||
| c.from_json(res.content) | ||
| if vin: | ||
| if vin == c.vin: | ||
| url = EVSTATS_URL.format(c.vin, c.onstar) | ||
| res = requests.get(url, headers=headers,cookies=self.cookies, allow_redirects=False) | ||
| c.from_json(res.content) | ||
| return [c] | ||
| else: | ||
| url = EVSTATS_URL.format(c.vin, c.onstar) | ||
| res = requests.get(url, headers=headers,cookies=self.cookies, allow_redirects=False) | ||
| c.from_json(res.content) | ||
|
|
||
| return self.cars | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that instead of putting the special logic on the update_cars to take vin, probably just make vin optional to the MyChevy constructor. Then get_cars() will just filter out the list of cars that don't match the vin, and should throw an exception if no car was found. That will let update_cars() remain unchanged, as the loop will just be a single car element.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I like that idea better. I will look into making the change to the constructor. I will also try to create a unit test for it too and then submit a new pull request.