watchforstock / evohome-client

Python client to access the Evohome web service
Apache License 2.0
88 stars 52 forks source link

Bugfix and refactor - ready to pull #79

Closed zxdavb closed 5 years ago

zxdavb commented 5 years ago

Lots of stuff:

  1. Added some additional logging.

  2. Removed base.py altogether; this required a bit of refactoring.

  3. Minor improvements to docstrings, including PEP-257.

  4. As a rule, moved lint-hints out to column 81 to distinguish them from comments.

  5. Removed superfluous code, such as the if conditional:

    if response.status_code != requests.codes.ok:
    requests.raise_for_status()
  6. Minor improvements to code (e.g. A=B=None, instead of A=None, and B=None).

  7. Removed unneeded wrappers, e.g.: self.client._convert(response.text) becomes response.json().

  8. Fix dodgy super()s.

  9. Fixes a potential bug, (e.g. access_token == ''):

    if self.access_token is None or self.access_token_expires is None:
    self._basic_login()

    becomes

    if not self.access_token or not self.access_token_expires:
    self._basic_login()

    BTW, not (self.access_token and self.access_token_expires) is more concise, but the above reads better.

zxdavb commented 5 years ago

I think a few more things are needed:

  1. I think there is a bug when > location per account - I will chase this Not really
  2. Refactoring due to removal of base.py isn't necessarily complete - I will review
  3. DONE: Something I forget (line 102 - if self.refresh_token is None:)
  4. DONE: I think we should get rid of raise EvohomeClientInvalidPostData()
zxdavb commented 5 years ago

After commit 0a034db:

(hass) dbonnes@vm-builder:~/evohome-client/evohomeclient2$ flake8 *.py --ignore=E501
(hass) dbonnes@vm-builder:~/evohome-client/evohomeclient2$ pylint *.py --disable=C0301
************* Module evohomeclient2.controlsystem
controlsystem.py:21:2: W0511: TODO: is systemId set via self.__dict__.update(local_data)? (fixme)
controlsystem.py:10:0: R0902: Too many instance attributes (8/7) (too-many-instance-attributes)
************* Module evohomeclient2
__init__.py:39:4: R0913: Too many arguments (7/5) (too-many-arguments)

------------------------------------------------------------------
Your code has been rated at 9.91/10 (previous run: 9.89/10, +0.03)
zxdavb commented 5 years ago

Argh! client.locations[0]._gateways[0]._control_systems[0].zones has two copies of every zone!

And I can't work out why! OK, I was stupid.

Fixed now.

zxdavb commented 5 years ago

So the good news is that chasing this bug required me to make extensive use of refresh_tokens and access_tokens on my HomeAssistant component - they performed flawlessly.