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

[Tradfri] Fix bug in color space conversion #4351

Merged
merged 1 commit into from Oct 4, 2017

Conversation

Projects
None yet
5 participants
@hreichert
Copy link
Contributor

commented Sep 26, 2017

Ensure that calculated RGB values are not negative.
Added new unit test case.

Fixes: #4349

@openhab-bot

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2017

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/ikea-tradfri-gateway/26135/127

int redRounded = (int) Math.round(red * 255.0);
int greenRounded = (int) Math.round(green * 255.0);
int blueRounded = (int) Math.round(blue * 255.0);
int redRounded = ensureRgbRange((int) Math.round(red * 255.0));

This comment has been minimized.

Copy link
@triller-telekom

triller-telekom Sep 28, 2017

Contributor

I wonder if this is the right place for the fix. Shouldn't we make sure that red/green/blue is not negative and not above 1? At least the later is already checked in lines 108-120 so you would only need to check for negative numbers.

This comment has been minimized.

Copy link
@hreichert

hreichert Sep 28, 2017

Author Contributor

Thanks for your comment, that would be indeed better.
I'm going to change this and rebase on master, as there are now duplicate method names in the tests I guess.

This comment has been minimized.

Copy link
@hreichert

hreichert Oct 2, 2017

Author Contributor

Done.

[Tradfri] Fix bug in color space conversion
Ensure that calculated RGB values are not negative.

Fixes: #4349

Signed-off-by: Holger Reichert <mail@h0lger.de>

@hreichert hreichert force-pushed the hreichert:4349-tradfri-fix-color-conversion branch from 0f74e56 to d61ac8d Oct 2, 2017

@hreichert

This comment has been minimized.

Copy link
Contributor Author

commented Oct 2, 2017

Updated PR with suggested change from @triller-telekom

@sjka sjka merged commit d93353b into eclipse:master Oct 4, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
ip-validation
Details
@sjka

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2017

Thanks!

@hreichert hreichert deleted the hreichert:4349-tradfri-fix-color-conversion branch Oct 4, 2017

@kaikreuzer kaikreuzer added this to the 0.9.0 milestone Nov 30, 2017

@kaikreuzer kaikreuzer added the bug label Dec 15, 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.