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

Add (EU-based) Honeywell evohome CH/DHW controller #16427

Merged
merged 88 commits into from Sep 27, 2018

Conversation

@zxdavb
Contributor

zxdavb commented Sep 4, 2018

Honeywell evohome CH/DHW Controllers (EU/EMEA only)

This is a hub component (for it's slave heating zones, with/without a DHW controller, a.k.a. boiler), and a climate entity. Thus implemented as a evohome component.

No support for heating zones, DHW controller as yet - this will be added in future PRs.

Description:

Support for a EU/EMEA-based Honeywell evohome controllers accessible via mytotalconnectcomfort.com Such systems consists of a 'location' with a temperature control system (TCS, controller) with 0-12 heating zones (e.g. TRVs, UFH relays) and, optionally, a DHW controller.

To be clear, this component uses the evohomeclient client library, and does not (can not) support US-based systems (honeywell.py uses two distinct client libraries, evohomeclient, and somecomfort).

Related issue (if applicable):
Addresses many of the limitations of the existing honeywell.py component:

  • exposes the controller, and provides access to its operating modes (e.g. proper Away mode support)
  • permits access to locations other than location 0 (or, more simply, you have multiple controllers - you can choose only one, though)
  • TBA: exposes the DHW controller, and exposes its operating modes, etc.
  • TBA: allows access to higher-precision current_temperatures via the v1 client library (0.1 displayed, 0.01 logged)
  • significant improvements to efficiency (e.g. batching add_entities(), and initial update of state delayed until after EVENT_HOMEASSISTANT_START)
  • many fewer api calls / scan_interval - it seems you can safely use a scan_interval of 60 seconds
  • TBA: access to schedules & setpoints (read-only for now)

This component uses v2 of the evohomeclient client library (honeywell.py uses v1 only), with many new features/bug fixes:

  • uses newer api, exposing more information
  • resolve LOGGER bug (via a workaround in the component)
  • much better exception handling

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#6252

Example entry for configuration.yaml (if applicable):

evohome:
  username: !secret evohome_username
  password: !secret evohome_password

# scan_interval: 300     # seconds, you can probably get away with 60
# location_idx: 0        # if you have more than 1 location, use this

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • [ N/A ] Tests have been added to verify that the new code works.
Add support for Honeywell evohome CH/DHW systems
More flake8 corrections

Passes Flake8 tests

Almost passed flake8.pylint!

Passed all tox tests

Now it needs serious testing!

Almost ready to submit

BUGFIX: DHW state now functional

More improvements to available()

Solved the DHW temp units problem!

Last minute bug squash

to improve dicts merge

Trying to rebase

fixing more rbase errors

revert to creating HTTP_error_code internally for now

ready to submit PR

Added support for Honeywell evohome CH/DHW systems

@zxdavb zxdavb requested a review from home-assistant/core as a code owner Sep 4, 2018

@homeassistant

This comment has been minimized.

homeassistant commented Sep 4, 2018

Hi @zxdavb,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@homeassistant homeassistant added cla-signed and removed cla-needed labels Sep 4, 2018

@zxdavb

This comment has been minimized.

Contributor

zxdavb commented Sep 4, 2018

There are three architectural issues with this PR:

  1. The Climate entity is under discussion
  2. The controller is implemented as a Climate-like entity (there seems to be no appropriate component class elsewhere in HA).
  3. The DHW controller is implemented as a Climate-like entity (there is currently no Boiler component class).

@zxdavb zxdavb changed the title from Add support for Honeywell evohome CH/DHW systems to Refactor/Add EU-based Honeywell evohome CH/DHW systems Sep 7, 2018

zxdavb added some commits Sep 8, 2018

zxdavb added some commits Sep 25, 2018

@MartinHjelmare

I think this is ok now. There are still some remaining if _LOGGER.isEnabledFor(logging.DEBUG): which I don't think is necessary, but nothing major in the way of merging in my opinion.

zxdavb added some commits Sep 25, 2018

@zxdavb

This comment has been minimized.

Contributor

zxdavb commented Sep 25, 2018

@MartinHjelmare thanks for all your help

@zxdavb

This comment has been minimized.

Contributor

zxdavb commented Sep 26, 2018

From: https://deploy-preview-92--developers-home-assistant.netlify.com/docs/en/next/integration_quality_scale_index.html, for a QIS score of Silver:

  • Set an appropriate SCAN_INTERVAL (if a polling integration) - 180 sec
  • Raise PlatformNotReady if unable to connect during platform setup (if appropriate) - N/A (see above)
  • Handles expiration of auth credentials. Refresh if possible or print correct error and fail setup. If based on a config entry, should trigger a new config entry flow to re-authorize. - _credentials in configuration.yaml, and handles bad credentials with a _LOGGER.error() & return False
  • Handles internet unavailable. Log a warning once when unavailable, log once when reconnected - needs adding in a future PR
  • Handles device/service unavailable. Log a warning once when unavailable, log once when reconnected - yes
  • Set available property to False if appropriate (docs) - yes
  • Entities have unique ID (if available) - the api is undocumented, but it is believed the Honeywell-assigned IDs are unique
@MartinHjelmare

This comment has been minimized.

Member

MartinHjelmare commented Sep 26, 2018

Note that the last point means that the unique_id property should be defined in the platform entity class.

https://developers.home-assistant.io/docs/en/entity_index.html#generic-properties

@zxdavb

This comment has been minimized.

Contributor

zxdavb commented Sep 26, 2018

Note that the last point means that the unique_id property should be defined in the platform entity class.

OK, I'll take that tick off, and look to achieving Silver in a subsequent PR (should be doable), once I understand exactly you mean, above.

zxdavb added some commits Sep 27, 2018

@balloob

This comment has been minimized.

Member

balloob commented Sep 27, 2018

🎉 🎉 🎉 🎉 🎉

@balloob balloob merged commit 8d65230 into home-assistant:dev Sep 27, 2018

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.0003%) to 93.602%
Details

@wafflebot wafflebot bot removed the in progress label Sep 27, 2018

@zxdavb zxdavb deleted the zxdavb:evohome-component branch Sep 27, 2018

@balloob balloob referenced this pull request Oct 12, 2018

Merged

0.80.0 #17361

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment