Color temperature for lights by farmio · Pull Request #154 · XKNX/xknx

farmio · 2018-10-07T09:27:41Z

added support for absolute and relative color temperature functions of lights.

'tunable_white' sets the relative color temperature in %
'color_temperature' sets the absolute color temperature in Kelvin

this PR supersedes #153

coveralls · 2018-10-07T09:31:28Z

Coverage increased (+0.2%) to 89.245% when pulling 8b4d345 on farmio:color-temperature into b5d8be0 on XKNX:master.

farmio · 2018-11-11T16:17:57Z

I don't know why the coveralls coverage decreased. Everything I have added should have tests as well.
I haven't touched the files reported for decreasing coverage:
xknx/devices/notification.py and
xknx/devices/action.py

Julius2342 · 2018-11-11T17:09:56Z

Coveralls sometims does not like us :)

(im pretty busy with some 35C3 stuff atm, but will have a look at your MR asap...)

marvin-w · 2018-12-26T14:30:11Z

@farmio: Can you please update this PR so that it is up to date with the master branch? :)

marvin-w

@farmio Thank you for the PR. Please merge the current master in your remote branch and take care of the review comments (where needed). I'd then merge it and take care of the PR at HA.

It would be great if you could retest this since at least I am unable to test this without the appropriate devices.

marvin-w · 2018-12-26T14:31:39Z

 from homeassistant.helpers.script import Script

-REQUIREMENTS = ['xknx==0.8.5']
+REQUIREMENTS = ['xknx==0.9.0']


please remove this from the PR. The current version will be 0.9.3 and we will merge it from HA directly into XKNX.

marvin-w · 2018-12-26T14:35:17Z

    PYTHONPATH = {toxinidir}
 commands =
    py.test --cov --cov-report= {posargs}
+whitelist_externals = make


Why did you add this?

Without it tox throws a warning:
pylint runtests: commands[0] | make pylint WARNING: test command found but not installed in testenv cmd: /usr/bin/make env: /Users/meti/Documents/Dev/xknx/.tox/pylint Maybe you forgot to specify a dependency? See also the whitelist_externals envconfig setting.
on my system. Same for
pydocstyle runtests: commands[0] | make pydocstyle

Could this be the reason for the decreased coverage? These warnings occurred with the travis tests of the latest PR #161 too.
See for example https://travis-ci.org/XKNX/xknx/jobs/471596429 line 589 ff.
cc @Julius2342

farmio · 2018-12-27T01:30:56Z

I have tested it with Home-Assistant 0.84.6 and it works properly with an MDT Led Controller AKD-0424R.02

The only error was

File "***/.homeassistant/custom_components/climate/xknx.py", line 72, in
vol.Optional(CONF_MIN_TEMP): cv.Coerce(float),
AttributeError: module 'homeassistant.helpers.config_validation' has no attribute 'Coerce'

but I haven't touched that file.

marvin-w · 2018-12-27T01:49:20Z

@farmio Thanks for your fast feedback. We somehow need to get rid of those coverage "errors". I already fixed the error in climate.py in this PR: home-assistant/core#19546.

I will merge the changes into our project once the PR is merged.

There seem to be a lot of changes in this PR that are not related to the actual code changes. Can you fix this please?

farmio · 2018-12-27T07:50:28Z

Never mind. I have done another rebase and force-pushed it. Seems good now.

I don't really know how these changes got in here. They are not visible at master...farmio:color-temperature . Maybe something went wrong when I did the git rebase upstream/master.
What can I do about it? Shall I create a new PR?

Also I am not entirely sure if ATTR_WHITE_VALUE and SUPPORT_WHITE_VALUE are the best choices for relative color temperature settings. I couldn't find documentation about this back in October.
It works, but maybe it will conflict with RGBW lights once they get implemented in xknx. I don't really know if tunable white is a thing with RGBW.

- added tunable white function for setting relative color temperature for lights - tests for tunable white function - whitelist 'make' for tox - removed double whitespace in device.py

- added color temperature function for setting absolute color temperature values for lights - added tests for color remperature function - reordered DPTMAP in remote_value_sensor.py by DPT number - added DPTColorTemperature class for DPT 7.600

- added color temperature for home assistant plugin - added min and max kelvin / mireds

- removed unused import - bumped required version of xknx in ha-plugin - disabled pylint too-many-locals warning in light.py

- added color temperature function for setting absolute color temperature values for lights - added tests for color remperature function - reordered DPTMAP in remote_value_sensor.py by DPT number - added DPTColorTemperature class for DPT 7.600

- added color temperature for home assistant plugin - added min and max kelvin / mireds

- removed unused import - bumped required version of xknx in ha-plugin - disabled pylint too-many-locals warning in light.py

Avoid triple conversion while comparing against min / max color temperature when a new value is set.

marvin-w · 2018-12-27T18:08:53Z

@farmio thanks for updating the PR. I will take another look tomorrow when I'm back home!

Julius2342 · 2018-12-28T14:37:16Z

🐬

farmio mentioned this pull request Oct 7, 2018

tunable white function for lights #153

Closed

marvin-w requested changes Dec 26, 2018

View reviewed changes

farmio added 11 commits December 27, 2018 15:19

tunable white function for lights

8b50ce0

- added tunable white function for setting relative color temperature for lights - tests for tunable white function - whitelist 'make' for tox - removed double whitespace in device.py

add RemoteValueDpt2ByteUnsigned

eb90d7a

color temperature function for light

9333226

- added color temperature function for setting absolute color temperature values for lights - added tests for color remperature function - reordered DPTMAP in remote_value_sensor.py by DPT number - added DPTColorTemperature class for DPT 7.600

tuneable white for home assistant plugin

e3a5c53

- added color temperature for home assistant plugin - added min and max kelvin / mireds

minor fixes to soothe the ci beast

facf362

- removed unused import - bumped required version of xknx in ha-plugin - disabled pylint too-many-locals warning in light.py

color temperature function for light

a8c1222

- added color temperature function for setting absolute color temperature values for lights - added tests for color remperature function - reordered DPTMAP in remote_value_sensor.py by DPT number - added DPTColorTemperature class for DPT 7.600

tuneable white for home assistant plugin

7589134

- added color temperature for home assistant plugin - added min and max kelvin / mireds

minor fixes to soothe the ci beast

1be80a2

- removed unused import - bumped required version of xknx in ha-plugin - disabled pylint too-many-locals warning in light.py

rebased to 0.9.3

edd7f66

set relative color temperature (tuneable white) from HA plugin

2df4820

Update .gitignore

ac432ab

farmio force-pushed the color-temperature branch from 0dded5b to ac432ab Compare December 27, 2018 15:00

avoid multiple conversions from mired to kelvin and vice versa

8b4d345

Avoid triple conversion while comparing against min / max color temperature when a new value is set.

Julius2342 merged commit b26b8a8 into XKNX:master Dec 28, 2018

farmio deleted the color-temperature branch December 28, 2018 19:34

farmio mentioned this pull request Dec 29, 2018

update light.knx for xknx 0.9.4 home-assistant/home-assistant.io#7979

Merged

2 tasks

Uh oh!

Conversation

farmio commented Oct 7, 2018

Uh oh!

coveralls commented Oct 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

farmio commented Nov 11, 2018

Uh oh!

Julius2342 commented Nov 11, 2018

Uh oh!

marvin-w commented Dec 26, 2018

Uh oh!

marvin-w left a comment

Choose a reason for hiding this comment

Uh oh!

marvin-w Dec 26, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marvin-w Dec 26, 2018

Choose a reason for hiding this comment

Uh oh!

farmio Dec 26, 2018

Choose a reason for hiding this comment

Uh oh!

farmio Dec 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

farmio commented Dec 27, 2018

Uh oh!

marvin-w commented Dec 27, 2018

Uh oh!

farmio commented Dec 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marvin-w commented Dec 27, 2018

Uh oh!

Julius2342 commented Dec 28, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coveralls commented Oct 7, 2018 •

edited

Loading

farmio Dec 27, 2018 •

edited

Loading

farmio commented Dec 27, 2018 •

edited

Loading