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

Adding support for FRITZ DECT 100 (temp sensor) #20308

Merged
merged 1 commit into from Jan 30, 2019

Conversation

Projects
None yet
6 participants
@choss
Copy link
Contributor

choss commented Jan 21, 2019

Description:

Added automatic setup of fritz!box components which are temperature sensors only.

Related issue (if applicable): n/a

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

Example entry for configuration.yaml (if applicable):

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

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][ex-requir]).
  • New dependencies are only imported inside functions that use them ([example][ex-import]).
  • 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.
Show resolved Hide resolved homeassistant/components/sensor/fritzbox.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/fritzbox.py
Show resolved Hide resolved homeassistant/components/sensor/fritzbox.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/fritzbox.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/fritzbox.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/fritzbox.py

@choss choss force-pushed the choss:dev branch from ab3d784 to b986308 Jan 21, 2019

@choss choss force-pushed the choss:dev branch 2 times, most recently from 706e08f to 0c8496d Jan 21, 2019

@homeassistant

This comment has been minimized.

Copy link

homeassistant commented Jan 21, 2019

Hi @choss,

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!

@frenck

This comment has been minimized.

Copy link
Member

frenck commented Jan 21, 2019

⚠️ I was unable to find a documentation PR (matching this code change) in our documentation repository.

📚Please, update the documentation and open a PR for it (be sure to create a documentation PR against the next branch) and link 🔗 this PR and the new documentation PR by mentioning the link to the PR's in both openings posts/descriptions.

🏷 I am adding the docs-missing label until this has been resolved.

👍 Thanks (times a million!)

@frenck frenck added the docs-missing label Jan 21, 2019

@choss choss force-pushed the choss:dev branch from 0c8496d to 47da26c Jan 22, 2019

@choss choss referenced this pull request Jan 22, 2019

Merged

adding documentation for fritz.dect 100 sensor component #8242

2 of 2 tasks complete
@choss

This comment has been minimized.

Copy link
Contributor Author

choss commented Jan 22, 2019

⚠️ I was unable to find a documentation PR (matching this code change) in our documentation repository.

📚Please, update the documentation and open a PR for it (be sure to create a documentation PR against the next branch) and link 🔗 this PR and the new documentation PR by mentioning the link to the PR's in both openings posts/descriptions.

🏷 I am adding the docs-missing label until this has been resolved.

👍 Thanks (times a million!)

Documentation is added in the descriptions. Sorry for the mess I am making in this PR

@property
def name(self):
"""Return the name of the device."""
return self._device.name

This comment has been minimized.

@rytilahti

rytilahti Jan 29, 2019

Contributor

Is this doing I/O? If not (assuming update() populates them), this can be merged as it is. Otherwise this, state and device_state_attributes need be modified not to do any I/O.

This comment has been minimized.

@choss

choss Jan 30, 2019

Author Contributor

I checked the underlying library and it doesn't do I/O here. The underlying library caches it in the properties and updates the relevant parts with the update method.

This comment has been minimized.

@rytilahti

rytilahti Jan 30, 2019

Contributor

Great, merged! 🥇 Please update the ha_release part in the docs and let's get that merged in, too.

"""Get latest data and states from the device."""
try:
self._device.update()
except requests.exceptions.HTTPError as ex:

This comment has been minimized.

@rytilahti

rytilahti Jan 29, 2019

Contributor

Just a side note (and more related to the backend lib), but its developer should consider wrapping such exceptions related to internals of the library under a FritzException to make it easier to catch them.

@rytilahti rytilahti merged commit ed299a9 into home-assistant:dev Jan 30, 2019

5 checks passed

Hound No violations found. Woof!
WIP Legacy commit status override — see details
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 93.11%
Details

@wafflebot wafflebot bot removed the in progress label Jan 30, 2019

@home-assistant home-assistant deleted a comment from homeassistant Jan 31, 2019

@home-assistant home-assistant deleted a comment from homeassistant Jan 31, 2019

@balloob balloob referenced this pull request Feb 6, 2019

Merged

0.87.0 #20794

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