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

Standardized temperature units for LCN sensors #22108

Merged
merged 1 commit into from
Mar 16, 2019

Conversation

alengwenus
Copy link
Contributor

Use the standardized temperature units for Celsius and Fahrenheit for the LCN sensor compoenent.
Also the corresponding documentation has been updated.

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

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). - [ ] 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.

@ghost ghost added the in progress label Mar 16, 2019
@alengwenus alengwenus mentioned this pull request Mar 16, 2019
8 tasks
Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@MartinHjelmare
Copy link
Member

Can be merged when build passes.

@Danielhiversen Danielhiversen merged commit 0466e43 into home-assistant:dev Mar 16, 2019
@ghost ghost removed the in progress label Mar 16, 2019
@alengwenus alengwenus deleted the dev_lcn_temp_const branch March 17, 2019 05:10
@balloob balloob mentioned this pull request Apr 3, 2019
weaselshit pushed a commit to weaselshit/home-assistant that referenced this pull request Apr 4, 2019
There are only TEMP_CELSIUS and TEMP_FAHRENHEIT, even though several components
use Kelvins internally. home-assistant#22108 works around this, by handling Kelvins itself, while using own hardcoded string for Kelvins (incorrect, bt the way, see next commit for details).

This commit adds TEMP_FAHRENHEIT constant that may be used by all components. LCN component will be updated in a separate commit.
weaselshit pushed a commit to weaselshit/home-assistant that referenced this pull request Apr 4, 2019
This was addresssed partially in home-assistant#22108, but it uses wrong units for temperature
in Kelvins. It must be just "K" (Kelvins, plural), not "°K" ("degrees of Kelvin"), it has been like that for over 50 years now: https://en.wikipedia.org/wiki/Kelvin#Usage_conventions.

This commit ensures newly added TEMP_KELVIN constant from homeassistan.const is
used instead of hardcoded incorrect string.
weaselshit pushed a commit to weaselshit/home-assistant that referenced this pull request Apr 4, 2019
There are only TEMP_CELSIUS and TEMP_FAHRENHEIT, even though several components
use Kelvins internally. home-assistant#22108 works around this, by handling Kelvins itself, while using own hardcoded string for Kelvins (incorrect, bt the way, see next commit for details).

This commit adds TEMP_KELVINS constant that may be used by all components. LCN component will be updated in a separate commit.
weaselshit pushed a commit to weaselshit/home-assistant that referenced this pull request Apr 4, 2019
This was addresssed partially in home-assistant#22108, but it uses wrong units for temperature
in Kelvins. It must be just "K" (Kelvins, plural), not "°K" ("degrees of Kelvin"), it has been like that for over 50 years now: https://en.wikipedia.org/wiki/Kelvin#Usage_conventions.

This commit ensures newly added TEMP_KELVINS constant from homeassistan.const is
used instead of hardcoded incorrect string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed integration: lcn small-pr PRs with less than 30 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants