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

Fix/colorchooser #359

Merged
merged 6 commits into from Jul 11, 2016

Conversation

Projects
None yet
3 participants
@peuter
Copy link
Member

commented Jul 9, 2016

fixes initial sending of data to the backend when colorchooser gets loaded.

see: https://knx-user-forum.de/forum/supportforen/cometvisu/963486-problem-milight-colorchooser-mit-oh-als-backend

Also there are some minor code styling changes due to linter errors shown

@coveralls

This comment has been minimized.

Copy link

commented Jul 9, 2016

Coverage Status

Coverage remained the same at 31.128% when pulling 888bd23 on peuter:fix/colorshooser into 0b91a3b on CometVisu:develop.

@coveralls

This comment has been minimized.

Copy link

commented Jul 9, 2016

Coverage Status

Coverage decreased (-0.2%) to 30.927% when pulling b761c36 on peuter:fix/colorshooser into 0b91a3b on CometVisu:develop.

@@ -138,29 +141,29 @@ define( ['structure_custom', 'css!plugins/colorchooser/farbtastic/farbtastic.css
switch( wData.address[ ga ][2] )
{
case 'r':
wData.bus_r = value;
wData.bus_r = value * 100 / 255.0;

This comment has been minimized.

Copy link
@ChristianMayer

ChristianMayer Jul 9, 2016

Member

Only looking at the code (not really trying it): this seems wrong.

Here you are assuming value in the range of [0...255] and map it to 0...100 - and in line 146 below you are assuming value to be in the range of [0...100] and map it to [0...255]

@peuter peuter changed the title Fix/colorshooser Fix/colorchooser Jul 10, 2016

@coveralls

This comment has been minimized.

Copy link

commented Jul 10, 2016

Coverage Status

Coverage decreased (-0.2%) to 30.927% when pulling c79aa40 on peuter:fix/colorshooser into 0b91a3b on CometVisu:develop.

wData.bus_b = value[2];
wData.bus_r = value[0] * 100 / 255.0;
wData.bus_g = value[1] * 100 / 255.0;
wData.bus_b = value[2] * 100 / 255.0;
color = color.substring(0,1) +

This comment has been minimized.

Copy link
@ChristianMayer

ChristianMayer Jul 10, 2016

Member

The same as above. Is that really correct?

By comparing the cases I'd expect that lines 168, 169 and 170 should have been changed to toHex( value[...] * 255/100 ) instead

@peuter

This comment has been minimized.

Copy link
Member Author

commented Jul 11, 2016

You are right, I changed it.

@coveralls

This comment has been minimized.

Copy link

commented Jul 11, 2016

Coverage Status

Coverage increased (+2.3%) to 33.434% when pulling f7bbf2c on peuter:fix/colorshooser into 0b91a3b on CometVisu:develop.

@ChristianMayer ChristianMayer merged commit 1219260 into CometVisu:develop Jul 11, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+2.3%) to 33.434%
Details

@peuter peuter deleted the peuter:fix/colorshooser branch Oct 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.