Skip to content
This repository has been archived by the owner. It is now read-only.

[Tradfri] Fix bug in color space conversion #4351

Merged

Conversation

@hreichert
Copy link
Contributor

@hreichert hreichert commented Sep 26, 2017

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

Fixes: #4349

@openhab-bot
Copy link
Contributor

@openhab-bot openhab-bot 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));
Copy link
Contributor

@triller-telekom triller-telekom Sep 28, 2017

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.

Copy link
Contributor Author

@hreichert hreichert Sep 28, 2017

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.

Copy link
Contributor Author

@hreichert hreichert Oct 2, 2017

Done.

Ensure that calculated RGB values are not negative.

Fixes: eclipse-archived#4349

Signed-off-by: Holger Reichert <mail@h0lger.de>
@hreichert hreichert force-pushed the 4349-tradfri-fix-color-conversion branch from 0f74e56 to d61ac8d Oct 2, 2017
@hreichert
Copy link
Contributor Author

@hreichert hreichert commented Oct 2, 2017

Updated PR with suggested change from @triller-telekom

@sjsf sjsf merged commit d93353b into eclipse-archived:master Oct 4, 2017
2 checks passed
@sjsf
Copy link
Contributor

@sjsf sjsf commented Oct 4, 2017

Thanks!

@hreichert hreichert deleted the 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 subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants