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

Improved Fritz!Box thermostat support #14789

Merged
merged 2 commits into from
Jun 5, 2018
Merged

Improved Fritz!Box thermostat support #14789

merged 2 commits into from
Jun 5, 2018

Conversation

thomaskr
Copy link
Contributor

@thomaskr thomaskr commented Jun 3, 2018

  • on and off state

  • unittests

Description:

The recent version of the Fritz!Box thermostat connector lacks support for on and off mode and reports target temperatures of 126.5 and 127.0 °C with mode manual instead.
This is a result of two special temperature values (253, 254) used by Fritz!Box API. pyfritzbox in between converts them to 126.5 and 127.

I fixed the target temperature display, using 0°C (off) and 30°C (on) instead, added on and off mode and several unit tests.

No configuration changes are required. The changes should be backward compatible but old bad temperature and mode values will remain in history.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox.

Even without any change tox fails local machine with Ubuntu 18.04 in an unrelated test. Tox runs in Travis CI.

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:

  • Tests have been added to verify that the new code works.

* on and off state

* unittests
@homeassistant
Copy link
Contributor

Hi @thomaskr,

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!

@syssi
Copy link
Member

syssi commented Jun 3, 2018

Why do you choose 30°C as target temperature for "on"?

@thomaskr
Copy link
Contributor Author

thomaskr commented Jun 3, 2018

@syssi Due to how pyfritzbox works internally it had to be a temperature > 28.0°C (the MAX_TEMPERATURE). So, it could also be 28.5 or 29 but smoother values look more comfortable to the user. Pretty much the same applies to the off temperature.
If there is a suitable option in Home Assistant not to present a specific temperature, please point me into the right direction.

* Removed target temperature from on and off mode
@thomaskr
Copy link
Contributor Author

thomaskr commented Jun 5, 2018

@syssi I noticed that setting the target_temperature to None will have the right effect.

@syssi syssi merged commit 549abd9 into home-assistant:dev Jun 5, 2018
@balloob balloob mentioned this pull request Jun 22, 2018
girlpunk pushed a commit to girlpunk/home-assistant that referenced this pull request Sep 4, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Sep 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants